Yet another round of dotted generate fixes

git-svn-id: file://localhost/svn/verilator/trunk/verilator@856 77ca24e4-aefa-0310-84f0-b9a241c72d87

Changes

@@ -3,6 +3,12 @@ Revision history for Verilator
 The contributors that suggested a given feature are shown in [].  [by ...]
 indicates the contributor was also the author of the fix; Thanks!
 
+* Verilator 3.63*
+
+**** Fix again dotted references into generate cells.  [David Hewson]
+     Verilator no longer accepts duplicated variables inside unique
+     generate blocks as this is illegal according to the specification.
+
 * Verilator 3.630 12/19/2006
 
 **   Support $readmemb and $readmemh.  [Eugene Weber, Arthur Kahlich]

bin/verilator

@@ -1177,17 +1177,15 @@ standard; arrayed instances are named {cellName}__{instanceNumber}.  For
 example a[2] is instead a__2.  Thus you cannot use a parameter or variable
 to select the index number; expand it into a case statement manually.
 
-References into generate statements also use different names from the
+References into generate statements should match the names from the Verilog
-Verilog standard; the top level generate, any begin statement, and any
+standard; subject to converting generate-for a[#] to a__# as described
-place where there is a implied begin statement (under each generate-if or
+above.  When a dotted signal cannot be resolved, Verilator will print a
-generate-case) adds a hierarchy level named genblkI<instanceNumber>.  And
+list of known scopes to help debugging.
-each generate-for adds genforI<loopCount>.  The best bet is to comment out
+
-the references and run with --debug and look in the at the _begin.tree
+Verilator creates numbered "genblk" when a begin: name is not specified
-file, and see the hierarchy by looking at the names of all of the CELLs in
+around a block inside a generate statement.  These numbers may differ
-question.  Replace __DOT__ with . in your program.  For example most cells
+between other simulators, but the Verilog specification does not allow
-under a for loop would be something like
+users to use these names, so it should not matter.
-"genblk__DOT__genfor0__DOT__I<cellname>"; so use genblk.genfor0.I<cellname> to
-see inside that cell.
 
 =head2 Latches
 

src/V3Link.cpp

@@ -65,6 +65,7 @@ private:
     int		m_paramNum;	// Parameter number, for position based connection
     V3SymTable* m_curVarsp;	// Symbol table of variables and tasks under table we're inserting into
     V3SymTable* m_cellVarsp;	// Symbol table of variables under cell's module
+    int		m_beginNum;	// Begin block number, 0=none seen
     vector<V3SymTable*> m_delSymps;	// Symbol tables to delete
 
     //int debug() { return 9; }
@@ -131,6 +132,7 @@ private:
 	if (!m_curVarsp) nodep->v3fatalSrc("NULL");
 	m_cellVarsp = NULL;
 	m_paramNum = 0;
+	m_beginNum = 0;
 	nodep->iterateChildren(*this);
 	// Prep for next
 	m_curVarsp = NULL;
@@ -158,7 +160,7 @@ private:
 			findvarp->combineType(nodep);
 			nodep->unlinkFrBack(); nodep=NULL;
 		    } else {
-			nodep->v3error("Duplicate declaration of signal.");
+			nodep->v3error("Duplicate declaration of signal: "<<nodep->prettyName());
 			findvarp->v3error("... Location of original declaration");
 		    }
 		} else {
@@ -244,6 +246,16 @@ private:
 	// Link variables underneath blocks
 	// Remember the existing symbol table scope
 	V3SymTable* upperVarsp = m_curVarsp;
+	// Rename "genblk"s to include a number
+	// All blocks are numbered in the standard, IE we start with "genblk1" even if only one.
+	UINFO(8,"   "<<nodep<<endl);
+	if (m_idState==ID_FIND && nodep->name() == "genblk") {
+	    ++m_beginNum;
+	    nodep->name(nodep->name()+cvtToStr(m_beginNum));
+	}
+	// Recurse
+	int oldNum = m_beginNum;
+	m_beginNum = 0;
 	{
 	    // Create symbol table for the task's vars
 	    if (V3SymTable* localVarsp = nodep->userp()->castSymTable()) {
@@ -256,6 +268,7 @@ private:
 	    nodep->iterateChildren(*this);
 	}
 	m_curVarsp = upperVarsp;
+	m_beginNum = oldNum;
     }
     virtual void visit(AstNodeFTaskRef* nodep, AstNUser*) {
 	// NodeFTaskRef: Resolve its reference
@@ -399,6 +412,7 @@ public:
 	m_cellVarsp = NULL;
 	m_modp = NULL;
 	m_paramNum = 0;
+	m_beginNum = 0;
 	//
 	rootp->accept(*this);
     }

src/V3LinkResolve.cpp

@@ -55,8 +55,6 @@ private:
     AstModule*	m_modp;		// Current module
     AstNodeFTask* m_ftaskp;	// Function or task we're inside
     bool	m_setRefLvalue;	// Set VarRefs to lvalues for pin assignments
-    AstBegin*	m_beginLastp;	// Last begin block seen (not present one)
-    int		m_beginNum;	// Begin block number, 0=none seen
 
     //int debug() { return 9; }
 
@@ -72,33 +70,9 @@ private:
 	// Module: Create sim table for entire module and iterate
 	UINFO(8,"MODULE "<<nodep<<endl);
 	m_modp = nodep;
-	m_beginNum = 0;
-	m_beginLastp = NULL;
 	nodep->iterateChildren(*this);
 	m_modp = NULL;
     }
