diff --git a/Changes b/Changes index a7271be19..ebbab7cc7 100644 --- a/Changes +++ b/Changes @@ -15,6 +15,8 @@ The contributors that suggested a given feature are shown in []. Thanks! **** Fix internal error on initializing parameter array, bug1131. [Jie Xu] +**** Fix internal error on interface arrays, bug1135. [John Stevenson] + * Verilator 3.900 2017-01-15 diff --git a/src/V3Inst.cpp b/src/V3Inst.cpp index 88e824996..737090dde 100644 --- a/src/V3Inst.cpp +++ b/src/V3Inst.cpp @@ -143,7 +143,7 @@ private: } public: // CONSTUCTORS - explicit InstVisitor(AstNode* nodep) { + explicit InstVisitor(AstNetlist* nodep) { m_modp=NULL; m_cellp=NULL; // @@ -162,6 +162,9 @@ private: int m_instNum; // Current instantiation number int m_instLsb; // Current instantiation number + typedef map VarNameMap; + VarNameMap m_modVarNameMap; // Per module, name of cloned variables + static int debug() { static int level = -1; if (VL_UNLIKELY(level < 0)) level = v3Global.opt.debugSrcLevel(__FILE__); @@ -169,6 +172,11 @@ private: } // VISITORS + virtual void visit(AstNodeModule* nodep) { + m_modVarNameMap.clear(); + nodep->iterateChildren(*this); + } + virtual void visit(AstCell* nodep) { if (nodep->rangep()) { m_cellRangep = nodep->rangep(); @@ -232,21 +240,27 @@ private: AstUnpackArrayDType* arrdtype = nodep->dtypep()->castUnpackArrayDType(); AstNode* prev = NULL; for (int i = arrdtype->lsb(); i <= arrdtype->msb(); ++i) { - AstVar* varNewp = nodep->cloneTree(false); AstIfaceRefDType* ifaceRefp = arrdtype->subDTypep()->castIfaceRefDType()->cloneTree(false); arrdtype->addNextHere(ifaceRefp); ifaceRefp->cellp(NULL); - varNewp->name(varNewp->name() + "__BRA__" + cvtToStr(i) + "__KET__"); - varNewp->origName(varNewp->origName() + "__BRA__" + cvtToStr(i) + "__KET__"); - varNewp->dtypep(ifaceRefp); - if (!prev) { - prev = varNewp; - } else { - prev->addNextHere(varNewp); + + 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); + } } } - nodep->addNextHere(prev); - if (debug()==9) { prev->dumpTree(cout, "newintf: "); cout << endl; } + if (prev) nodep->addNextHere(prev); + if (prev && debug()==9) { prev->dumpTree(cout, "newintf: "); cout << endl; } } nodep->iterateChildren(*this); } @@ -303,7 +317,7 @@ private: } string index = AstNode::encodeNumber(constp->toSInt()); AstVarRef* varrefp = arrselp->lhsp()->castVarRef(); - AstVarXRef* newp = new AstVarXRef(nodep->fileline(),varrefp->name () + "__BRA__" + index + "__KET__", "", true); + AstVarXRef* newp = new AstVarXRef(nodep->fileline(), varrefp->name()+"__BRA__"+index+"__KET__", "", true); newp->dtypep(nodep->modVarp()->dtypep()); newp->packagep(varrefp->packagep()); arrselp->addNextHere(newp); @@ -319,16 +333,25 @@ private: // Clone the var referenced by the pin, and clone each var referenced by the varref // Clone pin varp: for (int i = pinArrp->lsb(); i <= pinArrp->msb(); ++i) { - AstVar* varNewp = pinVarp->cloneTree(false); AstIfaceRefDType* ifaceRefp = pinArrp->subDTypep()->castIfaceRefDType(); ifaceRefp->cellp(NULL); - varNewp->name(varNewp->name() + "__BRA__" + cvtToStr(i) + "__KET__"); - varNewp->origName(varNewp->origName() + "__BRA__" + cvtToStr(i) + "__KET__"); - varNewp->dtypep(ifaceRefp); - if (!prevp) { - prevp = varNewp; + + string varNewName = pinVarp->name() + "__BRA__" + cvtToStr(i) + "__KET__"; + VarNameMap::iterator it = m_modVarNameMap.find(varNewName); + AstVar* varNewp; + if (it != m_modVarNameMap.end()) { + varNewp = it->second; } else { - prevp->addNextHere(varNewp); + 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); + if (!prevp) { + prevp = varNewp; + } else { + prevp->addNextHere(varNewp); + } } // Now also clone the pin itself and update its varref AstPin* newp = nodep->cloneTree(false); @@ -336,7 +359,7 @@ private: newp->name(newp->name() + "__BRA__" + cvtToStr(i) + "__KET__"); // And replace exprp with a new varxref AstVarRef* varrefp = newp->exprp()->castVarRef(); - string newname = varrefp->name () + "__BRA__" + cvtToStr(i) + "__KET__"; + string newname = varrefp->name() + "__BRA__" + cvtToStr(i) + "__KET__"; AstVarXRef* newVarXRefp = new AstVarXRef (nodep->fileline(), newname, "", true); newVarXRefp->varp(newp->modVarp()); newVarXRefp->dtypep(newp->modVarp()->dtypep()); @@ -348,11 +371,12 @@ private: prevPinp->addNextHere(newp); } } - pinVarp->replaceWith(prevp); + if (prevp) { + pinVarp->replaceWith(prevp); + pushDeletep(pinVarp); + } // else pinVarp already unlinked when another instance did this step nodep->replaceWith(prevPinp); - pushDeletep(pinVarp); pushDeletep(nodep); - } } @@ -365,7 +389,7 @@ private: } public: // CONSTUCTORS - explicit InstDeVisitor(AstNode* nodep) { + explicit InstDeVisitor(AstNetlist* nodep) { m_cellRangep=NULL; m_instNum=0; m_instLsb=0; diff --git a/test_regress/t/t_interface_arraymux.pl b/test_regress/t/t_interface_arraymux.pl new file mode 100755 index 000000000..086fa8d0c --- /dev/null +++ b/test_regress/t/t_interface_arraymux.pl @@ -0,0 +1,19 @@ +#!/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 ( + verilator_flags2 => ["--lint-only"], + fails=>0, + verilator_make_gcc => 0, + make_top_shell => 0, + make_main => 0, + ); + +ok(1); +1; diff --git a/test_regress/t/t_interface_arraymux.v b/test_regress/t/t_interface_arraymux.v new file mode 100644 index 000000000..a77232702 --- /dev/null +++ b/test_regress/t/t_interface_arraymux.v @@ -0,0 +1,105 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2017 by John Stevenson. + +package pkg; + typedef logic [31:0] unique_id_t; + typedef struct packed { + unique_id_t foo; + } inner_thing_t; + typedef struct packed { + inner_thing_t bar; + inner_thing_t baz; + } outer_thing_t; + +endpackage + +import pkg::*; + +interface the_intf + #(parameter M=5); + outer_thing_t [M-1:0] things; + logic valid; + modport i ( + output things, + output valid); + modport t ( + input things, + input valid); +endinterface + +module ThingMuxOH + #( + parameter NTHINGS = 1, + parameter M = 5 ) + ( + input logic [NTHINGS-1:0] select_oh, + the_intf.t things_in [NTHINGS-1:0], + the_intf.i thing_out + ); +endmodule + +module Thinker + #( + parameter M = 5, + parameter N = 2) + ( + input logic clk, + input logic reset, + input unique_id_t uids[0:N-1], + the_intf.t thing_inp, + the_intf.i thing_out + ); + + the_intf #(.M(M)) curr_things [N-1:0] (); + the_intf #(.M(M)) prev_things [N-1:0] (); + the_intf #(.M(M)) curr_thing (); + the_intf #(.M(M)) prev_thing (); + + logic [N-1:0] select_oh; + + // 1st mux: + ThingMuxOH #( + .NTHINGS ( N ), + .M ( M )) + curr_thing_mux( + .select_oh( select_oh ), + .things_in( curr_things ), + .thing_out( curr_thing )); + + // 2nd mux, comment this out and no problem: + ThingMuxOH #( + .NTHINGS ( N ), + .M ( M )) + prev_thing_mux( + .select_oh( select_oh ), + .things_in( prev_things ), + .thing_out( prev_thing )); + +endmodule + +module t + ( + input logic clk, + input logic reset + ); + + localparam M = 5; + localparam N = 2; + + unique_id_t uids[0:N-1]; + + the_intf #(.M(M)) thing_inp(); + the_intf #(.M(M)) thing_out(); + + Thinker #( + .M ( M ), + .N ( N )) + thinker( + .clk ( clk ), + .reset ( reset ), + .uids ( uids ), + .thing_inp( thing_inp ), + .thing_out( thing_out )); +endmodule