From deb7a1c9c0ff327d601c3a2e0b84c5891a20b95f Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Fri, 28 Apr 2017 20:03:38 -0400 Subject: [PATCH] Fix non-arrayed cells with interface arrays, bug1153. --- Changes | 2 + src/V3Inst.cpp | 160 ++++++++++++++++++++++---------- test_regress/t/t_inst_darray.pl | 18 ++++ test_regress/t/t_inst_darray.v | 85 +++++++++++++++++ 4 files changed, 214 insertions(+), 51 deletions(-) create mode 100755 test_regress/t/t_inst_darray.pl create mode 100644 test_regress/t/t_inst_darray.v diff --git a/Changes b/Changes index b3a01bf9a..f278ae6f5 100644 --- a/Changes +++ b/Changes @@ -7,6 +7,8 @@ The contributors that suggested a given feature are shown in []. Thanks! *** Support ports of array of reals, bug1154. [J Briquet] +**** Fix non-arrayed cells with interface arrays, bug1153. [John Stevenson] + **** Add warning on mis-sized literal, bug1156. [Todd Strader] diff --git a/src/V3Inst.cpp b/src/V3Inst.cpp index ae65ed26f..9551f841d 100644 --- a/src/V3Inst.cpp +++ b/src/V3Inst.cpp @@ -90,7 +90,7 @@ private: // Make a ASSIGNW (expr, pin) AstNode* exprp = nodep->exprp()->cloneTree(false); if (exprp->width() != nodep->modVarp()->width()) - nodep->v3fatalSrc("Width mismatch, should have been handled in pinReconnectSimple\n"); + nodep->v3fatalSrc("Width mismatch, should have been handled in pinReconnectSimple"); if (nodep->modVarp()->isInout()) { nodep->v3fatalSrc("Unsupported: Verilator is a 2-state simulator"); } else if (nodep->modVarp()->isOutput()) { @@ -154,14 +154,10 @@ public: //###################################################################### -class InstDeVisitor : public AstNVisitor { - // Find all cells with arrays, and convert to non-arrayed +class InstDeModVarVisitor : public AstNVisitor { + // Expand all module variables, and save names for later reference private: // STATE - AstRange* m_cellRangep; // Range for arrayed instantiations, NULL for normal instantiations - int m_instNum; // Current instantiation number - int m_instLsb; // Current instantiation number - typedef map VarNameMap; VarNameMap m_modVarNameMap; // Per module, name of cloned variables @@ -172,15 +168,109 @@ private: } // VISITORS - virtual void visit(AstNodeModule* nodep) { + virtual void visit(AstVar* nodep) { + if (nodep->dtypep()->castIfaceRefDType()) { + UINFO(8," dm-1-VAR "<iterateChildren(*this); + } + // Save some time + virtual void visit(AstNodeMath*) {} + // Default: Just iterate + virtual void visit(AstNode* nodep) { + nodep->iterateChildren(*this); + } +public: + // METHODS + void insert(AstVar* nodep) { + UINFO(8," dmINSERT "<name(), nodep)); + } + AstVar* find(const string& name) { + VarNameMap::iterator it = m_modVarNameMap.find(name); + if (it != m_modVarNameMap.end()) { + return it->second; + } else { + return NULL; + } + } + void dump() { + for (VarNameMap::iterator it=m_modVarNameMap.begin(); it!=m_modVarNameMap.end(); ++it) { + cout<<"-namemap: "<first<<" -> "<second<accept(*this); + } + virtual ~InstDeModVarVisitor() {} +}; + +//###################################################################### + +class InstDeVisitor : public AstNVisitor { + // Find all cells with arrays, and convert to non-arrayed +private: + // STATE + AstRange* m_cellRangep; // Range for arrayed instantiations, NULL for normal instantiations + int m_instNum; // Current instantiation number + int m_instLsb; // Current instantiation number + InstDeModVarVisitor m_deModVars; // State of variables for current cell module + + typedef map VarNameMap; + + static int debug() { + static int level = -1; + if (VL_UNLIKELY(level < 0)) level = v3Global.opt.debugSrcLevel(__FILE__); + return level; + } + + // VISITORS + virtual void visit(AstVar* nodep) { + if (nodep->dtypep()->castUnpackArrayDType() + && nodep->dtypep()->castUnpackArrayDType()->subDTypep()->castIfaceRefDType()) { + UINFO(8," dv-vec-VAR "<dtypep()->castUnpackArrayDType(); + AstNode* prevp = NULL; + for (int i = arrdtype->lsb(); i <= arrdtype->msb(); ++i) { + string varNewName = nodep->name() + "__BRA__" + cvtToStr(i) + "__KET__"; + UINFO(8,"VAR name insert "<subDTypep()->castIfaceRefDType()->cloneTree(false); + arrdtype->addNextHere(ifaceRefp); + ifaceRefp->cellp(NULL); + + AstVar* varNewp = nodep->cloneTree(false); + varNewp->name(varNewName); + varNewp->origName(varNewp->origName() + "__BRA__" + cvtToStr(i) + "__KET__"); + varNewp->dtypep(ifaceRefp); + m_deModVars.insert(varNewp); + if (!prevp) { + prevp = varNewp; + } else { + prevp->addNextHere(varNewp); + } + } + } + if (prevp) nodep->addNextHere(prevp); + if (prevp && debug()==9) { prevp->dumpTree(cout, "newintf: "); cout << endl; } + } nodep->iterateChildren(*this); } virtual void visit(AstCell* nodep) { + UINFO(4," CELL "<modp()) nodep->v3fatalSrc("Unlinked"); + m_deModVars.accept(nodep->modp()); + // if (nodep->rangep()) { m_cellRangep = nodep->rangep(); - UINFO(4," CELL "<nextp()->castVar(); bool isIface = ifaceVarp @@ -199,9 +289,10 @@ private: // The spec says we add [x], but that won't work in C... newp->name(newp->name()+"__BRA__"+cvtToStr(m_instNum)+"__KET__"); newp->origName(newp->origName()+"__BRA__"+cvtToStr(m_instNum)+"__KET__"); + UINFO(8," CELL loop "<dtypep()->castUnpackArrayDType(); AstIfaceRefDType* origIfaceRefp = arrdtype->subDTypep()->castIfaceRefDType(); @@ -229,42 +320,11 @@ private: } nodep->unlinkFrBack(); pushDeletep(nodep); VL_DANGLING(nodep); } else { + m_cellRangep = NULL; nodep->iterateChildren(*this); } } - virtual void visit(AstVar* nodep) { - bool isIface = nodep->dtypep()->castUnpackArrayDType() - && nodep->dtypep()->castUnpackArrayDType()->subDTypep()->castIfaceRefDType(); - if (isIface) { - AstUnpackArrayDType* arrdtype = nodep->dtypep()->castUnpackArrayDType(); - AstNode* prev = NULL; - for (int i = arrdtype->lsb(); i <= arrdtype->msb(); ++i) { - AstIfaceRefDType* ifaceRefp = arrdtype->subDTypep()->castIfaceRefDType()->cloneTree(false); - arrdtype->addNextHere(ifaceRefp); - ifaceRefp->cellp(NULL); - - string varNewName = nodep->name() + "__BRA__" + cvtToStr(i) + "__KET__"; - VarNameMap::iterator it = m_modVarNameMap.find(varNewName); - if (it == m_modVarNameMap.end()) { - AstVar* varNewp = nodep->cloneTree(false); - m_modVarNameMap.insert(make_pair(varNewName, varNewp)); - varNewp->name(varNewName); - varNewp->origName(varNewp->origName() + "__BRA__" + cvtToStr(i) + "__KET__"); - varNewp->dtypep(ifaceRefp); - if (!prev) { - prev = varNewp; - } else { - prev->addNextHere(varNewp); - } - } - } - if (prev) nodep->addNextHere(prev); - if (prev && debug()==9) { prev->dumpTree(cout, "newintf: "); cout << endl; } - } - nodep->iterateChildren(*this); - } - virtual void visit(AstPin* nodep) { // Any non-direct pins need reconnection with a part-select if (!nodep->exprp()) return; // No-connect @@ -334,30 +394,28 @@ private: // Clone pin varp: for (int i = pinArrp->lsb(); i <= pinArrp->msb(); ++i) { string varNewName = pinVarp->name() + "__BRA__" + cvtToStr(i) + "__KET__"; - VarNameMap::iterator it = m_modVarNameMap.find(varNewName); AstVar* varNewp = NULL; - // Only clone the var once for each module + // Only clone the var once for all usages of a given child module if (!pinVarp->backp()) { - if (it != m_modVarNameMap.end()) { - varNewp = it->second; - } + varNewp = m_deModVars.find(varNewName); } else { AstIfaceRefDType* ifaceRefp = pinArrp->subDTypep()->castIfaceRefDType(); ifaceRefp->cellp(NULL); varNewp = pinVarp->cloneTree(false); - m_modVarNameMap.insert(make_pair(varNewName, varNewp)); varNewp->name(varNewName); varNewp->origName(varNewp->origName() + "__BRA__" + cvtToStr(i) + "__KET__"); varNewp->dtypep(ifaceRefp); + m_deModVars.insert(varNewp); if (!prevp) { prevp = varNewp; } else { prevp->addNextHere(varNewp); } } - if (varNewp == NULL) { - nodep->v3fatalSrc("Module dearray failed\n"); + if (!varNewp) { + if (debug()>=9) m_deModVars.dump(); + nodep->v3fatalSrc("Module dearray failed for "<1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_inst_darray.v b/test_regress/t/t_inst_darray.v new file mode 100644 index 000000000..acf610e0a --- /dev/null +++ b/test_regress/t/t_inst_darray.v @@ -0,0 +1,85 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2017 by John Stevenson. + +typedef logic [63:0] uid_t; +typedef logic [31:0] value_t; + +interface the_intf #(parameter M = 5); + logic valid; + uid_t uid; + value_t [M-1:0] values; + + modport i( + output valid, + output uid, + output values + ); + modport t( + input valid, + input uid, + input values + ); +endinterface + +module Contemplator #( + parameter IMPL = 0, + parameter M = 5, + parameter N = 1 ) + ( + input logic clk, + the_intf.i out [N-1:0] + ); + + the_intf #(.M(M)) inp[N-1:0] (); + + DeepThought #( + .N ( N )) + ultimateAnswerer( + .src ( inp ), + .dst ( out )); +endmodule + +module DeepThought #( + parameter N = 1 ) + ( + the_intf.t src[N-1:0], + the_intf.i dst[N-1:0] + ); +endmodule + + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + + localparam M = 5; + localparam N = 1; + + the_intf #(.M(M)) out0 [N-1:0] (); + the_intf #(.M(M)) out1 [N-1:0] (); + + Contemplator #( + .IMPL ( 0 ), + .M ( M ), + .N ( N )) + contemplatorOfTheZerothKind( + .clk ( clk ), + .out ( out0 )); + + Contemplator #( + .IMPL ( 1 ), + .M ( M ), + .N ( N )) + contemplatorOfTheFirstKind( + .clk ( clk ), + .out ( out1 )); + initial begin + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule