Fix non-arrayed cells with interface arrays, bug1153.

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]
 
 

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 {
+class InstDeModVarVisitor : public AstNVisitor {
-    // Find all cells with arrays, and convert to non-arrayed
+    // 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<string,AstVar*> 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    "<<nodep<<endl);
+	    insert(nodep);
+	}
+	nodep->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    "<<nodep<<endl);
+	m_modVarNameMap.insert(make_pair(nodep->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: "<<it->first<<" -> "<<it->second<<endl;
+	}
+    }
+public:
+    // CONSTUCTORS
+    explicit InstDeModVarVisitor() {}
+    void accept(AstNodeModule* nodep) {
+	UINFO(8,"  dmMODULE    "<<nodep<<endl);
 	m_modVarNameMap.clear();
+	nodep->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<string,AstVar*> 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    "<<nodep<<endl);
+	    AstUnpackArrayDType* arrdtype = nodep->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 "<<varNewName<<"  "<<nodep<<endl);
+		if (!m_deModVars.find(varNewName)) {
+		    AstIfaceRefDType* ifaceRefp = arrdtype->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   "<<nodep<<endl);
+	// Find submodule vars
+	if (!nodep->modp()) nodep->v3fatalSrc("Unlinked");
+	m_deModVars.accept(nodep->modp());
+	//
 	if (nodep->rangep()) {
 	    m_cellRangep = nodep->rangep();
-	    UINFO(4,"  CELL   "<<nodep<<endl);
 
 	    AstVar* ifaceVarp = nodep->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  "<<newp<<endl);
 
-		// If this AstCell is actually an interface instantiation, let's ensure we also clone
+		// If this AstCell is actually an interface instantiation, also clone the IfaceRef
-		// the IfaceRef.
+		// within the same parent module as the cell
 		if (isIface) {
 		    AstUnpackArrayDType* arrdtype = ifaceVarp->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 = m_deModVars.find(varNewName);
-			varNewp = it->second;
-		    }
 		} 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) {
+		if (!varNewp) {
-		    nodep->v3fatalSrc("Module dearray failed\n");
+		    if (debug()>=9) m_deModVars.dump();
+		    nodep->v3fatalSrc("Module dearray failed for "<<AstNode::prettyName(varNewName));
 		}
 
 		// But clone the pin for each module instance

test_regress/t/t_inst_darray.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;

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