diff --git a/src/V3LinkCells.cpp b/src/V3LinkCells.cpp index 3542be191..f1cf0deb3 100644 --- a/src/V3LinkCells.cpp +++ b/src/V3LinkCells.cpp @@ -131,11 +131,7 @@ class LinkCellsVisitor final : public VNVisitor { AstNodeModule* findModuleSym(const string& modName) { const VSymEnt* const foundp = m_mods.rootp()->findIdFallback(modName); - if (!foundp) { - return nullptr; - } else { - return VN_AS(foundp->nodep(), NodeModule); - } + return foundp ? VN_AS(foundp->nodep(), NodeModule) : nullptr; } AstNodeModule* resolveModule(AstNode* nodep, const string& modName) { @@ -534,8 +530,14 @@ class LinkCellsVisitor final : public VNVisitor { if (pinp->name() == "") pinp->name("__paramNumber" + cvtToStr(pinp->pinNum())); } if (m_varp) { // Parser didn't know what was interface, resolve now - const AstNodeModule* const varModp = findModuleSym(nodep->name()); - if (VN_IS(varModp, Iface)) m_varp->setIfaceRef(); + AstNodeModule* const varModp = findModuleSym(nodep->name()); + if (AstIface* const ifacep = VN_CAST(varModp, Iface)) { + // Might be an interface, but might also not really be due to interface being + // hidden by another declaration. Assume it is relevant and order as-if. + // This is safe because an interface cannot instantiate a module, so false + // module->interface edges are harmless. + newEdge(vertex(m_modp), vertex(ifacep), 1, false); + } } } void visit(AstClassOrPackageRef* nodep) override { diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index ba749b975..683bc3a50 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -154,6 +154,7 @@ private: // MEMBERS VSymGraph m_syms; // Symbol table by hierarchy + VSymGraph m_mods; // Symbol table of all module names VSymEnt* m_dunitEntp = nullptr; // $unit entry std::multimap m_nameScopeSymMap; // Map of scope referenced by non-pretty textual name @@ -209,10 +210,12 @@ public: // CONSTRUCTORS LinkDotState(AstNetlist* rootp, VLinkDotStep step) : m_syms{rootp} + , m_mods{rootp} , m_step(step) { UINFO(4, __FUNCTION__ << ": " << endl); s_errorThisp = this; V3Error::errorExitCb(preErrorDumpHandler); // If get error, dump self + readModNames(); } ~LinkDotState() { V3Error::errorExitCb(nullptr); @@ -262,6 +265,18 @@ public: } } + AstNodeModule* findModuleSym(const string& modName) { + const VSymEnt* const foundp = m_mods.rootp()->findIdFallback(modName); + return foundp ? VN_AS(foundp->nodep(), NodeModule) : nullptr; + } + void readModNames() { + // Look at all modules, and store pointers to all module names + for (AstNodeModule *nextp, *nodep = v3Global.rootp()->modulesp(); nodep; nodep = nextp) { + nextp = VN_AS(nodep->nextp(), NodeModule); + m_mods.rootp()->insert(nodep->name(), new VSymEnt{&m_mods, nodep}); + } + } + VSymEnt* rootEntp() const { return m_syms.rootp(); } VSymEnt* dunitEntp() const { return m_dunitEntp; } void checkDuplicate(VSymEnt* lookupSymp, AstNode* nodep, const string& name) { @@ -427,7 +442,7 @@ public: abovep->reinsert(name, symp); return symp; } - static bool existsModScope(AstNodeModule* nodep) { return nodep->user1p() != nullptr; } + static bool existsNodeSym(AstNode* nodep) { return nodep->user1p() != nullptr; } static VSymEnt* getNodeSym(AstNode* nodep) { // Don't use this in ResolveVisitor, as we need to pick up the proper // reference under each SCOPE @@ -1114,6 +1129,10 @@ class LinkDotFindVisitor final : public VNVisitor { nodep->user1p(m_curSymp); iterateChildren(nodep); } + void visit(AstRefDType* nodep) override { // FindVisitor:: + nodep->user1p(m_curSymp); + iterateChildren(nodep); + } void visit(AstNodeBlock* nodep) override { // FindVisitor:: UINFO(5, " " << nodep << endl); if (nodep->name() == "" && nodep->unnamed()) { @@ -1494,8 +1513,7 @@ class LinkDotFindVisitor final : public VNVisitor { } } } - VSymEnt* const insp - = m_statep->insertSym(m_curSymp, nodep->name(), nodep, m_classOrPackagep); + m_statep->insertSym(m_curSymp, nodep->name(), nodep, m_classOrPackagep); if (m_statep->forPrimary() && nodep->isGParam()) { ++m_paramNum; VSymEnt* const symp @@ -1503,13 +1521,6 @@ class LinkDotFindVisitor final : public VNVisitor { nodep, m_classOrPackagep); symp->exported(false); } - AstIfaceRefDType* const ifacerefp - = LinkDotState::ifaceRefFromArray(nodep->subDTypep()); - if (ifacerefp) { - // Can't resolve until interfaces and modport names are - // known; see notes at top - m_statep->insertIfaceVarSym(insp); - } } } } @@ -1758,9 +1769,94 @@ public: //====================================================================== +class LinkDotFindIfaceVisitor final : public VNVisitor { + // NODE STATE + // *::user1p() -> See LinkDotState + + // STATE - for current visit position (use VL_RESTORER) + LinkDotState* const m_statep; // State to pass between visitors, including symbol table + AstNode* m_declp = nullptr; // Current declaring object that may soon contain IfaceRefDType + + // METHODS + const VSymEnt* findNonDeclSym(VSymEnt* symp, const string& name) { + // Find if there is a symbol of given name, ignoring the node that is declaring + // it (m_declp) itself. Thus if searching for "ifc" (an interface): + // + // module x; // Finishes here and finds the typedef + // typedef foo ifc; + // ifc ifc; // symp starts pointing here, but matches m_declp + while (symp) { + const VSymEnt* const foundp = symp->findIdFlat(name); + if (foundp && foundp->nodep() != m_declp) return foundp; + symp = symp->fallbackp(); + } + return nullptr; + } + + // VISITORS + void visit(AstRefDType* nodep) override { // FindIfaceVisitor:: + if (m_statep->forPrimary() && !nodep->classOrPackagep()) { + UINFO(9, " FindIfc: " << nodep << endl); + // If under a var, ignore the var itself as might be e.g. "intf intf;" + // Critical tests: + // t_interface_param_genblk.v // Checks this does make interface + // t_interface_hidden.v // Checks this doesn't making interface when hidden + if (m_statep->existsNodeSym(nodep)) { + VSymEnt* symp = m_statep->getNodeSym(nodep); + const VSymEnt* foundp = findNonDeclSym(symp, nodep->name()); + AstNode* foundNodep = nullptr; + // This: v4make test_regress/t/t_interface_param_genblk.py --debug + // --debugi-V3LinkDot 9 Passes with this commented out: + if (foundp) foundNodep = foundp->nodep(); + if (!foundNodep) foundNodep = m_statep->findModuleSym(nodep->name()); + if (foundNodep) UINFO(9, " Ifc foundNodep " << foundNodep << endl); + if (AstIface* const defp = VN_CAST(foundNodep, Iface)) { + // Must be found as module name, and not hidden/ by normal symbol (foundp) + AstIfaceRefDType* const newp + = new AstIfaceRefDType{nodep->fileline(), "", nodep->name()}; + if (nodep->paramsp()) + newp->addParamsp(nodep->paramsp()->unlinkFrBackWithNext()); + newp->ifacep(defp); + newp->user1u(nodep->user1u()); + UINFO(9, " Resolved interface " << nodep << " => " << defp << endl); + nodep->replaceWith(newp); + VL_DO_DANGLING(pushDeletep(nodep), nodep); + return; + } + } + } + iterateChildren(nodep); + } + void visit(AstVar* nodep) override { // FindVisitor:: + VL_RESTORER(m_declp); + m_declp = nodep; + iterateChildren(nodep); + AstIfaceRefDType* const ifacerefp = LinkDotState::ifaceRefFromArray(nodep->subDTypep()); + if (ifacerefp && m_statep->existsNodeSym(nodep)) { + // Can't resolve until interfaces and modport names are + // known; see notes at top + UINFO(9, " FindIfc Var IfaceRef " << ifacerefp << endl); + if (!ifacerefp->isVirtual()) nodep->setIfaceRef(); + VSymEnt* const symp = m_statep->getNodeSym(nodep); + m_statep->insertIfaceVarSym(symp); + } + } + void visit(AstNode* nodep) override { iterateChildren(nodep); } // FindIfaceVisitor:: + +public: + // CONSTRUCTORS + LinkDotFindIfaceVisitor(AstNetlist* rootp, LinkDotState* statep) + : m_statep{statep} { + UINFO(4, __FUNCTION__ << ": " << endl); + iterate(rootp); + } + ~LinkDotFindIfaceVisitor() override = default; +}; + +//====================================================================== + class LinkDotParamVisitor final : public VNVisitor { // NODE STATE - // Cleared on global // *::user1p() -> See LinkDotState // *::user2p() -> See LinkDotState // *::user4() -> See LinkDotState @@ -4452,6 +4548,9 @@ void V3LinkDot::linkDotGuts(AstNetlist* rootp, VLinkDotStep step) { { LinkDotFindVisitor{rootp, &state}; } dumpSubstep("prelinkdot-find"); + { LinkDotFindIfaceVisitor{rootp, &state}; } + dumpSubstep("prelinkdot-findiface"); + if (step == LDS_PRIMARY || step == LDS_PARAMED) { // Initial link stage, resolve parameters and interfaces { LinkDotParamVisitor{rootp, &state}; } diff --git a/src/verilog.y b/src/verilog.y index caed8eeb2..455f90916 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -1554,8 +1554,10 @@ port: // ==IEEE: port portDirNetE id/*interface*/ portSig variable_dimensionListE sigAttrListE { // VAR for now, but V3LinkCells may call setIfcaeRef on it later $$ = $3; VARDECL(VAR); VARIO(NONE); - AstNodeDType* const dtp = new AstIfaceRefDType{$2, "", *$2}; - VARDTYPE(dtp); VARIOANSI(); + // Although know it's an interface, use AstRefDType for forward compatibility + // with future parser. V3LinkCells will convert to AstIfaceRefDType. + AstNodeDType* const dtp = new AstRefDType{$2, *$2}; + VARDTYPE(dtp); addNextNull($$, VARDONEP($$, $4, $5)); } | portDirNetE id/*interface*/ '.' idAny/*modport*/ portSig variable_dimensionListE sigAttrListE { // VAR for now, but V3LinkCells may call setIfcaeRef on it later diff --git a/test_regress/t/t_interface_missing_bad.out b/test_regress/t/t_interface_missing_bad.out index 0bfc8cedc..01956743a 100644 --- a/test_regress/t/t_interface_missing_bad.out +++ b/test_regress/t/t_interface_missing_bad.out @@ -1,11 +1,17 @@ -%Error: t/t_interface_missing_bad.v:14:4: Cannot find file containing interface: 'foo_intf' +%Error: t/t_interface_missing_bad.v:14:13: Pin is not an in/out/inout/interface: 'foo' + 14 | foo_intf foo + | ^~~ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_interface_missing_bad.v:14:4: Can't find typedef/interface: 'foo_intf' 14 | foo_intf foo | ^~~~~~~~ - ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. %Error: t/t_interface_missing_bad.v:20:4: Cannot find file containing interface: 'foo_intf' 20 | foo_intf the_foo (); | ^~~~~~~~ %Error: t/t_interface_missing_bad.v:25:15: Found definition of 'the_foo' as a CELL but expected a variable 25 | .foo (the_foo) | ^~~~~~~ +%Error: t/t_interface_missing_bad.v:25:10: Instance attempts to connect to 'foo', but it is a variable + 25 | .foo (the_foo) + | ^~~ %Error: Exiting due to diff --git a/test_regress/t/t_interface_param_genblk.v b/test_regress/t/t_interface_param_genblk.v index 67092468a..2a84ad5bb 100644 --- a/test_regress/t/t_interface_param_genblk.v +++ b/test_regress/t/t_interface_param_genblk.v @@ -25,7 +25,7 @@ module t; endmodule module sub ( - intf intf + intf intf // Having this named same "intf intf" important for V3LinkDot coverage ); if (intf.A == 10) begin