Fix internal error on interface arrays, bug1135.

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
 

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<string,AstVar*> 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__");
+		string varNewName = nodep->name() + "__BRA__" + cvtToStr(i) + "__KET__";
-		varNewp->dtypep(ifaceRefp);
+		VarNameMap::iterator it = m_modVarNameMap.find(varNewName);
-		if (!prev) {
+		if (it == m_modVarNameMap.end()) {
-		    prev = varNewp;
+		    AstVar* varNewp = nodep->cloneTree(false);
-		} else {
+		    m_modVarNameMap.insert(make_pair(varNewName, varNewp));
-		    prev->addNextHere(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 (prev) nodep->addNextHere(prev);
-	    if (debug()==9) { prev->dumpTree(cout, "newintf: "); cout << endl; }
+	    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__");
+		string varNewName = pinVarp->name() + "__BRA__" + cvtToStr(i) + "__KET__";
-		varNewp->dtypep(ifaceRefp);
+		VarNameMap::iterator it = m_modVarNameMap.find(varNewName);
-		if (!prevp) {
+		AstVar* varNewp;
-		    prevp = 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;

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;

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