-    virtual void visit(AstBegin* nodep, AstNUser*) {
-	// Rename "genblk"s to include a number if there was more then one per block
-	// This is better then doing it at parse time, because if there's only one, it's named logically.
-	UINFO(8,"   "<<nodep<<endl);
-	++m_beginNum;
-	if (m_beginNum == 2) {  // We didn't know in time to fix the first one.  Do now
-	    if (!m_beginLastp) nodep->v3fatalSrc("Where was first begin?");
-	    m_beginLastp->name(m_beginLastp->name()+"1");
-	}
-	if (m_beginNum > 1) {  // Now us
-	    nodep->name(nodep->name()+cvtToStr(m_beginNum));
-	}
-	// Recurse
-	int oldNum = m_beginNum;
-	m_beginNum = 0;
-	m_beginLastp = NULL;
-	{
-	    nodep->iterateChildren(*this);
-	}
-	m_beginNum = oldNum;
-	m_beginLastp = nodep;  // Us, so 2nd begin can change our name
-    }
     virtual void visit(AstVar* nodep, AstNUser*) {
 	nodep->iterateChildren(*this);
 	if (nodep->arraysp() && nodep->isIO()) {
@@ -389,8 +363,6 @@ public:
 	m_ftaskp = NULL;
 	m_modp = NULL;
 	m_setRefLvalue = false;
-	m_beginNum = 0;
-	m_beginLastp = NULL;
 	//
 	rootp->accept(*this);
     }

src/V3Unroll.cpp

@@ -342,18 +342,16 @@ private:
 	nodep->iterateChildren(*this);
 	m_inBegin = lastBegin;
 
-	if (m_varModeReplace && !m_inBegin // above begin, not this one
+	if (m_varModeReplace && !m_inBegin // no upper begin, excluding this one
 	    ) {
 	    // Rename it, as otherwise we may get a conflict
 	    // V3Begin sees these DOTs and makes CellInlines for us.
 	    string nname = (string)"genfor"+cvtToStr(m_varValuep->asInt())+"__DOT__"+nodep->name();
-	    if (nodep->name() != "genblk"
+	    // Verilog seems to drop the for loop name and tack on [#]
-		&& nodep->name().find("__DOT__") == string::npos) {
+	    //nname = nodep->name() + "__BRA__" + cvtToStr(m_varValuep->asInt()) + "__KET__";
-		// Verilog seems to drop the for loop name and tack on [#]
+	    // However we don't parse [#]'s correctly, so just use __ for now.
-		//nname = nodep->name() + "__BRA__" + cvtToStr(m_varValuep->asInt()) + "__KET__";
+	    // (Fixing parsing to allow hardcoded numbers then leads to shift/reduce conflicts.)
-		// However we don't parse [#]'s correctly, so just use __ for now.
+	    nname = nodep->name() + "__" + cvtToStr(m_varValuep->asInt());
-		nname = nodep->name() + "__" + cvtToStr(m_varValuep->asInt());
-	    }
 	    //UINFO(8,"   Rename begin "<<nname<<" "<<nodep<<endl);
 	    nodep->name(nname);
 	}

src/verilog.y

@@ -442,7 +442,7 @@ genItemBlock:	genItem					{ $$ = new AstBegin(CRELINE(),"genblk",$1); }
 	|	genItemBegin				{ $$ = $1; }
 	;
 
-genTopBlock:	genItemList				{ $$ = new AstBegin(CRELINE(),"genblk",$1); }
+genTopBlock:	genItemList				{ $$ = $1; }
 	|	genItemBegin				{ $$ = $1; }
 	;
 

test_regress/t/t_gen_for.v

@@ -88,14 +88,13 @@ module paramed (/*AUTOARG*/
       // No else
    endgenerate
 
+   genvar 	      i;
    generate
-      genvar 	      i;
       // Empty loop body
       for (i=0; i<3; i=i+1) begin end
    endgenerate
 
    generate
-      genvar 	      i;
       if (MODE==0) begin
 	 // Flip bitorder, direct assign method
 	 for (i=0; i<WIDTH; i=i+1) begin
@@ -105,12 +104,11 @@ module paramed (/*AUTOARG*/
       else if (MODE==1) begin
 	 // Flip using instantiation
 	 for (i=0; i<WIDTH; i=i+1) begin
+	    integer from = WIDTH-i-1;
 	    if (i==0) begin	// Test if's within a for
-	       integer from = WIDTH-i-1;
 	       mbuf m0 (.q(out[i]), .a(in[from]));
 	    end
 	    else begin
-	       integer from = WIDTH-i-1;
 	       mbuf ma (.q(out[i]), .a(in[from]));
 	    end
 	 end

test_regress/t/t_gen_intdot.v

@@ -13,7 +13,7 @@ module t (/*AUTOARG*/
 
    wire 	out;
    reg 		in;
-   
+
    Genit g (.clk(clk), .value(in), .result(out));
 
    always @ (posedge clk) begin
@@ -56,7 +56,7 @@ module Generate (clk, value, result);
    reg Internal;
 
    assign result = Internal ^ clk;
-  
+
    always @(posedge clk)
      Internal <= #1 value;
 endmodule
@@ -99,7 +99,7 @@ module Genit (clk, value, result);
 `endif
 
 `ifdef verilator
-   wire Result2 = t.g.genblk.foo__0.tt.gen.Internal;
+   wire Result2 = t.g.foo__0.tt.gen.Internal;
 `else
    wire Result2 = t.g.foo[0].tt.gen.Internal;  // Works - Do not change!
 `endif

test_regress/t/t_gen_intdot2.pl

@@ -0,0 +1,18 @@
+#!/usr/bin/perl
+if (!$::Driver) { use FindBin; exec("./driver.pl", @ARGV, $0); die; }
+# $Id: t_gen_intdot.pl 837 2006-12-12 18:25:33Z wsnyder $
+# 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
+# General Public License or the Perl Artistic License.
+
+compile (
+	 );
+
+execute (
+	 check_finished=>1,
+     );
+
+ok(1);
+1;

test_regress/t/t_gen_intdot2.v

@@ -0,0 +1,178 @@
+// $Id: t_gen_intdot.v 840 2006-12-18 18:14:53Z wsnyder $
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2003-2006 by Wilson Snyder.
+
+module t (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+   input clk;
+   integer 	cyc=0;
+
+   reg 		check;
+   initial check = 1'b0;
+   Genit g (.clk(clk), .check(check));
+
+   always @ (posedge clk) begin
+      //$write("[%0t] cyc==%0d %x %x\n",$time, cyc, check, out);
+      cyc <= cyc + 1;
+      if (cyc==0) begin
+	 // Setup
+	 check <= 1'b0;
+      end
+      else if (cyc==1) begin
+	 check <= 1'b1;
+      end
+      else if (cyc==9) begin
+	 $write("*-* All Finished *-*\n");
+	 $finish;
+      end
+   end
+
+//`define WAVES
+`ifdef WAVES
+   initial begin
+      $dumpfile("obj_dir/t_gen_intdot.vcd");
+      $dumpvars(12, t);
+   end
+`endif
+
+endmodule
+
+module One;
+   wire one = 1'b1;
+endmodule
+
+module Genit (
+    input clk,
+    input check);
+
+   // ARRAY
+   One cellarray1[1:0] ();	//cellarray[0..1][0..1]
+`ifdef verilator
+   always @ (posedge clk) if (cellarray1__0.one !== 1'b1) $stop;
+   always @ (posedge clk) if (cellarray1__1.one !== 1'b1) $stop;
+`else
+   always @ (posedge clk) if (cellarray1[0].one !== 1'b1) $stop;
+   always @ (posedge clk) if (cellarray1[1].one !== 1'b1) $stop;
+`endif
+
+   // IF
+   generate
+      // genblk1 refers to the if's name, not the "generate" itself.
+      if (1'b1) // IMPLIED begin: genblk1
+	One ifcell1(); // genblk1.ifcell1
+      else
+	One ifcell1(); // genblk1.ifcell1
+   endgenerate
+   // On compliant simulators "Implicit name" not allowed here; IE we can't use "genblk1" etc
+`ifdef verilator
+   always @ (posedge clk) if (genblk1.ifcell1.one !== 1'b1) $stop;
+//`else // NOT SUPPORTED accoring to spec - generic block references
+`endif
+
+   generate
+      begin : namedif2
+	 if (1'b1)
+	   One ifcell2();   // namedif2.genblk1.ifcell2
+      end
+   endgenerate
+`ifdef verilator
+   always @ (posedge clk) if (namedif2.genblk1.ifcell2.one !== 1'b1) $stop;
+//`else // NOT SUPPORTED accoring to spec - generic block references
+`endif
+
+   generate
+      if (1'b1)
+	begin : namedif3
+	   One ifcell3();  // namedif3.ifcell3
+	end
+   endgenerate
+   always @ (posedge clk) if (namedif3.ifcell3.one !== 1'b1) $stop;
+
+   // CASE
+   generate
+      case (1'b1)
+	1'b1 :
+	  One casecell10();	// genblk3.casecell10
+      endcase
+   endgenerate
+`ifdef verilator
+   always @ (posedge clk) if (genblk3.casecell10.one !== 1'b1) $stop;
+//`else // NOT SUPPORTED accoring to spec - generic block references
+`endif
+
+   generate
+      case (1'b1)
+	1'b1 : begin : namedcase11
+	  One casecell11();
+	end
+      endcase
+   endgenerate
+   always @ (posedge clk) if (namedcase11.casecell11.one !== 1'b1) $stop;
+
+   genvar i;
+   genvar j;
+
+   // IF
+   generate
+      for (i = 0; i < 2; i = i + 1)
+	One cellfor20 ();	// genblk5[0..1].cellfor20
+   endgenerate
+`ifdef verilator
+   always @ (posedge clk) if (genblk4__0.cellfor20.one !== 1'b1) $stop;
+   always @ (posedge clk) if (genblk4__1.cellfor20.one !== 1'b1) $stop;
+//`else // NOT SUPPORTED accoring to spec - generic block references
+`endif
+
+   // COMBO
+   generate
+      for (i = 0; i < 2; i = i + 1)
+	begin : namedfor21
+	   One cellfor21 ();	// namedfor21[0..1].cellfor21
+	end
+   endgenerate
+`ifdef verilator
+   always @ (posedge clk) if (namedfor21__0.cellfor21.one !== 1'b1) $stop;
+   always @ (posedge clk) if (namedfor21__1.cellfor21.one !== 1'b1) $stop;
+`else
+   always @ (posedge clk) if (namedfor21[0].cellfor21.one !== 1'b1) $stop;
+   always @ (posedge clk) if (namedfor21[1].cellfor21.one !== 1'b1) $stop;
+`endif
+
+   generate
+      for (i = 0; i < 2; i = i + 1)
+	begin : namedfor30
+	   for (j = 0; j < 2; j = j + 1)
+	     begin : forb30
+		if (j == 0)
+		  begin : forif30
+		     One cellfor30a ();  // namedfor30[0..1].forb30[0].forif30.cellfor30a
+		  end
+		else
+`ifdef verilator
+		  begin : forif30b
+`else
+		  begin : forif30 // forif30 seems to work on some simulators, not verilator yet
+`endif
+		     One cellfor30b ();  // namedfor30[0..1].forb30[1].forif30.cellfor30b
+		  end
+	     end
+	end
+   endgenerate
+`ifdef verilator
+   always @ (posedge clk) if (namedfor30__0.forb30__0.forif30.cellfor30a.one !== 1'b1) $stop;
+   always @ (posedge clk) if (namedfor30__0.forb30__1.forif30b.cellfor30b.one !== 1'b1) $stop;
+   always @ (posedge clk) if (namedfor30__1.forb30__0.forif30.cellfor30a.one !== 1'b1) $stop;
+   always @ (posedge clk) if (namedfor30__1.forb30__1.forif30b.cellfor30b.one !== 1'b1) $stop;
+`else
+   always @ (posedge clk) if (namedfor30[0].forb30[0].forif30.cellfor30a.one !== 1'b1) $stop;
+   always @ (posedge clk) if (namedfor30[0].forb30[1].forif30.cellfor30b.one !== 1'b1) $stop;
+   always @ (posedge clk) if (namedfor30[1].forb30[0].forif30.cellfor30a.one !== 1'b1) $stop;
+   always @ (posedge clk) if (namedfor30[1].forb30[1].forif30.cellfor30b.one !== 1'b1) $stop;
+`endif
+
+endmodule
+