From 02546791da15f90ad48b6195950c849046b68195 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Thu, 24 Sep 2026 23:41:24 +0200 Subject: [PATCH] Internals: Remove V3LinkDot::linkDotArrayed (#8495) Follow up to #8476 and #8493, now that elaboration fully expands instance arrays, linkDotArrayed's only remaining purpose was to name AstBegin blocks created after V3Width. Fix up earlier passes to name all Begin blocks they create, or avoid creating AstBegin in the first place if possible. One warning about assigning to a modport input of a generic interface port had to move into LinkLValue (previously this only used to be reported by linkDotArrayed, as the type/modportness only resolved in V3Param, and the lvalueness is subsequently only known in LinkLValue). Then remove linkDotArrayed and simplify V3LinkDot.cpp --- src/V3Assert.cpp | 15 ++-- src/V3AssertNfa.cpp | 16 ++-- src/V3AssertPre.cpp | 45 +++++----- src/V3AstNodeExpr.h | 3 + src/V3AstNodes.cpp | 2 + src/V3LinkDot.cpp | 60 +++++-------- src/V3LinkDot.h | 3 +- src/V3LinkLValue.cpp | 10 +++ src/V3Randomize.cpp | 102 ++++++++++++++-------- src/Verilator.cpp | 4 - test_regress/t/t_constraint_json_only.out | 2 +- test_regress/t/t_randcase_bad.out | 2 +- 12 files changed, 143 insertions(+), 121 deletions(-) diff --git a/src/V3Assert.cpp b/src/V3Assert.cpp index fe985972d..a04ac05fa 100644 --- a/src/V3Assert.cpp +++ b/src/V3Assert.cpp @@ -324,6 +324,7 @@ class AssertVisitor final : public VNVisitor { VDouble0 m_statLiftedCaseExprs; // Count of purified case expressions AstNodeFTask* m_ftaskp = nullptr; // Current function/task V3UniqueNames m_caseTempNames{"__VCase"}; + V3UniqueNames m_matchCountNames{"__VnfaRemainingMatchCount"}; // Match replay counter names // Maps from (expression, senTree) to the AstAlways that computes its delayed values. std::unordered_map, std::unordered_map, AstAlways*>> m_modExpr2Sen2DelayedAlwaysp; @@ -749,11 +750,11 @@ class AssertVisitor final : public VNVisitor { // reaches zero. matchCountp->unlinkFrBack(); AstVar* const remainingp = new AstVar{ - flp, VVarType::BLOCKTEMP, "__VnfaRemainingMatchCount", matchCountp->dtypep()}; - remainingp->lifetime(VLifetime::AUTOMATIC_EXPLICIT); - AstBegin* const replayp = new AstBegin{flp, "", remainingp, true}; - replayp->addStmtsp( - new AstAssign{flp, new AstVarRef{flp, remainingp, VAccess::WRITE}, matchCountp}); + flp, VVarType::MODULETEMP, m_matchCountNames.get(""), matchCountp->dtypep()}; + remainingp->lifetime(VLifetime::STATIC_EXPLICIT); + m_modp->addStmtsp(remainingp); + AstNode* const replaysp + = new AstAssign{flp, new AstVarRef{flp, remainingp, VAccess::WRITE}, matchCountp}; AstLoop* const loopp = new AstLoop{flp}; loopp->addStmtsp( new AstLoopTest{flp, loopp, new AstVarRef{flp, remainingp, VAccess::READ}}); @@ -763,8 +764,8 @@ class AssertVisitor final : public VNVisitor { new AstSub{flp, new AstVarRef{flp, remainingp, VAccess::READ}, new AstConst{flp, AstConst::WidthedValue{}, remainingp->dtypep()->width(), 1}}}); - replayp->addStmtsp(loopp); - passsp = replayp; + replaysp->addNext(loopp); + passsp = replaysp; } AstNode* bodysp = assertBody(nodep, propExprp, passsp, failsp); if (disablep) bodysp = new AstIf{flp, new AstLogNot{flp, disablep}, bodysp}; diff --git a/src/V3AssertNfa.cpp b/src/V3AssertNfa.cpp index fbc9c02da..6e3c19d1e 100644 --- a/src/V3AssertNfa.cpp +++ b/src/V3AssertNfa.cpp @@ -2894,6 +2894,7 @@ class AssertNfaVisitor final : public VNVisitor { V3UniqueNames m_propVarNames{"__Vpropvar"}; // Property-local variable names V3UniqueNames m_disableCntNames{"__VnfaDis"}; // Disable-iff counter names V3UniqueNames m_propTempNames{"__VnfaSampled"}; // Hoisted $sampled(propp) temps + V3UniqueNames m_failCountNames{"__VnfaRemainingFailCount"}; // Fail replay counter names std::set m_inliningProps; // Recursion guard for inlineNamedProperty template @@ -3276,13 +3277,12 @@ class AssertNfaVisitor final : public VNVisitor { // IEEE 1800-2023 16.12 requires one action-block evaluation per failed // thread. AstAssert handles the first, so replay the rest here. AstVar* const remainingFailCountVarp - = new AstVar{flp, VVarType::BLOCKTEMP, "__VnfaRemainingFailCount", + = new AstVar{flp, VVarType::MODULETEMP, m_failCountNames.get(""), m_modp->findBasicDType(VBasicDTypeKwd::UINT32)}; - remainingFailCountVarp->lifetime(VLifetime::AUTOMATIC_EXPLICIT); - AstBegin* const replayBlockp = new AstBegin{flp, "", remainingFailCountVarp, true}; - replayBlockp->addStmtsp( - new AstAssign{flp, new AstVarRef{flp, remainingFailCountVarp, VAccess::WRITE}, - threadFailCountp}); + remainingFailCountVarp->lifetime(VLifetime::STATIC_EXPLICIT); + m_modp->addStmtsp(remainingFailCountVarp); + AstNode* const replayStmtsp = new AstAssign{ + flp, new AstVarRef{flp, remainingFailCountVarp, VAccess::WRITE}, threadFailCountp}; AstLoop* const replayLoopp = new AstLoop{flp}; replayLoopp->addStmtsp(new AstLoopTest{ flp, replayLoopp, @@ -3296,9 +3296,9 @@ class AssertNfaVisitor final : public VNVisitor { replayLoopp->addStmtsp( new AstAssign{flp, new AstVarRef{flp, remainingFailCountVarp, VAccess::WRITE}, decrementedFailCountp}); - replayBlockp->addStmtsp(replayLoopp); + replayStmtsp->addNext(replayLoopp); m_modp->addStmtsp( - new AstAlways{flp, VAlwaysKwd::ALWAYS, threadFailReplaySenTreep, replayBlockp}); + new AstAlways{flp, VAlwaysKwd::ALWAYS, threadFailReplaySenTreep, replayStmtsp}); } } diff --git a/src/V3AssertPre.cpp b/src/V3AssertPre.cpp index 238c68593..baedef97f 100644 --- a/src/V3AssertPre.cpp +++ b/src/V3AssertPre.cpp @@ -68,6 +68,7 @@ private: V3UniqueNames m_gotoRepNames{"__VgotoRep"}; // Goto repetition counter name generator V3UniqueNames m_nonConsRepNames{"__VnonConsRep"}; // Nonconsecutive rep name generator V3UniqueNames m_disableCntNames{"__VdisableCnt"}; // Disable condition counter name generator + V3UniqueNames m_blockNames{"__VassertBlock"}; // Names of blocks with temporaries V3UniqueNames m_propVarNames{"__Vpropvar"}; // Property-local variable name generator V3UniqueNames m_activeNames{"__VassertsActive"}; // Active asserts map name generator bool m_inAssign = false; // True if in an AssignNode @@ -562,8 +563,7 @@ private: } if (m_disableSeqIfp && remainp) { AstIf* const disableSeqIfp = m_disableSeqIfp->cloneTree(false); - // Keep continuation statements in a proper statement-list container. - disableSeqIfp->addThensp(new AstBegin{flp, "", remainp, true}); + disableSeqIfp->addThensp(remainp); remainp = disableSeqIfp; } if (remainp) { @@ -878,7 +878,7 @@ private: = getProcessAssocArrayDelete(new AstVarRef{flp, activep, VAccess::WRITE}); // Main assertion block - AstBegin* const bodyp = new AstBegin{flp, "", nullptr, true}; + AstBegin* const bodyp = new AstBegin{flp, m_blockNames.get(""), nullptr, true}; bodyp->addStmtsp(incrementp); bodyp->addStmtsp(loopp); bodyp->addStmtsp(clausep); @@ -907,7 +907,7 @@ private: new AstConst{flp, 1}}}); // Final assertion block - AstBegin* const finalp = new AstBegin{flp, "", nullptr, true}; + AstBegin* const finalp = new AstBegin{flp, m_blockNames.get(""), nullptr, true}; finalp->addStmtsp(activeCountp); finalp->addStmtsp(initActiveCountp); finalp->addStmtsp(finalLoopp); @@ -1073,15 +1073,13 @@ private: windowp->addStmtsp(new AstLoopTest{ flp, windowp, new AstNot{flp, new AstVarRef{flp, doneVarp, VAccess::READ}}}); // if (expr) { fail; done = 1; } -- window closed, expr true again - AstBegin* const failBlockp = new AstBegin{flp, "", nullptr, true}; - failBlockp->addStmtsp(new AstPExprClause{flp, false}); - failBlockp->addStmtsp(setDone()); - windowp->addStmtsp(new AstIf{flp, exprp->cloneTreePure(false), failBlockp}); + AstNode* const failsp = new AstPExprClause{flp, false}; + failsp->addNext(setDone()); + windowp->addStmtsp(new AstIf{flp, exprp->cloneTreePure(false), failsp}); // if (rhs) { pass; done = 1; } -- consequent true at this !expr endpoint - AstBegin* const passBlockp = new AstBegin{flp, "", nullptr, true}; - passBlockp->addStmtsp(new AstPExprClause{flp, true}); - passBlockp->addStmtsp(setDone()); - windowp->addStmtsp(new AstIf{flp, rhsp, passBlockp}); + AstNode* const passsp = new AstPExprClause{flp, true}; + passsp->addNext(setDone()); + windowp->addStmtsp(new AstIf{flp, rhsp, passsp}); // @(clk) -- advance to next cycle in window windowp->addStmtsp( new AstEventControl{flp, new AstSenTree{flp, sensesp->cloneTree(false)}, nullptr}); @@ -1295,7 +1293,7 @@ private: // */ } AstBegin* const bodyp = pexprp->bodyp(); AstNode* const origStmtsp = bodyp->stmtsp()->unlinkFrBackWithNext(); - AstIf* const guardp = new AstIf{flp, condp, new AstBegin{flp, "", origStmtsp, true}}; + AstIf* const guardp = new AstIf{flp, condp, origStmtsp}; bodyp->addStmtsp(guardp); nodep->replaceWith(pexprp); // Don't iterate pexprp here -- it was already iterated when created @@ -1369,10 +1367,9 @@ private: loopp->addStmtsp(new AstLoopTest{ flp, loopp, new AstLogNot{flp, new AstVarRef{flp, donep, VAccess::READ}}}); { - AstBegin* const passp = new AstBegin{flp, "", nullptr, true}; - passp->addStmtsp(new AstPExprClause{flp}); - passp->addStmtsp(decrementVar); - passp->addStmtsp(setDone()); + AstNode* const passp = new AstPExprClause{flp}; + passp->addNext(decrementVar); + passp->addNext(setDone()); AstNodeExpr* passCondp = rhsp; if (nodep->isOverlapping()) { passCondp = new AstLogAnd{flp, lhsp->cloneTreePure(false), passCondp}; @@ -1380,10 +1377,9 @@ private: loopp->addStmtsp(new AstIf{flp, passCondp, passp}); } { - AstBegin* const failp = new AstBegin{flp, "", nullptr, true}; - failp->addStmtsp(new AstPExprClause{flp, false}); - failp->addStmtsp(decrementVar->cloneTree(false)); - failp->addStmtsp(setDone()); + AstNode* const failp = new AstPExprClause{flp, false}; + failp->addNext(decrementVar->cloneTree(false)); + failp->addNext(setDone()); loopp->addStmtsp(new AstIf{ flp, new AstLogAnd{flp, @@ -1397,7 +1393,7 @@ private: flp, new AstLogNot{flp, new AstVarRef{flp, donep, VAccess::READ}}, delayp}); // Main assertion block - AstBegin* const bodyp = new AstBegin{flp, "", nullptr, true}; + AstBegin* const bodyp = new AstBegin{flp, m_blockNames.get(""), nullptr, true}; bodyp->addStmtsp(donep); bodyp->addStmtsp(new AstAssign{flp, new AstVarRef{flp, donep, VAccess::WRITE}, new AstConst{flp, AstConst::BitFalse{}}}); @@ -1429,7 +1425,7 @@ private: new AstConst{flp, 1}}}); // Final assertion block - AstBegin* const finalp = new AstBegin{flp, "", nullptr, true}; + AstBegin* const finalp = new AstBegin{flp, m_blockNames.get(""), nullptr, true}; finalp->addStmtsp(activeCountp); finalp->addStmtsp(initActiveCountp); finalp->addStmtsp(finalLoopp); @@ -1460,7 +1456,7 @@ private: AstNodeExpr* const passCondp = nodep->isOverlapping() ? new AstLogAnd{flp, lhsp->cloneTreePure(false), rhsCopyp} : rhsCopyp; - AstBegin* const beginp = new AstBegin{flp, "", loopp, true}; + AstBegin* const beginp = new AstBegin{flp, m_blockNames.get(""), loopp, true}; beginp->addStmtsp( new AstIf{flp, passCondp, new AstPExprClause{flp}, new AstPExprClause{flp, false}}); @@ -1578,6 +1574,7 @@ private: = new AstAssign{flp, new AstVarRef{flp, initialCntp, VAccess::WRITE}, readCntRefp->cloneTree(false)}; // Prepend to the sequence body to keep statement list structure valid. + UASSERT_OBJ(!bodyp->name().empty(), bodyp, "Sequence body should be named"); AstNode* const origStmtsp = bodyp->stmtsp()->unlinkFrBackWithNext(); bodyp->addStmtsp(initialCntp); initialCntp->addNextHere(assignp); diff --git a/src/V3AstNodeExpr.h b/src/V3AstNodeExpr.h index 70f5c0f72..209afa425 100644 --- a/src/V3AstNodeExpr.h +++ b/src/V3AstNodeExpr.h @@ -6464,6 +6464,7 @@ class AstVarXRef final : public AstNodeVarRef { string m_dotted; // Dotted part of scope the name()'ed reference is under or "" string m_inlinedDots; // Dotted hierarchy flattened out bool m_containsGenBlock = false; // Contains gen block reference + bool m_readOnlyModport = false; // Linked via an input-only modport, until V3LinkLValue public: AstVarXRef(FileLine* fl, const string& name, const string& dotted, const VAccess& access) : ASTGEN_SUPER_VarXRef(fl, nullptr, access) @@ -6481,6 +6482,8 @@ public: void inlinedDots(const string& flag) { m_inlinedDots = flag; } bool containsGenBlock() const { return m_containsGenBlock; } void containsGenBlock(const bool flag) { m_containsGenBlock = flag; } + bool readOnlyModport() const { return m_readOnlyModport; } + void readOnlyModport(const bool flag) { m_readOnlyModport = flag; } string emitVerilog() override { V3ERROR_NA_RETURN(""); } string emitC() override { V3ERROR_NA_RETURN(""); } bool cleanOut() const override { return true; } diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 671ffabb9..cfa3441de 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -4263,6 +4263,7 @@ bool AstVarScope::sameNode(const AstNode* samep) const { void AstVarXRef::dump(std::ostream& str) const { Super::dump(str); if (containsGenBlock()) str << " [GENBLK]"; + if (readOnlyModport()) str << " [ROMODPORT]"; str << ".=" << dotted() << " "; if (inlinedDots() != "") str << " inline.=" << inlinedDots() << " - "; if (varScopep()) { @@ -4275,6 +4276,7 @@ void AstVarXRef::dump(std::ostream& str) const { } void AstVarXRef::dumpJson(std::ostream& str) const { dumpJsonBoolFuncIf(str, containsGenBlock); + dumpJsonBoolFuncIf(str, readOnlyModport); dumpJsonStrFunc(str, dotted); dumpJsonStrFunc(str, inlinedDots); dumpJsonGen(str); diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index a71b9a5bc..57507f074 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -293,7 +293,6 @@ public: int stepNumber() const { return static_cast(m_step); } bool forPrimary() const { return m_step == LDS_PRIMARY; } bool forParamed() const { return m_step == LDS_PARAMED; } - bool forPrearray() const { return m_step == LDS_PARAMED || m_step == LDS_PRIMARY; } bool forScopeCreation() const { return m_step == LDS_SCOPED; } // METHODS @@ -804,7 +803,7 @@ public: baddot = ident; // So user can see where they botched it okSymp = lookupSymp; string altIdent; - if (forPrearray()) { + if (!forScopeCreation()) { // GENFOR Begin is foo__BRA__##__KET__ after we've genloop unrolled, // but presently should be just "foo". // Likewise cell foo__[array] before we've expanded arrays is just foo. @@ -841,7 +840,7 @@ public: else if (ident == "$root") { lookupSymp = rootEntp(); // We've added the '$root' module, now everything else is one lower - if (!forPrearray()) { + if (forScopeCreation()) { lookupSymp = lookupSymp->findIdFlat(ident); UASSERT(lookupSymp, "Cannot find $root module under netlist"); } @@ -1230,7 +1229,7 @@ class LinkDotFindVisitor final : public VNVisitor { UINFO(8, "Top Module: " << modp); m_scope = "TOP"; - if (m_statep->forPrearray() && v3Global.opt.topIfacesSupported()) { + if (!m_statep->forScopeCreation() && v3Global.opt.topIfacesSupported()) { for (AstNode* subnodep = modp->stmtsp(); subnodep; subnodep = subnodep->nextp()) { if (AstVar* const varp = VN_CAST(subnodep, Var)) { if (varp->isIfaceRef()) { @@ -1286,8 +1285,8 @@ class LinkDotFindVisitor final : public VNVisitor { void visit(AstTypeTable*) override {} // FindVisitor:: void visit(AstConstPool*) override {} // FindVisitor:: void visit(AstIfaceRefDType* nodep) override { // FindVisitor:: - if ((m_statep->forPrimary() || m_statep->forParamed()) && nodep->isVirtual() - && nodep->ifacep() && !nodep->ifacep()->user3()) { + if (!m_statep->forScopeCreation() && nodep->isVirtual() && nodep->ifacep() + && !nodep->ifacep()->user3()) { m_virtIfaces.push_back(nodep->ifacep()); nodep->ifacep()->user3(true); } @@ -1301,7 +1300,7 @@ class LinkDotFindVisitor final : public VNVisitor { // Packages will be under top after the initial phases, but until then // need separate handling const bool standalonePkg - = !m_modSymp && (m_statep->forPrearray() && VN_IS(nodep, Package)); + = !m_modSymp && !m_statep->forScopeCreation() && VN_IS(nodep, Package); const bool doit = (m_modSymp || standalonePkg); VL_RESTORER_COPY(m_scope); VL_RESTORER(m_classOrPackagep); @@ -1973,7 +1972,7 @@ class LinkDotFindVisitor final : public VNVisitor { // dtype comes from the other side. VL_DO_DANGLING(varDtp->unlinkFrBack()->deleteTree(), varDtp); findvarp->childDTypep(otherDtp->unlinkFrBack()); - } else if (m_statep->forPrearray() && otherDtp && varDtp + } else if (!m_statep->forScopeCreation() && otherDtp && varDtp && !(VN_IS(otherDtp, BasicDType) && VN_AS(otherDtp, BasicDType)->implicit())) { // otherDtp and varDtp both non-nullptr and neither are implicit @@ -2481,7 +2480,7 @@ class LinkDotParamVisitor final : public VNVisitor { UINFO(5, " " << nodep); if ((nodep->dead() || !nodep->user4()) && !nodep->hierParams()) { UINFO(4, "Mark dead module " << nodep); - UASSERT_OBJ(m_statep->forPrearray(), nodep, + UASSERT_OBJ(!m_statep->forScopeCreation(), nodep, "Dead module persisted past where should have removed"); // Don't remove now, because we may have a tree of // parameterized modules with VARXREFs into the deleted module @@ -4249,8 +4248,7 @@ class LinkDotResolveVisitor final : public VNVisitor { // m_ds.m_dotSymp is symbol table relative to "."'s above now UASSERT_OBJ(m_ds.m_dotSymp, nodep, "nullptr lookup symbol table"); // Generally resolved during Primary, but might be at param time under AstUnlinkedRef - UASSERT_OBJ(m_statep->forPrimary() || m_statep->forPrearray(), nodep, - "ParseRefs should no longer exist"); + UASSERT_OBJ(!m_statep->forScopeCreation(), nodep, "ParseRefs should no longer exist"); const DotStates lastStates = m_ds; const bool start = (m_ds.m_dotPos == DP_NONE); // Save, as m_dotp will be changed bool first = start || m_ds.m_dotPos == DP_FIRST; @@ -4569,6 +4567,13 @@ class LinkDotResolveVisitor final : public VNVisitor { = new AstVarXRef{nodep->fileline(), nodep->name(), m_ds.m_dotText, VAccess::READ}; // lvalue'ness computed later refp->varp(varp); + // Not linked again after V3LinkLValue, so that reports writing it + if (const AstModportVarRef* const mvarp + = VN_CAST(foundp->nodep(), ModportVarRef)) { + if (m_statep->forParamed() && mvarp->direction().isReadOnly()) { + refp->readOnlyModport(true); + } + } refp->containsGenBlock(m_ds.m_genBlk); if (varp->attrSplitVar()) { refp->v3warn( @@ -5080,20 +5085,6 @@ class LinkDotResolveVisitor final : public VNVisitor { << okSymp->cellErrorScopes(nodep)); return; } - // V3Inst may have expanded arrays of interfaces to AstVarXRef's even though - // they are in the same module; convert to normal VarRefs (but not if dotted) - if (!m_statep->forPrearray() && !m_statep->forScopeCreation() - && nodep->dotted().empty()) { - if (const AstIfaceRefDType* const ifaceDtp - = VN_CAST(nodep->dtypep(), IfaceRefDType)) { - if (!ifaceDtp->isVirtual()) { - AstVarRef* const newrefp - = new AstVarRef{nodep->fileline(), nodep->varp(), nodep->access()}; - nodep->replaceWith(newrefp); - VL_DO_DANGLING(pushDeletep(nodep), nodep); - } - } - } } else { VSymEnt* const foundp = m_statep->findSymPrefixed(dotSymp, nodep->name(), baddot, true); @@ -5446,7 +5437,7 @@ class LinkDotResolveVisitor final : public VNVisitor { << foundp->nodep()->typeName() << " but expected a task/function"); } - } else if (VN_IS(nodep, New) && m_statep->forPrearray()) { + } else if (VN_IS(nodep, New) && !m_statep->forScopeCreation()) { // Resolved in V3Width } else if ((nodep->name() == "pre_randomize" || nodep->name() == "post_randomize") && VN_IS(dotSymp->nodep(), Class)) { @@ -6512,19 +6503,16 @@ void V3LinkDot::linkDotGuts(AstNetlist* rootp, VLinkDotStep step) { { LinkDotFindIfaceVisitor{rootp, &state}; } dumpSubstep("prelinkdot-findiface"); - if (step == LDS_PRIMARY || step == LDS_PARAMED) { - // Initial link stage, resolve parameters and interfaces - { LinkDotParamVisitor{rootp, &state}; } - dumpSubstep("prelinkdot-param"); - } else if (step == LDS_ARRAYED) { - } else if (step == LDS_SCOPED) { + if (step == LDS_SCOPED) { // Well after the initial link when we're ready to operate on the flat design, // process AstScope's. This needs to be separate pass after whole hierarchy graph created. { LinkDotScopeVisitor{rootp, &state}; } v3Global.assertScoped(true); dumpSubstep("prelinkdot-scoped"); } else { - v3fatalSrc("Bad case"); + // Initial link stage, resolve parameters and interfaces + { LinkDotParamVisitor{rootp, &state}; } + dumpSubstep("prelinkdot-param"); } state.dumpSelf("prelinkdot"); state.computeIfaceModSyms(); @@ -6547,12 +6535,6 @@ void V3LinkDot::linkDotParamed(AstNetlist* nodep) { V3Global::dumpCheckGlobalTree("linkdotparam", 0, dumpTreeEitherLevel() >= 3); } -void V3LinkDot::linkDotArrayed(AstNetlist* nodep) { - UINFO(2, __FUNCTION__ << ":"); - linkDotGuts(nodep, LDS_ARRAYED); - V3Global::dumpCheckGlobalTree("linkdot", 0, dumpTreeEitherLevel() >= 6); -} - void V3LinkDot::linkDotScope(AstNetlist* nodep) { UINFO(2, __FUNCTION__ << ":"); linkDotGuts(nodep, LDS_SCOPED); diff --git a/src/V3LinkDot.h b/src/V3LinkDot.h index 257d522c3..040913dac 100644 --- a/src/V3LinkDot.h +++ b/src/V3LinkDot.h @@ -25,7 +25,7 @@ //============================================================================ -enum VLinkDotStep : uint8_t { LDS_PRIMARY, LDS_PARAMED, LDS_ARRAYED, LDS_SCOPED }; +enum VLinkDotStep : uint8_t { LDS_PRIMARY, LDS_PARAMED, LDS_SCOPED }; class V3LinkDot final { static void dumpSubstep(const string& name) VL_MT_DISABLED; @@ -34,7 +34,6 @@ class V3LinkDot final { public: static void linkDotPrimary(AstNetlist* nodep) VL_MT_DISABLED; static void linkDotParamed(AstNetlist* nodep) VL_MT_DISABLED; - static void linkDotArrayed(AstNetlist* nodep) VL_MT_DISABLED; static void linkDotScope(AstNetlist* nodep) VL_MT_DISABLED; }; diff --git a/src/V3LinkLValue.cpp b/src/V3LinkLValue.cpp index a14486fa3..d33f6e11d 100644 --- a/src/V3LinkLValue.cpp +++ b/src/V3LinkLValue.cpp @@ -48,6 +48,16 @@ class LinkLValueVisitor final : public VNVisitor { // VarRef: LValue its reference if (m_setIfRand && !(nodep->varp() && nodep->varp()->isRand())) return; if (m_setRefLvalue != VAccess::NOCHANGE) nodep->access(m_setRefLvalue); + // Linked via an input-only modport by V3LinkDot, before it was known if written + if (AstVarXRef* const xrefp = VN_CAST(nodep, VarXRef)) { + if (xrefp->readOnlyModport()) { + if (nodep->access().isWriteOrRW()) { + nodep->v3error( + "Attempt to drive input-only modport: " << nodep->prettyNameQ()); + } + xrefp->readOnlyModport(false); + } + } if (nodep->varp() && nodep->access().isWriteOrRW()) { if (nodep->varp()->isParam()) { // All parameters that did get constified happened before now diff --git a/src/V3Randomize.cpp b/src/V3Randomize.cpp index 0580a937b..54df76dd2 100644 --- a/src/V3Randomize.cpp +++ b/src/V3Randomize.cpp @@ -787,6 +787,7 @@ class ConstraintExprVisitor final : public VNVisitor { AstNodeExpr* m_conditionp = nullptr; // Condition under which current expression is defined // (nullptr == always defined) uint32_t* m_uniqueConstraintId = nullptr; // Current ID of unique call + V3UniqueNames& m_uniqueNames; // Unique names of temporaries, and of blocks holding them AstNode* m_firstExpressionInsideIndexp = nullptr; class NestedAccessPath final { @@ -1487,14 +1488,13 @@ class ConstraintExprVisitor final : public VNVisitor { AstClass* const elemClassp = elemClassRefDtp->classp(); AstNodeModule* const varClassp = VN_AS(varp->user2p(), NodeModule); - AstVar* const iterVarp - = new AstVar{fl, VVarType::BLOCKTEMP, "__Vi", varp->findUInt32DType()}; + AstVar* const iterVarp = new AstVar{fl, VVarType::BLOCKTEMP, m_uniqueNames.get("__Vi"), + varp->findUInt32DType()}; iterVarp->funcLocal(true); iterVarp->lifetime(VLifetime::AUTOMATIC_EXPLICIT); - AstNode* const stmtsp = iterVarp; - stmtsp->addNext( - new AstAssign{fl, new AstVarRef{fl, iterVarp, VAccess::WRITE}, new AstConst{fl, 0}}); + AstNode* const stmtsp + = new AstAssign{fl, new AstVarRef{fl, iterVarp, VAccess::WRITE}, new AstConst{fl, 0}}; AstNodeExpr* sizep; if (isUnpackedClassRefArray) { @@ -1540,14 +1540,19 @@ class ConstraintExprVisitor final : public VNVisitor { fl, new AstVarRef{fl, iterVarp, VAccess::WRITE}, new AstAdd{fl, new AstConst{fl, 1}, new AstVarRef{fl, iterVarp, VAccess::READ}}}); - AstBegin* const beginp = new AstBegin{fl, "", stmtsp, true}; varp->user3(true); AstNodeFTask* initTaskp = m_inlineInitTaskp; if (!initTaskp) { initTaskp = VN_AS(m_memberMap.findMember(varClassp, "new"), NodeFTask); UASSERT_OBJ(initTaskp, varClassp, "No new() in class"); } - initTaskp->addStmtsp(beginp); + // At the beginning, so declared before referenced + if (AstNode* const initStmtsp = initTaskp->stmtsp()) { + initStmtsp->addHereThisAsNext(iterVarp); + } else { + initTaskp->addStmtsp(iterVarp); + } + initTaskp->addStmtsp(stmtsp); } void createSolverVarHandle(AstVar* const varp, const bool structSelOrCMeth, @@ -2559,16 +2564,18 @@ class ConstraintExprVisitor final : public VNVisitor { AstCExpr* const cexprp = new AstCExpr{fl}; cexprp->dtypeSetString(); cexprp->add("([&]{\nstd::string ret;\n"); - cexprp->add(new AstBegin{ - fl, "", new AstForeach{fl, nodep->headerp()->unlinkFrBack(), bodyp}, true}); + cexprp->add(new AstBegin{fl, m_uniqueNames.get("__VrandBlock"), + new AstForeach{fl, nodep->headerp()->unlinkFrBack(), bodyp}, + true}); cexprp->add("return ret.empty() ? \"#b1\" : \"(bvand\" + ret + \")\";\n})()"); nodep->replaceWith(new AstSFormatF{fl, "%s", false, cexprp}); } else { iterateAndNextNull(nodep->bodyp()); AstNode* const bodyp = prependDistPreamble(nodep, nodep->bodyp()->unlinkFrBackWithNext()); - nodep->replaceWith(new AstBegin{ - fl, "", new AstForeach{fl, nodep->headerp()->unlinkFrBack(), bodyp}, true}); + nodep->replaceWith( + new AstBegin{fl, m_uniqueNames.get("__VrandBlock"), + new AstForeach{fl, nodep->headerp()->unlinkFrBack(), bodyp}, true}); } UASSERT_OBJ(!nodep->user3p(), nodep, "Dist bucket preamble not injected into foreach"); VL_DO_DANGLING(nodep->deleteTree(), nodep); @@ -2915,7 +2922,8 @@ class ConstraintExprVisitor final : public VNVisitor { AstCExpr* const cexprp = new AstCExpr{fl}; cexprp->dtypeSetString(); cexprp->add("([&]{\nstd::string ret;\n"); - cexprp->add(new AstBegin{fl, "", new AstForeach{fl, headerp, cstmtp}, true}); + cexprp->add(new AstBegin{fl, m_uniqueNames.get("__VrandBlock"), + new AstForeach{fl, headerp, cstmtp}, true}); cexprp->add("return ret.empty() ? \"#b0\" : \"(bvor\" + ret + \")\";\n})()"); nodep->replaceWith(new AstSFormatF{fl, "%s", false, cexprp}); VL_DO_DANGLING(nodep->deleteTree(), nodep); @@ -3144,7 +3152,8 @@ class ConstraintExprVisitor final : public VNVisitor { AstCExpr* const cexprp = new AstCExpr{fl}; cexprp->dtypeSetString(); cexprp->add("([&]{\nstd::string ret = \"" + identity + "\";\n"); - cexprp->add(new AstBegin{fl, "", new AstForeach{fl, headerp, cstmtp}, true}); + cexprp->add(new AstBegin{fl, m_uniqueNames.get("__VrandBlock"), + new AstForeach{fl, headerp, cstmtp}, true}); cexprp->add("return ret;\n})()"); nodep->replaceWith(new AstSFormatF{fl, "%s", false, cexprp}); } else { @@ -3152,7 +3161,8 @@ class ConstraintExprVisitor final : public VNVisitor { AstCExpr* const cexprp = new AstCExpr{fl}; cexprp->dtypeSetString(); cexprp->add("([&]{\nstd::string ret;\n"); - cexprp->add(new AstBegin{fl, "", new AstForeach{fl, headerp, cstmtp}, true}); + cexprp->add(new AstBegin{fl, m_uniqueNames.get("__VrandBlock"), + new AstForeach{fl, headerp, cstmtp}, true}); cexprp->add("return ret.empty() ? \"" + identity + "\" : \"(" + smtOp + "\" + ret + \")\";\n})()"); nodep->replaceWith(new AstSFormatF{fl, "%s", false, cexprp}); @@ -3342,7 +3352,7 @@ public: explicit ConstraintExprVisitor(AstClass* classp, VMemberMap& memberMap, AstNode* nodep, AstNodeFTask* inlineInitTaskp, AstVar* genp, AstVar* randModeVarp, std::set& writtenVars, - uint32_t* uniqueConstraintId, + uint32_t* uniqueConstraintId, V3UniqueNames& uniqueNames, AstNodeFTask* memberselInitTaskp = nullptr, std::set* sizeConstrainedArraysp = nullptr) : m_classp{classp} @@ -3353,7 +3363,8 @@ public: , m_memberMap{memberMap} , m_writtenVars{writtenVars} , m_sizeConstrainedArraysp{sizeConstrainedArraysp} - , m_uniqueConstraintId{uniqueConstraintId} { + , m_uniqueConstraintId{uniqueConstraintId} + , m_uniqueNames{uniqueNames} { // Pre-pass before SMT lowering: extract conditional disable-soft // directives as runtime AstIf statements and append them to the // constraint-items chain so they reach the setup task body. The SMT @@ -3657,10 +3668,11 @@ class RandomizeVisitor final : public VNVisitor { V3UniqueNames m_modeUniqueNames{"__Vmode"}; // For generating unique rand/constraint // mode state var names V3UniqueNames m_inlineUniqueStdName{"__VStdrand"}; + V3UniqueNames m_uniqueNames; // Unique names of temporaries, and of blocks holding them VMemberMap m_memberMap; // Member names cached for fast lookup AstNodeModule* m_modp = nullptr; // Current module std::unordered_map m_stdMap; // Map from module/class to AST Var - const AstNodeFTask* m_ftaskp = nullptr; // Current function/task + AstNodeFTask* m_ftaskp = nullptr; // Current function/task AstNodeStmt* m_stmtp = nullptr; // Current statement AstDynArrayDType* m_dynarrayDtp = nullptr; // Dynamic array type (for rand mode) size_t m_enumValueTabCount = 0; // Number of tables with enum values created @@ -4107,8 +4119,8 @@ class RandomizeVisitor final : public VNVisitor { UASSERT_OBJ(newp, classp, "No new() in class"); newp->addStmtsp(ifp); } - static AstNode* makeModeSetLoop(FileLine* const fl, AstNodeExpr* const lhsp, - AstNodeExpr* const rhsp, bool inTask) { + AstNode* makeModeSetLoop(FileLine* const fl, AstNodeExpr* const lhsp, AstNodeExpr* const rhsp, + bool inTask) { AstVar* const iterVarp = new AstVar{fl, VVarType::BLOCKTEMP, "i", lhsp->findUInt32DType()}; iterVarp->funcLocal(inTask); iterVarp->lifetime(VLifetime::AUTOMATIC_EXPLICIT); @@ -4130,7 +4142,24 @@ class RandomizeVisitor final : public VNVisitor { loopp->addStmtsp(new AstAssign{ fl, new AstVarRef{fl, iterVarp, VAccess::WRITE}, new AstAdd{fl, new AstConst{fl, 1}, new AstVarRef{fl, iterVarp, VAccess::READ}}}); - return new AstBegin{fl, "", stmtsp, true}; + return new AstBegin{fl, m_uniqueNames.get("__VrandBlock"), stmtsp, true}; + } + void addTempVarsp(AstVar* varsp) { + for (AstVar* varp = varsp; varp; varp = VN_AS(varp->nextp(), Var)) { + if (m_ftaskp) { + varp->funcLocal(true); + } else { + varp->varType(VVarType::MODULETEMP); + varp->lifetime(VLifetime::STATIC_EXPLICIT); + } + } + if (!m_ftaskp) { + m_modp->addStmtsp(varsp); + } else if (AstNode* const stmtsp = m_ftaskp->stmtsp()) { + stmtsp->addHereThisAsNext(varsp); + } else { + m_ftaskp->addStmtsp(varsp); + } } AstNodeStmt* wrapIfRandMode(AstClass* classp, AstVar* const varp, AstNodeStmt* stmtp) { const RandomizeMode rmode = {.asUQuad = varp->user1()}; @@ -4828,7 +4857,8 @@ class RandomizeVisitor final : public VNVisitor { = new AstVarRef{memberVarp->fileline(), classp, memberVarp, VAccess::WRITE}; AstNodeStmt* const stmtp = newRandStmtsp(fl, refp, randcVarp, basicFvarp); if (!refp->backp()) VL_DO_DANGLING(refp->deleteTree(), refp); - basicRandomizep->addStmtsp(new AstBegin{fl, "", stmtp, false}); + basicRandomizep->addStmtsp( + new AstBegin{fl, m_uniqueNames.get("__VrandBlock"), stmtp, false}); } }); } @@ -5020,12 +5050,13 @@ class RandomizeVisitor final : public VNVisitor { UASSERT_OBJ(storeStmtsp && setStmtsp && restoreStmtsp, nodep, "Should have stmts"); VNRelinker relinker; m_stmtp->unlinkFrBack(&relinker); - AstNode* const stmtsp = tmpVarps; - stmtsp->addNext(storeStmtsp); + AstNode* const stmtsp = storeStmtsp; stmtsp->addNext(setStmtsp); stmtsp->addNext(m_stmtp); stmtsp->addNext(restoreStmtsp); - relinker.relink(new AstBegin{nodep->fileline(), "", stmtsp, true}); + relinker.relink(stmtsp); + // After relinking, so the function body is complete + addTempVarsp(tmpVarps); } } @@ -5746,8 +5777,9 @@ class RandomizeVisitor final : public VNVisitor { } std::set& sizeArrays = m_sizeConstrainedArrays[classp]; ConstraintExprVisitor{ - classp, m_memberMap, constrp->itemsp(), nullptr, genp, - randModeVarp, m_writtenVars, &m_uniqueConstraintId, randomizep, &sizeArrays}; + classp, m_memberMap, constrp->itemsp(), nullptr, + genp, randModeVarp, m_writtenVars, &m_uniqueConstraintId, + m_uniqueNames, randomizep, &sizeArrays}; if (constrp->itemsp()) { taskp->addStmtsp(wrapIfConstraintMode( nodep, constrp, constrp->itemsp()->unlinkFrBackWithNext())); @@ -6035,7 +6067,7 @@ class RandomizeVisitor final : public VNVisitor { AstVar* const randVarp = new AstVar{fl, VVarType::BLOCKTEMP, name, sumDTypep}; randVarp->noSubst(true); randVarp->lifetime(VLifetime::AUTOMATIC_EXPLICIT); - if (m_ftaskp) randVarp->funcLocal(true); + addTempVarsp(randVarp); AstNodeExpr* sump = new AstConst{fl, AstConst::WidthedValue{}, 64, 0}; AstNodeIf* const firstIfsp = new AstIf{fl, new AstConst{fl, AstConst::BitFalse{}}, nullptr, nullptr}; @@ -6063,13 +6095,13 @@ class RandomizeVisitor final : public VNVisitor { dispp->fmtp()->timeunit(m_modp->timeunit()); ifsp->addElsesp(dispp); - AstNode* const newp = randVarp; AstNodeExpr* randp = new AstRand{fl, nullptr, false}; randp->dtypeSetUInt64(); AstVarRef* const randVarRefp = new AstVarRef{fl, randVarp, VAccess::WRITE}; - newp->addNext(new AstAssign{fl, randVarRefp, - new AstAdd{fl, new AstConst{fl, AstConst::Unsized64{}, 1}, - new AstModDiv{fl, randp, sump}}}); + AstNode* const newp + = new AstAssign{fl, randVarRefp, + new AstAdd{fl, new AstConst{fl, AstConst::Unsized64{}, 1}, + new AstModDiv{fl, randp, sump}}}; newp->addNext(firstIfsp); if (debug() >= 9) newp->dumpTreeAndNext(cout, "- rcnew: "); nodep->replaceWith(newp); @@ -6230,7 +6262,7 @@ class RandomizeVisitor final : public VNVisitor { expandUniqueElementList(capturedTreep); ConstraintExprVisitor{ nullptr, m_memberMap, capturedTreep, randomizeFuncp, stdrand, - nullptr, m_writtenVars, &m_uniqueConstraintId, nullptr}; + nullptr, m_writtenVars, &m_uniqueConstraintId, m_uniqueNames, nullptr}; } AstCExpr* const solverCallp = new AstCExpr{fl}; solverCallp->dtypeSetBit(); @@ -6446,9 +6478,9 @@ class RandomizeVisitor final : public VNVisitor { { expandUniqueElementList(capturedTreep); - ConstraintExprVisitor{classp, m_memberMap, capturedTreep, randomizeFuncp, - localGenp, randModeVarp, m_writtenVars, &m_uniqueConstraintId, - nullptr}; + ConstraintExprVisitor{ + classp, m_memberMap, capturedTreep, randomizeFuncp, localGenp, + randModeVarp, m_writtenVars, &m_uniqueConstraintId, m_uniqueNames, nullptr}; } // Call the solver and set return value diff --git a/src/Verilator.cpp b/src/Verilator.cpp index 8cb60b194..ee6f82889 100644 --- a/src/Verilator.cpp +++ b/src/Verilator.cpp @@ -284,10 +284,6 @@ static void process() { // should be after constifyAllLint() which flattens to 1D bit vector V3SplitVar::splitVariable(v3Global.rootp()); - // Nothing to relink, but LinkDot names blocks created since V3Width, - // e.g. by V3AssertNfa. TODO: get rid of this - V3LinkDot::linkDotArrayed(v3Global.rootp()); - if (v3Global.opt.timing().isSetTrue()) { // Generate classes and tasks required to maintain proper lifetimes for references // in forks diff --git a/test_regress/t/t_constraint_json_only.out b/test_regress/t/t_constraint_json_only.out index 87e25c434..8d266c6b7 100644 --- a/test_regress/t/t_constraint_json_only.out +++ b/test_regress/t/t_constraint_json_only.out @@ -500,7 +500,7 @@ ]}, {"type":"TASK","name":"arr_uniq_setup_constraint","addr":"(KJ)","loc":"d,7:1,7:6","method":true,"lifetime":"NONE","cname":"arr_uniq_setup_constraint", "stmtsp": [ - {"type":"BEGIN","addr":"(LJ)","loc":"d,42:5,42:12","implied":true,"unnamed":true, + {"type":"BEGIN","name":"__VrandBlock__0","addr":"(LJ)","loc":"d,42:5,42:12","implied":true, "stmtsp": [ {"type":"FOREACH","addr":"(MJ)","loc":"d,42:5,42:12", "headerp": [ diff --git a/test_regress/t/t_randcase_bad.out b/test_regress/t/t_randcase_bad.out index cf24d9fcb..e2fed7e86 100644 --- a/test_regress/t/t_randcase_bad.out +++ b/test_regress/t/t_randcase_bad.out @@ -1,2 +1,2 @@ -[0] %Error: t_randcase_bad.v:12: Assertion failed in top.t.unnamedblk2_1: All randcase items had 0 weights (IEEE 1800-2023 18.16) +[0] %Error: t_randcase_bad.v:12: Assertion failed in top.t: All randcase items had 0 weights (IEEE 1800-2023 18.16) *-* All Finished *-*