diff --git a/Changes b/Changes index 5e997be3f..99f927833 100644 --- a/Changes +++ b/Changes @@ -4,6 +4,8 @@ The contributors that suggested a given feature are shown in []. Thanks! * Verilator 3.915 devel +*** Support self-recursive modules, bug659. [Sean Moore, et al] + **** Fix MacOS portability, bug1232. [Jeff Bush] **** Detect MSB overflow when under VL_DEBUG, bug1238. [Junyi Xi] diff --git a/src/V3Ast.h b/src/V3Ast.h index b80bf7409..7281c0718 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -1977,6 +1977,8 @@ private: bool m_inLibrary:1; // From a library, no error if not used, never top level bool m_dead:1; // LinkDot believes is dead; will remove in Dead visitors bool m_internal:1; // Internally created + bool m_recursive:1; // Recursive module + bool m_recursiveClone:1; // If recursive, what module it clones, otherwise NULL int m_level; // 1=top module, 2=cell off top module, ... int m_varNum; // Incrementing variable number int m_typeNum; // Incrementing implicit type number @@ -1984,7 +1986,8 @@ public: AstNodeModule(FileLine* fl, const string& name) : AstNode (fl) ,m_name(name), m_origName(name) - ,m_modPublic(false), m_modTrace(false), m_inLibrary(false), m_dead(false), m_internal(false) + ,m_modPublic(false), m_modTrace(false), m_inLibrary(false), m_dead(false) + ,m_internal(false), m_recursive(false), m_recursiveClone(false) ,m_level(0), m_varNum(0), m_typeNum(0) { } ASTNODE_BASE_FUNCS(NodeModule) virtual void dump(ostream& str); @@ -2014,6 +2017,10 @@ public: bool dead() const { return m_dead; } void internal(bool flag) { m_internal = flag; } bool internal() const { return m_internal; } + void recursive(bool flag) { m_recursive = flag; } + bool recursive() const { return m_recursive; } + void recursiveClone(bool flag) { m_recursiveClone = flag; } + bool recursiveClone() const { return m_recursiveClone; } }; //###################################################################### diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 958aabbb1..7f2fbe3a9 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -779,6 +779,7 @@ void AstCCast::dump(ostream& str) { } void AstCell::dump(ostream& str) { this->AstNode::dump(str); + if (recursive()) str<<" [RECURSIVE]"; if (modp()) { str<<" -> "; modp()->dump(str); } else { str<<" ->UNLINKED:"<AstNode::dump(str); diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 8346eb5f1..3271e4b3c 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -1639,13 +1639,14 @@ private: string m_modName; // Module the cell instances AstNodeModule* m_modp; // [AfterLink] Pointer to module instanced bool m_hasIfaceVar:1; // True if a Var has been created for this cell + bool m_recursive:1; // Self-recursive module bool m_trace:1; // Trace this cell public: AstCell(FileLine* fl, const string& instName, const string& modName, AstPin* pinsp, AstPin* paramsp, AstRange* rangep) : AstNode(fl) , m_name(instName), m_origName(instName), m_modName(modName) - , m_modp(NULL), m_hasIfaceVar(false), m_trace(true) { + , m_modp(NULL), m_hasIfaceVar(false), m_recursive(false), m_trace(true) { addNOp1p(pinsp); addNOp2p(paramsp); setNOp3p(rangep); } ASTNODE_NODE_FUNCS(Cell) // No cloneRelink, we presume cloneee's want the same module linkages @@ -1670,6 +1671,8 @@ public: void hasIfaceVar(bool flag) { m_hasIfaceVar = flag; } void trace(bool flag) { m_trace=flag; } bool isTrace() const { return m_trace; } + void recursive(bool flag) { m_recursive = flag; } + bool recursive() const { return m_recursive; } }; class AstCellInline : public AstNode { diff --git a/src/V3Dead.cpp b/src/V3Dead.cpp index bf9555ca5..deec05bb7 100644 --- a/src/V3Dead.cpp +++ b/src/V3Dead.cpp @@ -137,8 +137,10 @@ private: // VISITORS virtual void visit(AstNodeModule* nodep) { m_modp = nodep; - nodep->iterateChildren(*this); - checkAll(nodep); + if (!nodep->dead()) { + nodep->iterateChildren(*this); + checkAll(nodep); + } m_modp = NULL; } virtual void visit(AstCFunc* nodep) { @@ -280,12 +282,14 @@ private: AstNodeModule* nextmodp; for (AstNodeModule* modp = v3Global.rootp()->modulesp(); modp; modp=nextmodp) { nextmodp = modp->nextp()->castNodeModule(); - if (modp->level()>2 && modp->user1()==0 && !modp->internal()) { + if (modp->dead() || (modp->level()>2 && modp->user1()==0 && !modp->internal())) { // > 2 because L1 is the wrapper, L2 is the top user module UINFO(4," Dead module "<dead()) { // If was dead didn't increment user1's + DeadModVisitor visitor(modp); + } modp->unlinkFrBack()->deleteTree(); VL_DANGLING(modp); retry = true; } diff --git a/src/V3Graph.h b/src/V3Graph.h index e3046a932..a1a9ac896 100644 --- a/src/V3Graph.h +++ b/src/V3Graph.h @@ -181,6 +181,7 @@ public: virtual string dotShape() const { return ""; } virtual string dotStyle() const { return ""; } virtual string dotName() const { return ""; } + virtual uint32_t rankAdder() const { return 1; } virtual int sortCmp(const V3GraphVertex* rhsp) const { // LHS goes first if of lower rank, or lower fanout if (m_rank < rhsp->m_rank) return -1; diff --git a/src/V3GraphAlg.cpp b/src/V3GraphAlg.cpp index 6bf2be5cd..bcd6fbe22 100644 --- a/src/V3GraphAlg.cpp +++ b/src/V3GraphAlg.cpp @@ -300,7 +300,7 @@ private: vertexp->rank(currentRank); for (V3GraphEdge* edgep = vertexp->outBeginp(); edgep; edgep=edgep->outNextp()) { if (followEdge(edgep)) { - vertexIterate(edgep->top(),currentRank+1); + vertexIterate(edgep->top(), currentRank + vertexp->rankAdder()); } } vertexp->user(2); diff --git a/src/V3LinkCells.cpp b/src/V3LinkCells.cpp index abd01de1e..98ba20b0e 100644 --- a/src/V3LinkCells.cpp +++ b/src/V3LinkCells.cpp @@ -62,6 +62,8 @@ public: virtual ~LinkCellsVertex() {} AstNodeModule* modp() const { return m_modp; } virtual string name() const { return modp()->name(); } + // Recursive modules get space for maximum recursion + virtual uint32_t rankAdder() const { return m_modp->recursiveClone() ? (1+v3Global.opt.moduleRecursionDepth()) : 1; } }; class LibraryVertex : public V3GraphVertex { @@ -74,8 +76,9 @@ public: void LinkCellsGraph::loopsMessageCb(V3GraphVertex* vertexp) { if (LinkCellsVertex* vvertexp = dynamic_cast(vertexp)) { - vvertexp->modp()->v3error("Recursive module (module instantiates itself): " + vvertexp->modp()->v3error("Unsupported: Recursive multiple modules (module instantiates something leading back to itself): " <modp()->prettyName()); + vvertexp->modp()->v3error("Note self-recursion (module instantiating itself directly) is supported."); V3Error::abortIfErrors(); } else { // Everything should match above, but... v3fatalSrc("Recursive instantiations"); @@ -90,10 +93,13 @@ private: // NODE STATE // Entire netlist: // AstNodeModule::user1p() // V3GraphVertex* Vertex describing this module - // AstCell::user1() // bool Did it. + // AstNodeModule::user2p() // AstNodeModule* clone used for de-recursing + // AstCell::user1p() // ==V3NodeModule* if done, != if unprocessed + // AstCell::user2() // bool clone renaming completed // Allocated across all readFiles in V3Global::readFiles: // AstNode::user4p() // VSymEnt* Package and typedef symbol names AstUser1InUse m_inuser1; + AstUser2InUse m_inuser2; // STATE V3InFilter* m_filterp; // Parser filter @@ -249,7 +255,7 @@ private: // this move to post param, which would mean we do not auto-read modules // and means we cannot compute module levels until later. UINFO(4,"Link Bind: "<name()); + AstNodeModule* modp = resolveModule(nodep,nodep->name()); if (modp) { AstNode* cellsp = nodep->cellsp()->unlinkFrBackWithNext(); // Module may have already linked, so need to pick up these new cells @@ -266,16 +272,58 @@ private: virtual void visit(AstCell* nodep) { // Cell: Resolve its filename. If necessary, parse it. - if (nodep->user1SetOnce()) return; // AstBind and AstNodeModule may call a cell twice - if (!nodep->modp()) { + // Execute only once. Complication is that cloning may result in user1 being set (for pre-clone) + // so check if user1() matches the m_mod, if 0 never did it, if !=, it is an unprocessed clone + bool cloned = (nodep->user1p() && nodep->user1p()!=m_modp); + if (nodep->user1p()==m_modp) return; // AstBind and AstNodeModule may call a cell twice + nodep->user1p(m_modp); + // + if (!nodep->modp() || cloned) { UINFO(4,"Link Cell: "<modName()); if (cellmodp) { - nodep->modp(cellmodp); - // Track module depths, so can sort list from parent down to children - new V3GraphEdge(&m_graph, vertex(m_modp), vertex(cellmodp), 1, false); + if (cellmodp == m_modp + || cellmodp->user2p() == m_modp) { + UINFO(1,"Self-recursive module "<recursive(true); + nodep->recursive(true); + if (!cellmodp->recursiveClone()) { + // In the non-Vrcm, which needs to point to Vrcm flavor + // + // Make a clone which this cell points to + // Later, the clone's cells will also point clone'd name + // This lets us link the XREFs between the (uncloned) children so + // they don't point to the same module which would + // break parameter resolution. + AstNodeModule* otherModp = cellmodp->user2p()->castNodeModule(); + if (!otherModp) { + otherModp = cellmodp->cloneTree(false); + otherModp->name(otherModp->name()+"__Vrcm"); + otherModp->user1p(NULL); // Need new vertex + otherModp->user2p(cellmodp); + otherModp->recursiveClone(true); + // user1 etc will retain its pre-clone value + cellmodp->user2p(otherModp); + v3Global.rootp()->addModulep(otherModp); + new V3GraphEdge(&m_graph, vertex(cellmodp), vertex(otherModp), 1, false); + } + cellmodp = otherModp; + nodep->modp(cellmodp); + } + else { + // In the Vrcm, which needs to point back to Vrcm flavor + // The cell already has the correct resolution (to Vrcm) + nodep->modp(cellmodp); + // We don't create a V3GraphEdge (as it would be circular) + } + } + else { // Non-recursive + // Track module depths, so can sort list from parent down to children + nodep->modp(cellmodp); + new V3GraphEdge(&m_graph, vertex(m_modp), vertex(cellmodp), 1, false); + } } } // Remove AstCell(AstPin("",NULL)), it's a side effect of how we parse "()" @@ -310,6 +358,7 @@ private: if (pinp->name()=="") pinp->name("__paramNumber"+cvtToStr(pinp->pinNum())); } if (nodep->modp()) { + nodep->modName(nodep->modp()->name()); // Note what pins exist vl_unordered_set ports; // Symbol table of all connected port names for (AstPin* pinp = nodep->pinsp(); pinp; pinp=pinp->nextp()->castPin()) { diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 52e2cf58d..b62050607 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -87,6 +87,7 @@ private: // NODE STATE // Cleared on Netlist // AstNodeModule::user1p() // VSymEnt*. Last symbol created for this node + // AstNodeModule::user2() // bool. Currently processing for recursion check // ... Note maybe more than one, as can be multiple hierarchy places // AstVarScope::user2p() // AstVarScope*. Base alias for AstInline of this signal // AstVar::user2p() // AstFTask*. If a function variable, the task that links to the variable @@ -595,6 +596,7 @@ class LinkDotFindVisitor : public AstNVisitor { AstBegin* m_beginp; // Current Begin/end block AstNodeFTask* m_ftaskp; // Current function/task bool m_inGenerate; // Inside a generate + bool m_inRecursion; // Inside a recursive module int m_paramNum; // Parameter number, for position based connection int m_beginNum; // Begin block number, 0=none seen int m_modBeginNum; // Begin block number in module, 0=none seen @@ -682,7 +684,10 @@ class LinkDotFindVisitor : public AstNVisitor { int oldParamNum = m_paramNum; int oldBeginNum = m_beginNum; int oldModBeginNum = m_modBeginNum; - if (doit) { + if (doit && nodep->user2()) { + nodep->v3error("Unsupported: Identically recursive module (module instantiates itself, without changing parameters): " + <origName())); + } else if (doit) { UINFO(2," Link Module: "<dead()) nodep->v3fatalSrc("Module in cell tree mislabeled as dead?"); VSymEnt* upperSymp = m_curSymp ? m_curSymp : m_statep->rootEntp(); @@ -703,7 +708,9 @@ class LinkDotFindVisitor : public AstNVisitor { m_modBeginNum = 0; // m_modSymp/m_curSymp for non-packages set by AstCell above this module // Iterate + nodep->user2(true); nodep->iterateChildren(*this); + nodep->user2(false); nodep->user4(true); // Interfaces need another pass when signals are resolved if (AstIface* ifacep = nodep->castIface()) { @@ -730,6 +737,7 @@ class LinkDotFindVisitor : public AstNVisitor { virtual void visit(AstCell* nodep) { UINFO(5," CELL under "<recursive() && m_inRecursion) return; nodep->iterateChildren(*this); // Recurse in, preserving state string oldscope = m_scope; @@ -737,6 +745,7 @@ class LinkDotFindVisitor : public AstNVisitor { VSymEnt* oldModSymp = m_modSymp; VSymEnt* oldCurSymp = m_curSymp; int oldParamNum = m_paramNum; + bool oldRecursion = m_inRecursion; // Where do we add it? VSymEnt* aboveSymp = m_curSymp; string origname = AstNode::dedotName(nodep->name()); @@ -755,6 +764,7 @@ class LinkDotFindVisitor : public AstNVisitor { m_scope = m_scope+"."+nodep->name(); m_curSymp = m_modSymp = m_statep->insertCell(aboveSymp, m_modSymp, nodep, m_scope); m_beginp = NULL; + m_inRecursion = nodep->recursive(); // We don't report NotFoundModule, as may be a unused module in a generate if (nodep->modp()) nodep->modp()->accept(*this); } @@ -763,6 +773,7 @@ class LinkDotFindVisitor : public AstNVisitor { m_modSymp = oldModSymp; m_curSymp = oldCurSymp; m_paramNum = oldParamNum; + m_inRecursion = oldRecursion; } virtual void visit(AstCellInline* nodep) { UINFO(5," CELLINLINE under "< CellList; CellList m_cellps; // Cells left to process (in this module) + AstNodeModule* m_modp; // Current module being processed + string m_unlinkedTxt; // Text for AstUnlinkedRef // METHODS @@ -218,6 +220,7 @@ private: AstNodeModule* nodep = it->second; m_todoModps.erase(it); if (!nodep->user5SetOnce()) { // Process once; note clone() must clear so we do it again + m_modp = nodep; UINFO(4," MOD "<iterateChildren(*this); // Note above iterate may add to m_todoModps @@ -233,6 +236,7 @@ private: } } m_cellps.clear(); + m_modp = NULL; } } } @@ -245,6 +249,9 @@ private: virtual void visit(AstNodeModule* nodep) { if (nodep->dead()) { UINFO(4," MOD-dead. "<recursiveClone()) { + UINFO(4," MOD-recursive-dead. "<dead(true); // So Dead checks won't count references to it } else if (nodep->level() <= 2 // Haven't added top yet, so level 2 is the top || nodep->castPackage()) { // Likewise haven't done wrapTopPackages yet // Add request to END of modules left to process @@ -511,6 +518,7 @@ public: // CONSTUCTORS explicit ParamVisitor(AstNetlist* nodep) { m_longId = 0; + m_modp = NULL; // nodep->accept(*this); } @@ -539,6 +547,7 @@ void ParamVisitor::visitCell(AstCell* nodep) { //if (debug()) nodep->dumpTree(cout,"-cel2:\t"); string longname = srcModp->name(); bool any_overrides = false; + if (nodep->recursive()) any_overrides = true; // Must always clone __Vrcm (recursive modules) longname += "_"; if (debug()>8) nodep->paramsp()->dumpTreeAndNext(cout,"-cellparams:\t"); for (AstPin* pinp = nodep->paramsp(); pinp; pinp=pinp->nextp()->castPin()) { @@ -679,7 +688,21 @@ void ParamVisitor::visitCell(AstCell* nodep) { cellmodp = srcModp->cloneTree(false); cellmodp->name(newname); cellmodp->user5(false); // We need to re-recurse this module once changed - srcModp->addNextHere(cellmodp); // Keep tree sorted by cell occurrences + cellmodp->recursive(false); + cellmodp->recursiveClone(false); + nodep->recursive(false); + // Recursion may need level cleanups + if (cellmodp->level() <= m_modp->level()) cellmodp->level(m_modp->level()+1); + if ((cellmodp->level() - srcModp->level()) >= (v3Global.opt.moduleRecursionDepth() - 2)) { + nodep->v3error("Exceeded maximum --module-recursion-depth of "<nextp()->castNodeModule() + && insertp->nextp()->castNodeModule()->level() < cellmodp->level()) { + insertp = insertp->nextp()->castNodeModule(); + } + insertp->addNextHere(cellmodp); m_modNameMap.insert(make_pair(cellmodp->name(), ModInfo(cellmodp))); iter = m_modNameMap.find(newname); @@ -742,6 +765,8 @@ void ParamVisitor::visitCell(AstCell* nodep) { UINFO(8," Done with "<recursive(false); + // Delete the parameters from the cell; they're not relevant any longer. if (nodep->paramsp()) nodep->paramsp()->unlinkFrBackWithNext()->deleteTree(); UINFO(8," Done with "<1, + expect=> +'.*%Error: t/t_inst_recurse2_bad.v:\d+: Unsupported: Identically recursive module \(module instantiates itself, without changing parameters\): looped +%Error: Exiting due to.*', + ); + +ok(1); +1; diff --git a/test_regress/t/t_inst_recurse2_bad.v b/test_regress/t/t_inst_recurse2_bad.v new file mode 100644 index 000000000..bbf649654 --- /dev/null +++ b/test_regress/t/t_inst_recurse2_bad.v @@ -0,0 +1,19 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2005 by Wilson Snyder. + +module t (/*AUTOARG*/ + // Inputs + clk + ); + + input clk; + + looped looped (); + +endmodule + +module looped (/*AUTOARG*/); + looped looped (); +endmodule diff --git a/test_regress/t/t_inst_recurse_bad.pl b/test_regress/t/t_inst_recurse_bad.pl index 627f9cdc5..cd8941191 100755 --- a/test_regress/t/t_inst_recurse_bad.pl +++ b/test_regress/t/t_inst_recurse_bad.pl @@ -8,11 +8,12 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di # Version 2.0. compile ( - fails=>1, - expect=> -'.*%Error: t/t_inst_recurse_bad.v:\d+: Recursive module .module instantiates itself.: looped + fails=>1, + expect=> +'.*%Error: t/t_inst_recurse_bad.v:\d+: Unsupported: Recursive multiple modules \(module instantiates something leading back to itself\): looped +%Error: t/t_inst_recurse_bad.v:\d+: Note self-recursion \(module instantiating itself directly\) is supported. %Error: Exiting due to.*', - ); + ); ok(1); 1; diff --git a/test_regress/t/t_mod_recurse.pl b/test_regress/t/t_mod_recurse.pl index e74759103..f91289753 100755 --- a/test_regress/t/t_mod_recurse.pl +++ b/test_regress/t/t_mod_recurse.pl @@ -7,8 +7,6 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di # Lesser General Public License Version 3 or the Perl Artistic License # Version 2.0. -$Self->{vlt} and $Self->unsupported("Verilator unsupported, bug659"); - compile ( ); diff --git a/test_regress/t/t_mod_recurse.v b/test_regress/t/t_mod_recurse.v index 3cef4ff5d..031e97f6e 100644 --- a/test_regress/t/t_mod_recurse.v +++ b/test_regress/t/t_mod_recurse.v @@ -25,7 +25,7 @@ module t (/*AUTOARG*/ pe (.out(valid), .outN(value[2:0]), .tripline(tripline)); // Aggregate outputs into a single result vector - wire [63:0] result = {59'h0, valid, value}; + wire [63:0] result = {60'h0, valid, value}; // Test loop always @ (posedge clk) begin @@ -77,9 +77,9 @@ module PriorityChoice (out, outN, tripline); assign right = tripline[0]; always @(*) begin - out <= left || right ; - if(right) begin outN <= {1'b0}; end - else begin outN <= {1'b1}; end + out = left || right ; + if(right) begin outN = {1'b0}; end + else begin outN = {1'b1}; end end end else begin PriorityChoice #(.OCODEWIDTH(OCODEWIDTH-1)) @@ -98,11 +98,11 @@ module PriorityChoice (out, outN, tripline); ); always @(*) begin if(right) begin - out <= right; - outN <= {1'b0, rightN[OCODEWIDTH-2:0]}; + out = right; + outN = {1'b0, rightN[OCODEWIDTH-2:0]}; end else begin - out <= left; - outN <= {1'b1, leftN[OCODEWIDTH-2:0]}; + out = left; + outN = {1'b1, leftN[OCODEWIDTH-2:0]}; end end end diff --git a/test_regress/t/t_mod_recurse1.pl b/test_regress/t/t_mod_recurse1.pl new file mode 100755 index 000000000..f91289753 --- /dev/null +++ b/test_regress/t/t_mod_recurse1.pl @@ -0,0 +1,18 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 by Wilson Snyder. 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. + +compile ( + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_mod_recurse1.v b/test_regress/t/t_mod_recurse1.v new file mode 100644 index 000000000..ae493982f --- /dev/null +++ b/test_regress/t/t_mod_recurse1.v @@ -0,0 +1,35 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2013 by Sean Moore. + +module t (/*AUTOARG*/); + + rec rec (); + +endmodule + +module rec; + parameter DEPTH = 1; + + generate + if (DEPTH==1) begin + rec #(.DEPTH(DEPTH+1)) sub; + end + else if (DEPTH==2) begin + rec #(.DEPTH(DEPTH+1)) subb; + end + else if (DEPTH==3) begin + bottom #(.DEPTH(DEPTH+1)) bot; + end + endgenerate +endmodule + +module bottom; + parameter DEPTH = 1; + initial begin + if (DEPTH!=4) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule