From 7bd41bfbb90113f85602ae8fca2a282ace5575b3 Mon Sep 17 00:00:00 2001 From: Todd Strader Date: Wed, 3 Jun 2026 19:50:56 -0400 Subject: [PATCH] Fix MULTIDRIVEN in generates (#7709) --- src/V3Undriven.cpp | 11 +++- src/V3Width.cpp | 55 +++++++++---------- src/V3Width.h | 1 + .../t/t_lint_always_ff_multidriven_genif.py | 16 ++++++ .../t/t_lint_always_ff_multidriven_genif.v | 28 ++++++++++ 5 files changed, 79 insertions(+), 32 deletions(-) create mode 100755 test_regress/t/t_lint_always_ff_multidriven_genif.py create mode 100644 test_regress/t/t_lint_always_ff_multidriven_genif.v diff --git a/src/V3Undriven.cpp b/src/V3Undriven.cpp index 51187098f..0ac631d3b 100644 --- a/src/V3Undriven.cpp +++ b/src/V3Undriven.cpp @@ -29,6 +29,7 @@ #include "V3Stats.h" #include "V3UndrivenCapture.h" +#include "V3Width.h" #include @@ -369,6 +370,7 @@ class UndrivenVisitor final : public VNVisitorConst { bool m_inProcAssign = false; // In procedural assignment bool m_inFTaskRef = false; // In function or task call bool m_inInoutOrRefPin = false; // Connected to pin that is inout + bool m_inSelLhs = false; // Iterating the fromp of an AstSel (a partial-bit write target) const AstNodeFTask* m_taskp = nullptr; // Current task const AstAlways* m_alwaysp = nullptr; // Current always of either type const AstAlways* m_alwaysCombp = nullptr; // Current always if combo, otherwise nullptr @@ -473,8 +475,11 @@ class UndrivenVisitor final : public VNVisitorConst { entryp->usedBit(lsb, nodep->width(), varrefp); } } else { - // else other varrefs handled as unknown mess in AstVarRef - iterateChildrenConst(nodep); + // skip over static longest static prefix + iterateConst(nodep->lsbp()); + VL_RESTORER(m_inSelLhs); + m_inSelLhs = !V3Width::selectNonConstantRecurse(nodep->lsbp(), /*inSel=*/true); + iterateConst(nodep->fromp()); } } void visit(AstNodeVarRef* nodep) override { @@ -530,7 +535,7 @@ class UndrivenVisitor final : public VNVisitorConst { if (entryp->isDrivenWhole() && !m_inBBox && !VN_IS(nodep, VarXRef) && !VN_IS(nodep->dtypep()->skipRefp(), UnpackArrayDType) && !sameFileLine && !entryp->isUnderGen() && otherWritep && !entryp->isFtaskDriven() - && !ftaskDef + && !ftaskDef && !m_inSelLhs && !nodep->varp()->fileline()->warnIsOff(V3ErrorCode::MULTIDRIVEN)) { const bool otherWriteIsStaticInit = nodep->varp()->hasUserInit() && otherWritep == entryp->initStaticp(); diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 18013533b..7f45a9d23 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -9948,7 +9948,7 @@ class WidthVisitor final : public VNVisitor { } void checkForceReleaseLhs(AstNode* nodep, AstNode* lhsp) { // V3Force can't check as vector may have expanded, or propagated constant into index - if (AstNode* const selNodep = selectNonConstantRecurse(lhsp)) + if (AstNode* const selNodep = V3Width::selectNonConstantRecurse(lhsp)) nodep->v3error((VN_IS(nodep, Release) ? "Release"s : "Force"s) + " left-hand-side must not have variable bit/part select " "(IEEE 1800-2023 10.6.2)\n" @@ -9956,34 +9956,6 @@ class WidthVisitor final : public VNVisitor { << selNodep->warnOther() << "... Location of non-constant index\n" << selNodep->warnContextSecondary()); } - AstNode* selectNonConstantRecurse(AstNode* nodep, bool inSel = false) { - // If node has a non-constant select, return that select - AstNode* resultp = nullptr; - if (AstNodeSel* const anodep = VN_CAST(nodep, NodeSel)) { - resultp = selectNonConstantRecurse(anodep->fromp(), inSel); - if (resultp) return resultp; - resultp = selectNonConstantRecurse(anodep->bitp(), true); - } else if (AstSel* const anodep = VN_CAST(nodep, Sel)) { - resultp = selectNonConstantRecurse(anodep->fromp(), inSel); - if (resultp) return resultp; - resultp = selectNonConstantRecurse(anodep->lsbp(), true); - } else if (AstNodeVarRef* const anodep = VN_CAST(nodep, NodeVarRef)) { - if (inSel && !anodep->varp()->isParam() && !anodep->varp()->isGenVar()) return anodep; - } else { - if (AstNode* const refp = nodep->op1p()) - resultp = selectNonConstantRecurse(refp, inSel); - if (resultp) return resultp; - if (AstNode* const refp = nodep->op2p()) - resultp = selectNonConstantRecurse(refp, inSel); - if (resultp) return resultp; - if (AstNode* const refp = nodep->op3p()) - resultp = selectNonConstantRecurse(refp, inSel); - if (resultp) return resultp; - if (AstNode* const refp = nodep->op4p()) - resultp = selectNonConstantRecurse(refp, inSel); - } - return resultp; - } //---------------------------------------------------------------------- // METHODS - special type detection @@ -10139,3 +10111,28 @@ AstNode* V3Width::widthGenerateParamsEdit( // No WidthRemoveVisitor, as don't want to drop $signed etc inside gen blocks return nodep; } + +AstNode* V3Width::selectNonConstantRecurse(AstNode* nodep, bool inSel) { + // If node has a non-constant select, return that select + AstNode* resultp = nullptr; + if (AstNodeSel* const anodep = VN_CAST(nodep, NodeSel)) { + resultp = selectNonConstantRecurse(anodep->fromp(), inSel); + if (resultp) return resultp; + resultp = selectNonConstantRecurse(anodep->bitp(), true); + } else if (AstSel* const anodep = VN_CAST(nodep, Sel)) { + resultp = selectNonConstantRecurse(anodep->fromp(), inSel); + if (resultp) return resultp; + resultp = selectNonConstantRecurse(anodep->lsbp(), true); + } else if (AstNodeVarRef* const anodep = VN_CAST(nodep, NodeVarRef)) { + if (inSel && !anodep->varp()->isParam() && !anodep->varp()->isGenVar()) return anodep; + } else { + if (AstNode* const refp = nodep->op1p()) resultp = selectNonConstantRecurse(refp, inSel); + if (resultp) return resultp; + if (AstNode* const refp = nodep->op2p()) resultp = selectNonConstantRecurse(refp, inSel); + if (resultp) return resultp; + if (AstNode* const refp = nodep->op3p()) resultp = selectNonConstantRecurse(refp, inSel); + if (resultp) return resultp; + if (AstNode* const refp = nodep->op4p()) resultp = selectNonConstantRecurse(refp, inSel); + } + return resultp; +} diff --git a/src/V3Width.h b/src/V3Width.h index 7dd7471df..9634ff7a8 100644 --- a/src/V3Width.h +++ b/src/V3Width.h @@ -31,6 +31,7 @@ public: static void width(AstNetlist* nodep) VL_MT_DISABLED; static AstNode* widthParamsEdit(AstNode* nodep) VL_MT_DISABLED; static AstNode* widthGenerateParamsEdit(AstNode* nodep) VL_MT_DISABLED; + static AstNode* selectNonConstantRecurse(AstNode* nodep, bool inSel = false) VL_MT_DISABLED; // For use only in WidthVisitor // Replace AstSelBit, etc with AstSel/AstArraySel diff --git a/test_regress/t/t_lint_always_ff_multidriven_genif.py b/test_regress/t/t_lint_always_ff_multidriven_genif.py new file mode 100755 index 000000000..a5ea6627e --- /dev/null +++ b/test_regress/t/t_lint_always_ff_multidriven_genif.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of either the GNU Lesser General Public License Version 3 +# or the Perl Artistic License Version 2.0. +# SPDX-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('linter') + +test.lint() + +test.passes() diff --git a/test_regress/t/t_lint_always_ff_multidriven_genif.v b/test_regress/t/t_lint_always_ff_multidriven_genif.v new file mode 100644 index 000000000..9f35e96fd --- /dev/null +++ b/test_regress/t/t_lint_always_ff_multidriven_genif.v @@ -0,0 +1,28 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: CC0-1.0 + +module t(input wire clk); + + parameter int max_ports = 4; + + logic [max_ports-1:0][7:0] x; + + localparam logic [max_ports-1:0] use_a = 4'b0101; + + for (genvar port = 0; port < max_ports; port++) begin : g + if (use_a[port]) begin : g_a + always_ff @(posedge clk) x[max_ports - port - 1] <= "a"; + end else begin : g_b + always_ff @(posedge clk) x[max_ports - port - 1] <= "b"; + end + end + + initial begin + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule