From ee469eedaf24c04a632f92c22b2763c0be489f34 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sun, 14 Jul 2019 15:06:49 -0400 Subject: [PATCH] Fix some errors reporting wrong objects. --- src/V3AstNodes.h | 20 +++++++++---- src/V3Error.h | 2 ++ src/V3LinkDot.cpp | 16 +++++----- src/V3LinkLevel.cpp | 40 +++++++++++++++---------- src/V3Width.cpp | 31 ++++++++++++++----- src/verilog.y | 19 ++++++++---- test_regress/t/t_flag_topmodule_bad.out | 6 ++-- test_regress/t/t_mod_dup_bad.out | 4 ++- test_regress/t/t_multitop_sig_bad.out | 6 ++-- 9 files changed, 98 insertions(+), 46 deletions(-) diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 5b07d0e7a..62c1f1e4f 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -586,6 +586,7 @@ public: class AstIfaceRefDType : public AstNodeDType { // Reference to an interface, either for a port, or inside parent cell private: + FileLine* m_modportFileline; // Where modport token was string m_cellName; // "" = no cell, such as when connects to 'input' iface string m_ifaceName; // Interface name string m_modportName; // "" = no modport @@ -593,11 +594,15 @@ private: AstCell* m_cellp; // When exact parent cell known; not a guess AstModport* m_modportp; // NULL = unlinked or no modport public: - AstIfaceRefDType(FileLine* fl, const string& cellName, const string& ifaceName) - : AstNodeDType(fl), m_cellName(cellName), m_ifaceName(ifaceName), m_modportName(""), + AstIfaceRefDType(FileLine* fl, + const string& cellName, const string& ifaceName) + : AstNodeDType(fl), m_modportFileline(NULL), + m_cellName(cellName), m_ifaceName(ifaceName), m_modportName(""), m_ifacep(NULL), m_cellp(NULL), m_modportp(NULL) { } - AstIfaceRefDType(FileLine* fl, const string& cellName, const string& ifaceName, const string& modport) - : AstNodeDType(fl), m_cellName(cellName), m_ifaceName(ifaceName), m_modportName(modport), + AstIfaceRefDType(FileLine* fl, FileLine* modportFl, + const string& cellName, const string& ifaceName, const string& modport) + : AstNodeDType(fl), m_modportFileline(modportFl), + m_cellName(cellName), m_ifaceName(ifaceName), m_modportName(modport), m_ifacep(NULL), m_cellp(NULL), m_modportp(NULL) { } ASTNODE_NODE_FUNCS(IfaceRefDType) // METHODS @@ -613,6 +618,7 @@ public: virtual V3Hash sameHash() const { return V3Hash(m_cellp); } virtual int widthAlignBytes() const { return 1; } virtual int widthTotalBytes() const { return 1; } + FileLine* modportFileline() const { return m_modportFileline; } string cellName() const { return m_cellName; } void cellName(const string& name) { m_cellName = name; } string ifaceName() const { return m_ifaceName; } @@ -1820,6 +1826,7 @@ public: class AstCell : public AstNode { // A instantiation cell or interface call (don't know which until link) private: + FileLine* m_modNameFileline; // Where module the cell instances token was string m_name; // Cell name string m_origName; // Original name before dot addition string m_modName; // Module the cell instances @@ -1828,9 +1835,11 @@ private: bool m_recursive:1; // Self-recursive module bool m_trace:1; // Trace this cell public: - AstCell(FileLine* fl, const string& instName, const string& modName, + AstCell(FileLine* fl, FileLine* mfl, + const string& instName, const string& modName, AstPin* pinsp, AstPin* paramsp, AstRange* rangep) : AstNode(fl) + , m_modNameFileline(mfl) , m_name(instName), m_origName(instName), m_modName(modName) , m_modp(NULL), m_hasIfaceVar(false), m_recursive(false), m_trace(true) { addNOp1p(pinsp); addNOp2p(paramsp); setNOp3p(rangep); } @@ -1846,6 +1855,7 @@ public: void origName(const string& name) { m_origName = name; } string modName() const { return m_modName; } // * = Instance name void modName(const string& name) { m_modName = name; } + FileLine* modNameFileline() const { return m_modNameFileline; } AstPin* pinsp() const { return VN_CAST(op1p(), Pin); } // op1 = List of cell ports AstPin* paramsp() const { return VN_CAST(op2p(), Pin); } // op2 = List of parameter #(##) values AstRange* rangep() const { return VN_CAST(op3p(), Range); } // op3 = Range of arrayed instants (NULL=not ranged) diff --git a/src/V3Error.h b/src/V3Error.h index 7e68dee7e..bf20354ec 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -264,6 +264,8 @@ class V3Error { // When printing an error/warning, print prefix for multiline message static string warnMore(); + /// When building an error, don't show context info + static string warnContextNone() { V3Error::errorContexted(true); return ""; } // Internals for v3error()/v3fatal() macros only // Error end takes the string stream to output, be careful to seek() as needed diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 48d7a39ca..ae14a28bc 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -475,10 +475,11 @@ public: if (!ok) { string suggest = suggestSymFallback( ifaceSymp, ifacerefp->modportName(), LinkNodeMatcherModport()); - ifacerefp->v3error("Modport not found under interface " - <prettyNameQ(ifacerefp->ifaceName())<<": " - <prettyNameQ(ifacerefp->modportName())<warnMore()+suggest)); + ifacerefp->modportFileline() + ->v3error("Modport not found under interface " + <prettyNameQ(ifacerefp->ifaceName())<<": " + <prettyNameQ(ifacerefp->modportName())<warnMore()+suggest)); } } // Alias won't expand until interfaces and modport names are known; see notes at top @@ -1740,7 +1741,8 @@ private: string varName = ifacep->name()+"__Vmp__"+modportp->name()+"__Viftop"+cvtToStr(++m_modportNum); AstIfaceRefDType* idtypep - = new AstIfaceRefDType(fl, cellp->name(), ifacep->name(), modportp->name()); + = new AstIfaceRefDType(fl, modportp->fileline(), + cellp->name(), ifacep->name(), modportp->name()); idtypep->cellp(cellp); AstVar* varp = new AstVar(fl, AstVarType::IFACEREF, varName, VFlagChildDType(), idtypep); varp->isIfaceParent(true); @@ -2032,8 +2034,8 @@ private: ifaceRefVarp, false); nodep->replaceWith(newp); pushDeletep(nodep); VL_DANGLING(nodep); } else if (VN_IS(cellp->modp(), NotFoundModule)) { - cellp->v3error("Cannot find file containing interface: " - <modp()->prettyNameQ()); + cellp->modNameFileline()->v3error("Cannot find file containing interface: " + <modp()->prettyNameQ()); } } } diff --git a/src/V3LinkLevel.cpp b/src/V3LinkLevel.cpp index 4319d19a1..c7975816e 100644 --- a/src/V3LinkLevel.cpp +++ b/src/V3LinkLevel.cpp @@ -53,35 +53,41 @@ void V3LinkLevel::modSortByLevel() { typedef std::vector ModVec; - ModVec vec; - AstNodeModule* topp = NULL; + ModVec mods; // Modules + ModVec tops; // Top level modules for (AstNodeModule* nodep = v3Global.rootp()->modulesp(); nodep; nodep=VN_CAST(nodep->nextp(), NodeModule)) { if (nodep->level()<=2) { - if (topp) { - static int warnedOnce = 0; - nodep->v3warn(MULTITOP, "Multiple top level modules: " - <prettyNameQ()<<" and "<prettyNameQ() - <<(!warnedOnce++ - ? ("\n"+nodep->warnMore() - +"... Suggest see manual; fix the duplicates, or use --top-module to select top.") - : "")); - } - topp = nodep; + tops.push_back(nodep); } - vec.push_back(nodep); + mods.push_back(nodep); } + if (tops.size() >= 2) { + AstNode* secp = tops[1]; // Complain about second one, as first often intended + if (!secp->fileline()->warnIsOff(V3ErrorCode::MULTITOP)) { + secp->v3warn(MULTITOP, "Multiple top level modules\n" + <warnMore() + <<"... Suggest see manual; fix the duplicates, or use --top-module to select top." + <warnMore()<<"... Top module "<prettyNameQ()<warnContextSecondary(); + } + } + } + // Reorder the netlist's modules to have modules in level sorted order - stable_sort(vec.begin(), vec.end(), CmpLevel()); // Sort the vector + stable_sort(mods.begin(), mods.end(), CmpLevel()); // Sort the vector UINFO(9,"modSortByLevel() sorted\n"); // Comment required for gcc4.6.3 / bug666 - for (ModVec::iterator it = vec.begin(); it != vec.end(); ++it) { + for (ModVec::iterator it = mods.begin(); it != mods.end(); ++it) { AstNodeModule* nodep = *it; nodep->clearIter(); // Because we didn't iterate to find the node // pointers, may have a stale m_iterp() needing cleanup nodep->unlinkFrBack(); } UASSERT_OBJ(!v3Global.rootp()->modulesp(), v3Global.rootp(), "Unlink didn't work"); - for (ModVec::iterator it = vec.begin(); it != vec.end(); ++it) { + for (ModVec::iterator it = mods.begin(); it != mods.end(); ++it) { AstNodeModule* nodep = *it; v3Global.rootp()->addModulep(nodep); } @@ -117,6 +123,7 @@ void V3LinkLevel::wrapTop(AstNetlist* rootp) { for (AstNodeModule* modp = rootp->modulesp(); modp; modp=VN_CAST(modp->nextp(), NodeModule)) { if (VN_IS(modp, Package)) { AstCell* cellp = new AstCell(modp->fileline(), + modp->fileline(), // Could add __03a__03a="::" to prevent conflict // with module names/"v" modp->name(), @@ -164,6 +171,7 @@ void V3LinkLevel::wrapTopCell(AstNetlist* rootp) { // Add instance UINFO(5,"LOOP "<fileline(), + newmodp->fileline(), (!v3Global.opt.l2Name().empty() ? v3Global.opt.l2Name() : oldmodp->name()), oldmodp->name(), diff --git a/src/V3Width.cpp b/src/V3Width.cpp index b157cd885..94529dc59 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -421,7 +421,8 @@ private: if (m_vup->final()) { if (!nodep->dtypep()->widthSized()) { // See also error in V3Number - nodep->v3warn(WIDTHCONCAT, "Unsized numbers/parameters not allowed in concatenations."); + nodeForUnsizedWarning(nodep) + ->v3warn(WIDTHCONCAT, "Unsized numbers/parameters not allowed in concatenations."); } } } @@ -436,7 +437,8 @@ private: if (m_vup->final()) { if (!nodep->dtypep()->widthSized()) { // See also error in V3Number - nodep->v3warn(WIDTHCONCAT, "Unsized numbers/parameters not allowed in concatenations."); + nodeForUnsizedWarning(nodep) + ->v3warn(WIDTHCONCAT, "Unsized numbers/parameters not allowed in concatenations."); } } } @@ -470,7 +472,8 @@ private: if (m_vup->final()) { if (!nodep->dtypep()->widthSized()) { // See also error in V3Number - nodep->v3warn(WIDTHCONCAT, "Unsized numbers/parameters not allowed in replications."); + nodeForUnsizedWarning(nodep) + ->v3warn(WIDTHCONCAT, "Unsized numbers/parameters not allowed in replications."); } } } @@ -491,7 +494,8 @@ private: if (m_vup->final()) { if (!nodep->dtypep()->widthSized()) { // See also error in V3Number - nodep->v3warn(WIDTHCONCAT, "Unsized numbers/parameters not allowed in replications."); + nodeForUnsizedWarning(nodep) + ->v3warn(WIDTHCONCAT, "Unsized numbers/parameters not allowed in replications."); } } } @@ -518,7 +522,8 @@ private: if (m_vup->final()) { if (!nodep->dtypep()->widthSized()) { // See also error in V3Number - nodep->v3warn(WIDTHCONCAT, "Unsized numbers/parameters not allowed in streams."); + nodeForUnsizedWarning(nodep) + ->v3warn(WIDTHCONCAT, "Unsized numbers/parameters not allowed in streams."); } } } @@ -2516,8 +2521,9 @@ private: // We've resolved parameters and hit a module that we couldn't resolve. It's // finally time to report it. // Note only here in V3Width as this is first visitor after V3Dead. - nodep->v3error("Cannot find file containing module: '"<modName()<<"'"); - v3Global.opt.filePathLookedMsg(nodep->fileline(), nodep->modName()); + nodep->modNameFileline() + ->v3error("Cannot find file containing module: '"<modName()<<"'"); + v3Global.opt.filePathLookedMsg(nodep->modNameFileline(), nodep->modName()); } if (nodep->rangep()) { m_cellRangep = nodep->rangep(); @@ -4064,6 +4070,17 @@ private: pushDeletep(nodep); VL_DANGLING(nodep); } } + AstNode* nodeForUnsizedWarning(AstNode* nodep) { + // Return a nodep to use for unsized warnings, reporting on child if can + if (nodep->op1p() && nodep->op1p()->dtypep() + && !nodep->op1p()->dtypep()->widthSized()) { + return nodep->op1p(); + } else if (nodep->op2p() && nodep->op2p()->dtypep() + && !nodep->op2p()->dtypep()->widthSized()) { + return nodep->op2p(); + } + return nodep; // By default return this + } //---------------------------------------------------------------------- // METHODS - special iterators diff --git a/src/verilog.y b/src/verilog.y index da37fa057..97e35892e 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -61,6 +61,7 @@ public: AstNodeDType* m_memDTypep; // Pointer to data type for next member declaration bool m_pinAnsi; // In ANSI port list int m_pinNum; // Pin number currently parsing + FileLine* m_instModuleFl; // Fileline of module referenced for instantiations string m_instModule; // Name of module referenced for instantiations AstPin* m_instParamp; // Parameters for instantiations bool m_tracingParse; // Tracing disable for parser @@ -78,6 +79,7 @@ public: m_memDTypep = NULL; m_pinAnsi = false; m_pinNum = -1; + m_instModuleFl = NULL; m_instModule = ""; m_instParamp = NULL; m_varAttrp = NULL; @@ -208,7 +210,10 @@ int V3ParseGrammar::s_modTypeImpNum = 0; #define GATERANGE(rangep) { GRAMMARP->m_gateRangep = rangep; } -#define INSTPREP(modname,paramsp) { GRAMMARP->m_impliedDecl = true; GRAMMARP->m_instModule = modname; GRAMMARP->m_instParamp = paramsp; } +#define INSTPREP(modfl,modname,paramsp) { \ + GRAMMARP->m_impliedDecl = true; \ + GRAMMARP->m_instModuleFl = modfl; GRAMMARP->m_instModule = modname; \ + GRAMMARP->m_instParamp = paramsp; } #define DEL(nodep) { if (nodep) nodep->deleteTree(); } @@ -948,7 +953,7 @@ port: // ==IEEE: port $$->addNextNull(VARDONEP($$,$4,$5)); } | portDirNetE id/*interface*/ '.' idAny/*modport*/ portSig variable_dimensionListE sigAttrListE { $$ = $5; VARDECL(AstVarType::IFACEREF); VARIO(NONE); - VARDTYPE(new AstIfaceRefDType($2,"",*$2,*$4)); + VARDTYPE(new AstIfaceRefDType($2, $4, "", *$2, *$4)); $$->addNextNull(VARDONEP($$,$6,$7)); } | portDirNetE yINTERFACE portSig rangeListE sigAttrListE { $$ = NULL; BBUNSUP($2, "Unsupported: virtual or generic interfaces"); } @@ -2175,13 +2180,13 @@ etcInst: // IEEE: module_instantiation + gate_instantiation + udp_insta ; instDecl: - id parameter_value_assignmentE {INSTPREP(*$1,$2);} instnameList ';' + id parameter_value_assignmentE {INSTPREP($1,*$1,$2);} instnameList ';' { $$ = $4; GRAMMARP->m_impliedDecl=false; if (GRAMMARP->m_instParamp) { GRAMMARP->m_instParamp->deleteTree(); GRAMMARP->m_instParamp = NULL; } } // // IEEE: interface_identifier' .' modport_identifier list_of_interface_identifiers | id/*interface*/ '.' id/*modport*/ { VARRESET_NONLIST(AstVarType::IFACEREF); - VARDTYPE(new AstIfaceRefDType($1,"",*$1,*$3)); } + VARDTYPE(new AstIfaceRefDType($1, $3, "", *$1, *$3)); } mpInstnameList ';' { $$ = VARDONEP($5,NULL,NULL); } //UNSUP: strengthSpecE for udp_instantiations @@ -2203,11 +2208,13 @@ instnameList: instnameParen: // // Must clone m_instParamp as may be comma'ed list of instances - id instRangeE '(' cellpinList ')' { $$ = new AstCell($1, *$1, GRAMMARP->m_instModule, $4, + id instRangeE '(' cellpinList ')' { $$ = new AstCell($1, GRAMMARP->m_instModuleFl, + *$1, GRAMMARP->m_instModule, $4, AstPin::cloneTreeNull(GRAMMARP->m_instParamp, true), GRAMMARP->scrubRange($2)); $$->trace(GRAMMARP->allTracingOn($1)); } - | id instRangeE { $$ = new AstCell($1, *$1, GRAMMARP->m_instModule, NULL, + | id instRangeE { $$ = new AstCell($1, GRAMMARP->m_instModuleFl, + *$1, GRAMMARP->m_instModule, NULL, AstPin::cloneTreeNull(GRAMMARP->m_instParamp, true), GRAMMARP->scrubRange($2)); $$->trace(GRAMMARP->allTracingOn($1)); } diff --git a/test_regress/t/t_flag_topmodule_bad.out b/test_regress/t/t_flag_topmodule_bad.out index 6d74927d1..cc83a3175 100644 --- a/test_regress/t/t_flag_topmodule_bad.out +++ b/test_regress/t/t_flag_topmodule_bad.out @@ -1,5 +1,7 @@ -%Warning-MULTITOP: t/t_flag_topmodule.v:14: Multiple top level modules: 'a2' and 'a' +%Warning-MULTITOP: t/t_flag_topmodule.v:14: Multiple top level modules : ... Suggest see manual; fix the duplicates, or use --top-module to select top. ... Use "/* verilator lint_off MULTITOP */" and lint_on around source to disable this message. -%Warning-MULTITOP: t/t_flag_topmodule.v:21: Multiple top level modules: 'b' and 'a2' + : ... Top module 'a' + : ... Top module 'a2' + : ... Top module 'b' %Error: Exiting due to diff --git a/test_regress/t/t_mod_dup_bad.out b/test_regress/t/t_mod_dup_bad.out index 8911d429a..85969d71c 100644 --- a/test_regress/t/t_mod_dup_bad.out +++ b/test_regress/t/t_mod_dup_bad.out @@ -1,6 +1,8 @@ %Warning-MODDUP: t/t_mod_dup_bad.v:13: Duplicate declaration of module: 'a' t/t_mod_dup_bad.v:6: ... Location of original declaration ... Use "/* verilator lint_off MODDUP */" and lint_on around source to disable this message. -%Warning-MULTITOP: t/t_mod_dup_bad.v:16: Multiple top level modules: 'b' and 'test' +%Warning-MULTITOP: t/t_mod_dup_bad.v:16: Multiple top level modules : ... Suggest see manual; fix the duplicates, or use --top-module to select top. + : ... Top module 'test' + : ... Top module 'b' %Error: Exiting due to diff --git a/test_regress/t/t_multitop_sig_bad.out b/test_regress/t/t_multitop_sig_bad.out index e7e8f5080..67b5e82d3 100644 --- a/test_regress/t/t_multitop_sig_bad.out +++ b/test_regress/t/t_multitop_sig_bad.out @@ -1,5 +1,7 @@ -%Warning-MULTITOP: t/t_multitop_sig.v:14: Multiple top level modules: 'b' and 'a' +%Warning-MULTITOP: t/t_multitop_sig.v:14: Multiple top level modules : ... Suggest see manual; fix the duplicates, or use --top-module to select top. ... Use "/* verilator lint_off MULTITOP */" and lint_on around source to disable this message. -%Warning-MULTITOP: t/t_multitop_sig.v:22: Multiple top level modules: 'c' and 'b' + : ... Top module 'a' + : ... Top module 'b' + : ... Top module 'c' %Error: Exiting due to