Fix genblk naming to match IEEE (#2686).

Changes

@@ -19,6 +19,8 @@ The contributors that suggested a given feature are shown in []. Thanks!
 
 ****  Fix showing reference locations for BLKANDNBLK (#2170). [Yuri Victorovich]
 
+****  Fix genblk naming to match IEEE (#2686). [tinshark]
+
 
 * Verilator 4.106 2020-12-02
 

src/V3LinkDot.cpp

@@ -718,7 +718,6 @@ class LinkDotFindVisitor final : public AstNVisitor {
     AstNodeFTask* m_ftaskp = nullptr;  // Current function/task
     bool m_inRecursion = false;  // Inside a recursive module
     int m_paramNum = 0;  // Parameter number, for position based connection
-    int m_blockNum = 0;  // Begin block number, 0=none seen
     bool m_explicitNew = false;  // Hit a "new" function
     int m_modBlockNum = 0;  // Begin block number in module, 0=none seen
     int m_modWithNum = 0;  // With block number, 0=none seen
@@ -783,7 +782,6 @@ class LinkDotFindVisitor final : public AstNVisitor {
         VL_RESTORER(m_modSymp);
         VL_RESTORER(m_curSymp);
         VL_RESTORER(m_paramNum);
-        VL_RESTORER(m_blockNum);
         VL_RESTORER(m_modBlockNum);
         VL_RESTORER(m_modWithNum);
         if (doit && nodep->user2()) {
@@ -810,7 +808,6 @@ class LinkDotFindVisitor final : public AstNVisitor {
             }
             //
             m_paramNum = 0;
-            m_blockNum = 0;
             m_modBlockNum = 0;
             m_modWithNum = 0;
             // m_modSymp/m_curSymp for non-packages set by AstCell above this module
@@ -845,7 +842,6 @@ class LinkDotFindVisitor final : public AstNVisitor {
         VL_RESTORER(m_modSymp);
         VL_RESTORER(m_curSymp);
         VL_RESTORER(m_paramNum);
-        VL_RESTORER(m_blockNum);
         VL_RESTORER(m_modBlockNum);
         VL_RESTORER(m_modWithNum);
         {
@@ -859,7 +855,6 @@ class LinkDotFindVisitor final : public AstNVisitor {
             UINFO(9, "New module scope " << m_curSymp << endl);
             //
             m_paramNum = 0;
-            m_blockNum = 0;
             m_modBlockNum = 0;
             m_modWithNum = 0;
             m_explicitNew = false;
@@ -937,14 +932,6 @@ class LinkDotFindVisitor final : public AstNVisitor {
     }
     virtual void visit(AstNodeBlock* nodep) override {
         UINFO(5, "   " << nodep << endl);
-        // Rename "genblk"s to include a number
-        if (m_statep->forPrimary() && !nodep->user4SetOnce()) {
-            if (nodep->name() == "genblk") {
-                ++m_blockNum;
-                nodep->name(nodep->name() + cvtToStr(m_blockNum));
-            }
-        }
-        // All blocks are numbered in the standard, IE we start with "genblk1" even if only one.
         if (nodep->name() == "" && nodep->unnamed()) {
             // Unnamed blocks are only important when they contain var
             // decls, so search for them. (Otherwise adding all the
@@ -962,12 +949,10 @@ class LinkDotFindVisitor final : public AstNVisitor {
         if (nodep->name() == "") {
             iterateChildren(nodep);
         } else {
-            VL_RESTORER(m_blockNum);
             VL_RESTORER(m_blockp);
             VL_RESTORER(m_curSymp);
             VSymEnt* const oldCurSymp = m_curSymp;
             {
-                m_blockNum = 0;
                 m_blockp = nodep;
                 m_curSymp
                     = m_statep->insertBlock(m_curSymp, nodep->name(), nodep, m_classOrPackagep);

src/V3LinkParse.cpp

@@ -57,6 +57,8 @@ private:
     AstNodeModule* m_modp = nullptr;  // Current module
     AstNodeFTask* m_ftaskp = nullptr;  // Current task
     AstNodeDType* m_dtypep = nullptr;  // Current data type
+    int m_genblkAbove = 0;  // Begin block number of if/case/for above
+    int m_genblkNum = 0;  // Begin block number, 0=none seen
     VLifetime m_lifetime = VLifetime::STATIC;  // Propagating lifetime
 
     // METHODS
@@ -495,12 +497,16 @@ private:
         V3Config::applyModule(nodep);
 
         VL_RESTORER(m_modp);
+        VL_RESTORER(m_genblkAbove);
+        VL_RESTORER(m_genblkNum);
         VL_RESTORER(m_lifetime);
         {
             // Module: Create sim table for entire module and iterate
             cleanFileline(nodep);
             //
             m_modp = nodep;
+            m_genblkAbove = 0;
+            m_genblkNum = 0;
             m_valueModp = nodep;
             m_lifetime = nodep->lifetime();
             if (m_lifetime.isNone()) {
@@ -538,13 +544,45 @@ private:
                              || VN_IS(nodep->stmtsp(), GenCase))  // Has an if/case
                          && !nodep->stmtsp()->nextp());  // Has only one item
         // It's not FOR(BEGIN(...)) but we earlier changed it to BEGIN(FOR(...))
-        if (nodep->genforp() && nodep->name() == "") {
+        if (nodep->genforp()) {
-            nodep->name("genblk");
+            ++m_genblkNum;
-        } else if (nodep->generate() && nodep->name() == ""
+            if (nodep->name() == "") nodep->name("genblk" + cvtToStr(m_genblkNum));
-                   && (VN_IS(backp, CaseItem) || VN_IS(backp, GenIf)) && !nestedIf) {
-            nodep->name("genblk");
         }
+        if (nodep->generate() && nodep->name() == ""
+                   && (VN_IS(backp, CaseItem) || VN_IS(backp, GenIf)) && !nestedIf) {
+            nodep->name("genblk" + cvtToStr(m_genblkAbove));
+        }
+        if (nodep->name() != "") {
+            VL_RESTORER(m_genblkAbove);
+            VL_RESTORER(m_genblkNum);
+            m_genblkAbove = 0;
+            m_genblkNum = 0;
             iterateChildren(nodep);
+        } else {
+            iterateChildren(nodep);
+        }
+    }
+    virtual void visit(AstGenCase* nodep) override {
+        ++m_genblkNum;
+        cleanFileline(nodep);
+        {
+            VL_RESTORER(m_genblkAbove);
+            VL_RESTORER(m_genblkNum);
+            m_genblkAbove = m_genblkNum;
+            m_genblkNum = 0;
+            iterateChildren(nodep);
+        }
+    }
+    virtual void visit(AstGenIf* nodep) override {
+        ++m_genblkNum;
+        cleanFileline(nodep);
+        {
+            VL_RESTORER(m_genblkAbove);
+            VL_RESTORER(m_genblkNum);
+            m_genblkAbove = m_genblkNum;
+            m_genblkNum = 0;
+            iterateChildren(nodep);
+        }
     }
     virtual void visit(AstCase* nodep) override {
         V3Config::applyCase(nodep);

src/verilog.y

@@ -2465,7 +2465,8 @@ loop_generate_construct<nodep>:	// ==IEEE: loop_generate_construct
 			{ // Convert BEGIN(...) to BEGIN(GENFOR(...)), as we need the BEGIN to hide the local genvar
 			  AstBegin* lowerBegp = VN_CAST($9, Begin);
 			  UASSERT_OBJ(!($9 && !lowerBegp), $9, "Child of GENFOR should have been begin");
-			  if (!lowerBegp) lowerBegp = new AstBegin($1, "genblk", nullptr, true, true);  // Empty body
+
+			  if (!lowerBegp) lowerBegp = new AstBegin($1, "", nullptr, true, false);  // Empty body
 			  AstNode* lowerNoBegp = lowerBegp->stmtsp();
 			  if (lowerNoBegp) lowerNoBegp->unlinkFrBackWithNext();
 			  //
@@ -2479,7 +2480,7 @@ loop_generate_construct<nodep>:	// ==IEEE: loop_generate_construct
 			  }
 			  // Statements are under 'genforp' as cells under this
 			  // for loop won't get an extra layer of hierarchy tacked on
-			  blkp->addGenforp(new AstGenFor($1,initp,$5,$7,lowerNoBegp));
+			  blkp->addGenforp(new AstGenFor($1, initp, $5, $7, lowerNoBegp));
 			  $$ = blkp;
 			  VL_DO_DANGLING(lowerBegp->deleteTree(), lowerBegp);
 			}

test_regress/t/t_gen_genblk.out

@@ -1,6 +1,50 @@
-015: exp=top.t.show0 got=top.t.show0
+021: got=top.t.direct_ignored.show1
-019: exp=top.t.genblk1.show1 got=top.t.genblk1.show1
+023: got=top.t.direct_ignored.genblk1.show2 exp=1 gennum=1 
-023: exp=top.t.genblk2.show2 got=top.t.genblk2.show2
+
-028: exp=top.t.genblk3.genblk1.show3 got=top.t.genblk3.genblk1.show3
+030: got=top.t.empty_DISAGREE.genblk1.show2 exp=0 gennum=1  <ignored>
-034: exp=top.t.x1.x3.show4 got=top.t.x1.x3.show4
+
+037: got=top.t.empty_named_DISAGREE.genblk1.show2 exp=0 gennum=1  <ignored>
+
+043: got=top.t.unnamed_counts.show1
+046: got=top.t.unnamed_counts.genblk1.show2 exp=0 gennum=1  <ignored>
+
+052: got=top.t.named_counts.named.show1
+055: got=top.t.named_counts.genblk1.show2 exp=0 gennum=1  <ignored>
+
+061: got=top.t.if_direct_counts.genblk1.show1
+063: got=top.t.if_direct_counts.genblk2.show2 exp=2 gennum=2 
+
+069: got=top.t.if_begin_counts.genblk1.show1
+071: got=top.t.if_begin_counts.genblk2.show2 exp=2 gennum=2 
+
+076: got=top.t.if_named_counts.named.show1
+078: got=top.t.if_named_counts.named.subnamed.show1s
+082: got=top.t.if_named_counts.genblk2.show2 exp=2 gennum=2 
+
+089: got=top.t.begin_if_counts.genblk1.show1
+092: got=top.t.begin_if_counts.genblk2.show2 exp=2 gennum=2 
+
+099: got=top.t.for_empty_counts.genblk2.show2 exp=0 gennum=2  <ignored>
+
+104: got=top.t.for_direct_counts.genblk1[0].show1
+106: got=top.t.for_direct_counts.genblk2.show2 exp=2 gennum=2 
+
+111: got=top.t.for_named_counts.fornamed[0].show1
+114: got=top.t.for_named_counts.genblk2.show2 exp=2 gennum=2 
+
+119: got=top.t.for_begin_counts.genblk1[0].show1
+122: got=top.t.for_begin_counts.genblk2.show2 exp=2 gennum=2 
+
+132: got=top.t.if_if.genblk1.genblk1.show1
+136: got=top.t.if_if.genblk2.show2 exp=2 gennum=2 
+
+142: got=top.t.case_direct.genblk1.show1
+146: got=top.t.case_direct.genblk2.show2 exp=2 gennum=2 
+
+152: got=top.t.case_begin_counts.genblk1.show1
+156: got=top.t.case_begin_counts.genblk2.show2 exp=2 gennum=2 
+
+162: got=top.t.case_named_counts.subnamed.show1
+166: got=top.t.case_named_counts.genblk2.show2 exp=2 gennum=2 
+
 *-* All Finished *-*

test_regress/t/t_gen_genblk.pl

@@ -10,6 +10,8 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di
 
 scenarios(simulator => 1);
 
+$Self->{sim_time} = 11000;
+
 compile(
     );
 

test_regress/t/t_gen_genblk.v

@@ -3,6 +3,11 @@
 // any use, without warranty, 2020 by Wilson Snyder.
 // SPDX-License-Identifier: CC0-1.0
 
+`define CONCAT(a,b) a``b
+`define SHOW_LINED `CONCAT(show, `__LINE__)
+
+bit fails;
+
 module t (/*AUTOARG*/
    // Inputs
    clk, reset_l
@@ -12,45 +17,209 @@ module t (/*AUTOARG*/
    input        reset_l;
 
    generate
-      show #(`__LINE__, "top.t.show0") show0();
+      begin : direct_ignored
+         show #(`__LINE__) show1();
 
+         if (1) begin check #(`__LINE__, 1) show2(); end
+      end
+
+      begin : empty_DISAGREE
+         // DISAGREEMENT: if empty unnamed begin/end counts
+         begin end
+
+         if (1) begin check #(`__LINE__, 0) show2(); end
+      end
+
+      begin : empty_named_DISAGREE
+         // DISAGREEMENT: if empty named begin/end counts
+         begin : empty_inside_named end
+
+         if (1) begin check #(`__LINE__, 0) show2(); end
+      end
+
+      begin : unnamed_counts
+         // DISAGREEMENT: if unnamed begin/end counts
+         begin
+            show #(`__LINE__) show1();
+         end
+
+         if (1) begin check #(`__LINE__, 0) show2(); end
+      end
+
+      begin : named_counts
+         // DISAGREEMENT: if named begin/end counts
+         begin : named
+            show #(`__LINE__) show1();
+         end
+
+         if (1) begin check #(`__LINE__, 0) show2(); end
+      end
+
+      begin : if_direct_counts
          if (0) ;
          else if (0) ;
-      else if (1) show #(`__LINE__, "top.t.genblk1.show1") show1();
+         else if (1) show #(`__LINE__) show1();
 
+         if (1) begin check #(`__LINE__, 2) show2(); end
+      end
+
+      begin : if_begin_counts
          if (0) begin end
-      else if (0) begin end
+         else if (0) begin show #(`__LINE__) show1_NOT(); end
-      else if (1) begin show #(`__LINE__, "top.t.genblk2.show2") show2(); end
+         else if (1) begin show #(`__LINE__) show1(); end
 
+         if (1) begin check #(`__LINE__, 2) show2(); end
+      end
+
+      begin : if_named_counts
+         if (1) begin : named
+            show #(`__LINE__) show1();
+            if (1) begin : subnamed
+               show #(`__LINE__) show1s();
+            end
+         end
+
+         if (1) begin check #(`__LINE__, 2) show2(); end
+      end
+
+      begin : begin_if_counts
+         begin
+            if (0) begin end
+            else if (0) begin show #(`__LINE__) show1_NOT(); end
+            else if (1) begin show #(`__LINE__) show1(); end
+         end
+         // DISAGREEMENT: this could be genblk01
+         if (1) begin check #(`__LINE__, 2) show2(); end
+      end
+
+      begin : for_empty_counts
+         // DISAGREEMENT: if empty genfor counts
+         for (genvar g = 0; g < 1; ++g) ;
+
+         if (1) begin check #(`__LINE__, 0) show2(); end
+      end
+
+      begin : for_direct_counts
+         for (genvar g = 0; g < 1; ++g)
+            show #(`__LINE__) show1();
+
+         if (1) begin check #(`__LINE__, 2) show2(); end
+      end
+
+      begin : for_named_counts
+         for (genvar g = 0; g < 1; ++g) begin : fornamed
+            show #(`__LINE__) show1();
+         end
+
+         if (1) begin check #(`__LINE__, 2) show2(); end
+      end
+
+      begin : for_begin_counts
+         for (genvar g = 0; g < 1; ++g) begin
+            show #(`__LINE__) show1();
+         end
+
+         if (1) begin check #(`__LINE__, 2) show2(); end
+      end
+
+      begin : if_if
          if (0) ;
+         else if (0) begin : namedb
+         end
          else begin
             if (0) begin end
-         else if (1) begin show #(`__LINE__, "top.t.genblk3.genblk1.show3") show3(); end
+            else if (1) begin
+               show #(`__LINE__) show1();
+            end
          end
 
-      if (0) ;
+         if (1) begin check #(`__LINE__, 2) show2(); end
-      else begin : x1
-         if (0) begin : x2 end
-         else if (1) begin : x3 show #(`__LINE__, "top.t.x1.x3.show4") show4(); end
       end
+
+      begin : case_direct
+         case (1)
+           0 : show #(`__LINE__) show1a_NOT();
+           1 : show #(`__LINE__) show1();
+           2 : show #(`__LINE__) show1c_NOT();
+         endcase
+
+         if (1) begin check #(`__LINE__, 2) show2(); end
+      end
+
+      begin : case_begin_counts
+         case (1)
+           0 : begin show #(`__LINE__) show1a_NOT(); end
+           1 : begin show #(`__LINE__) show1(); end
+           2 : begin show #(`__LINE__) show1c_NOT(); end
+         endcase
+
+         if (1) begin check #(`__LINE__, 2) show2(); end
+      end
+
+      begin : case_named_counts
+         case (1)
+           0 : begin : subnamed show #(`__LINE__) show1a_NOT(); end
+           1 : begin : subnamed show #(`__LINE__) show1(); end
+           2 : begin : subnamed show #(`__LINE__) show1c_NOT(); end
+         endcase
+
+         if (1) begin check #(`__LINE__, 2) show2(); end
+      end
+
    endgenerate
 
    int cyc;
 
    always @ (posedge clk) begin
       cyc <= cyc + 1;
-      if (cyc == 99) begin
+      if (cyc == 999) begin
-         $write("*-* All Finished *-*\n");
+         if (fails) $stop;
+         else $write("*-* All Finished *-*\n");
          $finish;
       end
 
    end
 endmodule
 
-module show #(parameter LINE=0, parameter string EXPT) ();
+module show #(parameter LINE=0) ();
+   // Each instance compares on unique cycle based on line number
+   // so we get deterministic ordering (versus using an initial)
    always @ (posedge t.clk) begin
       if (t.cyc == LINE) begin
-         $display("%03d: exp=%s got=%m", LINE, EXPT);
+         $display("%03d: got=%m", LINE);
+      end
+   end
+endmodule
+
+module check #(parameter LINE=0, parameter EXP=0) ();
+   string mod;
+   int    gennum;
+   int    pos;
+
+   always @ (posedge t.clk) begin
+      if (t.cyc == LINE) begin
+         mod = $sformatf("%m");
+
+         gennum = 0;
+         for (int pos = 0; pos < mod.len(); ++pos) begin
+            if (mod.substr(pos, pos+5) == "genblk") begin
+               pos += 6;
+               // verilator lint_off WIDTH
+               gennum = mod[pos] - "0";
+               // verilator lint_on WIDTH
+               break;
+            end
+         end
+
+         $write("%03d: got=%s exp=%0d gennum=%0d ", LINE, mod, EXP, gennum);
+         if (EXP == 0) $display(" <ignored>");
+         else if (gennum != EXP) begin
+            $display (" %%Error");
+            fails = 1;
+         end
+         else $display;
+
+         $display;
       end
    end
 endmodule

test_regress/t/t_gen_genblk_noinl.pl

@@ -12,6 +12,8 @@ top_filename("t_gen_genblk.v");
 
 scenarios(simulator => 1);
 
+$Self->{sim_time} = 11000;
+
 compile(
     v_flags2 => ["-Oi"],
     );

test_regress/t/t_gen_intdot2.v

@@ -64,11 +64,8 @@ module Genit (
       else
 	One ifcell1(); // genblk1.ifcell1
    endgenerate
-   // On compliant simulators "Implicit name" not allowed here; IE we can't use "genblk1" etc
+   // DISAGREEMENT on this naming
-`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
@@ -76,10 +73,8 @@ module Genit (
 	   One ifcell2();   // namedif2.genblk1.ifcell2
       end
    endgenerate
-`ifdef verilator
+   // DISAGREEMENT on this naming
    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)
@@ -91,15 +86,15 @@ module Genit (
 
    // CASE
    generate
+      begin : casecheck
 	 case (1'b1)
 	   1'b1 :
-	  One casecell10();	// genblk3.casecell10
+	     One casecell10();	// genblk4.casecell10
 	 endcase
+      end
    endgenerate
-`ifdef verilator
+   // DISAGREEMENT on this naming
-   always @ (posedge clk) if (genblk3.casecell10.one !== 1'b1) $stop;
+   always @ (posedge clk) if (casecheck.genblk1.casecell10.one !== 1'b1) $stop;
-//`else // NOT SUPPORTED accoring to spec - generic block references
-`endif
 
    generate
       case (1'b1)
@@ -113,16 +108,15 @@ module Genit (
    genvar i;
    genvar j;
 
-   // IF
    generate
+      begin : genfor
 	 for (i = 0; i < 2; i = i + 1)
-	One cellfor20 ();	// genblk4[0..1].cellfor20
+	   One cellfor20 ();	// genfor.genblk1[0..1].cellfor20
+      end
    endgenerate
-`ifdef verilator
+   // DISAGREEMENT on this naming
-   always @ (posedge clk) if (genblk4[0].cellfor20.one !== 1'b1) $stop;
+   always @ (posedge clk) if (genfor.genblk1[0].cellfor20.one !== 1'b1) $stop;
-   always @ (posedge clk) if (genblk4[1].cellfor20.one !== 1'b1) $stop;
+   always @ (posedge clk) if (genfor.genblk1[1].cellfor20.one !== 1'b1) $stop;
-//`else // NOT SUPPORTED accoring to spec - generic block references
-`endif
 
    // COMBO
    generate

test_regress/t/t_generate_fatal_bad.out

@@ -50,7 +50,7 @@
    13 |    localparam integer BAZ = get_baz(BAR);
       |                             ^~~~~~~
 %Error: t/t_generate_fatal_bad.v:13:29: Expecting expression to be constant, but can't determine constant for FUNCREF 'get_baz'
-                                      : ... In instance t.genblk1.foo_inst4
+                                      : ... In instance t.genblk4.foo_inst4
         t/t_generate_fatal_bad.v:9:4: ... Location of non-constant STOP: $stop executed during function constification; maybe indicates assertion firing
         t/t_generate_fatal_bad.v:13:29: ... Called from get_baz() with parameters:
            bar = ?32?h7