Improve code coverage of V3SplitVar.cpp (#2418)

src/V3SplitVar.cpp

@@ -173,10 +173,7 @@ struct SplitVarImpl {
         if (const char* reason = cannotSplitVarTypeReason(varp->varType())) return reason;
         if (const char* reason = cannotSplitVarDirectionReason(varp->direction())) return reason;
         if (varp->isSigPublic()) return "it is public";
-        if (varp->isInoutish()) return "it is bidirectional";
         if (varp->isUsedLoopIdx()) return "it is used as a loop variable";
-        if (varp->isGenVar()) return "it is genvar";
-        if (varp->isParam()) return "it is parameter";
         return NULL;
     }
 
@@ -291,14 +288,6 @@ public:
 
 class UnpackRefMap {
 public:
-    struct Hash {
-        size_t operator()(const UnpackRef& r) const { return reinterpret_cast<size_t>(r.nodep()); }
-    };
-    struct Compare {
-        bool operator()(const UnpackRef& a, const UnpackRef& b) const {
-            return a.nodep() == b.nodep();
-        }
-    };
     typedef std::map<AstVar*, std::set<UnpackRef>, AstNodeComparator> MapType;
     typedef MapType::iterator MapIt;
     typedef MapType::value_type::second_type::iterator SetIt;
@@ -981,19 +970,6 @@ class SplitPackedVarVisitor : public AstNVisitor, public SplitVarImpl {
     int m_numSplit;  // Total number of split variables
     // key:variable to be split. value:location where the variable is referenced.
     PackedVarRefMap m_refs;
-    virtual void visit(AstNodeModule* nodep) VL_OVERRIDE {
-        UASSERT_OBJ(m_modp == NULL, m_modp, "Nested module declaration");
-        if (!VN_IS(nodep, Module)) {
-            UINFO(5, "Skip " << nodep->prettyNameQ() << "\n");
-            return;
-        }
-        UASSERT_OBJ(m_refs.empty(), nodep, "The last module didn't finish split()");
-        m_modp = nodep;
-        UINFO(3, "Start analyzing module " << nodep->prettyName() << '\n');
-        iterateChildren(nodep);
-        split();
-        m_modp = NULL;
-    }
     virtual void visit(AstNodeFTask* nodep) VL_OVERRIDE {
         if (!cannotSplitTaskReason(nodep)) iterateChildren(nodep);
     }
@@ -1238,7 +1214,7 @@ public:
         , m_modp(NULL)
         , m_numSplit(0) {
         // If you want ignore refs and walk the tne entire AST,
-        // just call iterate(nodep) and disable the following for-loop.
+        // just call iterateChildren(m_modp) and split() for each module
         for (SplitVarRefsMap::iterator it = refs.begin(), it_end = refs.end(); it != it_end;
              ++it) {
             m_modp = it->first;

test_regress/t/t_split_var_0.v

@@ -180,7 +180,7 @@ module barshift_2d_packed_array #(parameter DEPTH = 2, localparam WIDTH = 2**DEP
 
    generate
       for(genvar i = 0; i < DEPTH; ++i) begin
-         always_comb
+         always @(shift or tmp)
            /*verilator lint_off ALWCOMBORDER*/
            if (shift[i]) begin
               tmp[i+1+OFFSET] = {tmp[i+OFFSET][(1 << i)-1:0], tmp[i+OFFSET][WIDTH-1:(2**i)]};

test_regress/t/t_split_var_1_bad.out

@@ -11,28 +11,28 @@
 %Warning-SPLITVAR: t/t_split_var_1_bad.v:12:16: 'should_show_warning_ifs1' has split_var metacomment, but will not be split because it is not declared in a module.
    12 |    logic [7:0] should_show_warning_ifs1 [1:0] /*verilator split_var*/ ;
       |                ^~~~~~~~~~~~~~~~~~~~~~~~
-%Warning-SPLITVAR: t/t_split_var_1_bad.v:38:14: 'cannot_split1' has split_var metacomment but will not be split because it is accessed from another module via a dot.
+%Warning-SPLITVAR: t/t_split_var_1_bad.v:40:14: 'cannot_split1' has split_var metacomment but will not be split because it is accessed from another module via a dot.
-   38 |       i_sub0.cannot_split1[0] = 0;
+   40 |       i_sub0.cannot_split1[0] = 0;
       |              ^~~~~~~~~~~~~
-%Warning-SELRANGE: t/t_split_var_1_bad.v:83:33: Selection index out of range: 13 outside 12:10
+%Warning-SELRANGE: t/t_split_var_1_bad.v:90:33: Selection index out of range: 13 outside 12:10
                                               : ... In instance t.i_sub3
-   83 |    assign outwires[12] = inwires[13];   
+   90 |    assign outwires[12] = inwires[13];   
       |                                 ^
-%Warning-WIDTH: t/t_split_var_1_bad.v:39:31: Operator ASSIGN expects 8 bits on the Assign RHS, but Assign RHS's FUNCREF 'bad_func' generates 32 bits.
+%Warning-WIDTH: t/t_split_var_1_bad.v:41:31: Operator ASSIGN expects 8 bits on the Assign RHS, but Assign RHS's FUNCREF 'bad_func' generates 32 bits.
                                            : ... In instance t
-   39 |       i_sub0.cannot_split1[1] = bad_func(addr, rd_data0);
+   41 |       i_sub0.cannot_split1[1] = bad_func(addr, rd_data0);
       |                               ^
-%Error: t/t_split_var_1_bad.v:72:16: Illegal assignment of constant to unpacked array
+%Error: t/t_split_var_1_bad.v:79:16: Illegal assignment of constant to unpacked array
                                    : ... In instance t.i_sub2
-   72 |    assign b = a[0];   
+   79 |    assign b = a[0];   
       |                ^
-%Warning-SPLITVAR: t/t_split_var_1_bad.v:51:31: 'cannot_split0' has split_var metacomment but will not be split because index cannot be determined statically.
+%Warning-SPLITVAR: t/t_split_var_1_bad.v:56:31: 'cannot_split0' has split_var metacomment but will not be split because index cannot be determined statically.
                                               : ... In instance t.i_sub0
-   51 |       rd_data = cannot_split0[addr];
+   56 |       rd_data = cannot_split0[addr];
       |                               ^~~~
-%Warning-SPLITVAR: t/t_split_var_1_bad.v:83:34: 'inwires' has split_var metacomment but will not be split because index is out of range.
+%Warning-SPLITVAR: t/t_split_var_1_bad.v:90:34: 'inwires' has split_var metacomment but will not be split because index is out of range.
                                               : ... In instance t.i_sub3
-   83 |    assign outwires[12] = inwires[13];   
+   90 |    assign outwires[12] = inwires[13];   
       |                                  ^~
 %Warning-SPLITVAR: t/t_split_var_1_bad.v:17:9: 'should_show_warning0' has split_var metacomment but will not be split because it is not an aggregate type of bit nor logic
                                              : ... In instance t
@@ -46,20 +46,32 @@
                                               : ... In instance t
    19 |    wire   should_show_warning2                /*verilator split_var*/ ;
       |           ^~~~~~~~~~~~~~~~~~~~
-%Warning-SPLITVAR: t/t_split_var_1_bad.v:30:44: 'inout_port' has split_var metacomment but will not be split because it is an inout port
+%Warning-SPLITVAR: t/t_split_var_1_bad.v:23:16: 'public_signal' has split_var metacomment but will not be split because it is public
                                               : ... In instance t
-   30 |    function int bad_func(inout logic [3:0] inout_port /*verilator split_var*/ ,
+   23 |    logic [1:0] public_signal /*verilator public*/  /*verilator split_var*/ ;
+      |                ^~~~~~~~~~~~~
+%Warning-SPLITVAR: t/t_split_var_1_bad.v:31:44: 'inout_port' has split_var metacomment but will not be split because it is an inout port
+                                              : ... In instance t
+   31 |    function int bad_func(inout logic [3:0] inout_port /*verilator split_var*/ ,
       |                                            ^~~~~~~~~~
-%Warning-SPLITVAR: t/t_split_var_1_bad.v:31:42: 'ref_port' has split_var metacomment but will not be split because it is a ref argument
+%Warning-SPLITVAR: t/t_split_var_1_bad.v:32:42: 'ref_port' has split_var metacomment but will not be split because it is a ref argument
                                               : ... In instance t
-   31 |                          ref logic [7:0] ref_port /*verilator split_var*/ );
+   32 |                          ref logic [7:0] ref_port /*verilator split_var*/ );
       |                                          ^~~~~~~~
-%Warning-SPLITVAR: t/t_split_var_1_bad.v:57:11: 'cannot_split_genvar' has split_var metacomment but will not be split because it is not an aggregate type of bit nor logic
+%Warning-SPLITVAR: t/t_split_var_1_bad.v:37:19: 'loop_idx' has split_var metacomment but will not be split because it is used as a loop variable
+                                              : ... In instance t
+   37 |       logic [7:0] loop_idx /*verilator split_var*/ ;
+      |                   ^~~~~~~~
+%Warning-SPLITVAR: t/t_split_var_1_bad.v:62:11: 'cannot_split_genvar' has split_var metacomment but will not be split because it is not an aggregate type of bit nor logic
                                               : ... In instance t.i_sub1
-   57 |    genvar cannot_split_genvar /*verilator split_var*/ ;
+   62 |    genvar cannot_split_genvar /*verilator split_var*/ ;
       |           ^~~~~~~~~~~~~~~~~~~
-%Warning-SPLITVAR: t/t_split_var_1_bad.v:60:29: 'cannot_split' has split_var metacomment but will not be split because its bit range cannot be determined statically.
+%Warning-SPLITVAR: t/t_split_var_1_bad.v:65:65: 'cannot_split' has split_var metacomment but will not be split because its bit range cannot be determined statically.
                                               : ... In instance t.i_sub1
-   60 |       rd_data = cannot_split[addr];
+   65 |       logic [8:0] rd_tmp /*verilator split_var*/  = cannot_split[addr];
-      |                             ^
+      |                                                                 ^
+%Warning-SPLITVAR: t/t_split_var_1_bad.v:66:23: 'rd_tmp' has split_var metacomment but will not be split because its bit range cannot be determined statically.
+                                              : ... In instance t.i_sub1
+   66 |       rd_data = rd_tmp[{3'b0, addr[0]}+:8];
+      |                       ^
 %Error: Exiting due to

test_regress/t/t_split_var_1_bad.v

@@ -20,6 +20,7 @@ module t();
 
    logic [3:0] addr;
    logic [7:0] rd_data0, rd_data1, rd_data2;
+   logic [1:0] public_signal /*verilator public*/ /*verilator split_var*/;
 
    sub0 i_sub0(.addr(addr), .rd_data(rd_data0));
    sub1 i_sub1(.addr(addr), .rd_data(rd_data2));
@@ -33,10 +34,14 @@ module t();
    endfunction
 
    initial begin
+      logic [7:0] loop_idx /*verilator split_var*/;
       addr = 0;
       addr = 1;
       i_sub0.cannot_split1[0] = 0;
       i_sub0.cannot_split1[1] = bad_func(addr, rd_data0);
+      for (loop_idx = 0; loop_idx < 8'd4; loop_idx = loop_idx + 2) begin
+          addr += 1;
+      end
       $finish;
    end
 
@@ -55,9 +60,11 @@ endmodule
 
 module sub1(input [3:0]addr, output logic [7:0] rd_data);
    genvar cannot_split_genvar /*verilator split_var*/;
-   logic [15:0] [7:0] cannot_split  /*verilator split_var*/;
+   logic [15:0] [8:0] cannot_split  /*verilator split_var*/;
-   always_comb
+   always_comb begin
-      rd_data = cannot_split[addr];
+      logic [8:0] rd_tmp /*verilator split_var*/ = cannot_split[addr];
+      rd_data = rd_tmp[{3'b0, addr[0]}+:8];
+   end
 
 endmodule
 

test_regress/t/t_split_var_3_wreal.pl

@@ -0,0 +1,24 @@
+#!/usr/bin/env 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.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+scenarios(simulator => 1);
+
+compile(
+    verilator_flags2 => ['--stats'],
+    );
+
+execute(
+    check_finished => 1,
+    );
+
+file_grep($Self->{stats}, qr/SplitVar,\s+Split packed variables\s+(\d+)/i, 0);
+file_grep($Self->{stats}, qr/SplitVar,\s+Split unpacked arrays\s+(\d+)/i, 3);
+ok(1);
+1;

test_regress/t/t_split_var_3_wreal.v

@@ -0,0 +1,54 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2020 by Yutetsu TAKATSUKASA.
+// SPDX-License-Identifier: CC0-1.0
+
+`begin_keywords "VAMS-2.3"
+
+module t (/*autoarg*/
+   // Inputs
+   clk
+   );
+
+   input clk;
+
+   integer cyc=0;
+
+   real  vin[0:1] /*verilator split_var*/;
+   wreal vout[0:1] /*verilator split_var*/;
+   swap  i_swap(.in0(vin[0]), .in1(vin[1]), .out0(vout[0]), .out1(vout[1]));
+
+   always @ (posedge clk) begin
+      cyc <= cyc + 1;
+      if (cyc==0) begin
+        // Setup
+         vin[0] = 1.0;
+         vin[1] = 2.0;
+      end
+      else if (cyc==2) begin
+         vin[0] = 3.0;
+         vin[1] = 4.0;
+      end
+      else if (cyc==3) begin
+         if (vout[0] == vin[1] && vout[1] == vin[0]) begin
+            $write("*-* All Finished *-*\n");
+            $finish;
+         end else begin
+            $write("Mismatch %f %f\n", vout[0], vout[1]);
+            $stop;
+         end
+      end
+   end
+
+endmodule
+
+module swap
+  (input wreal in0, in1,
+   output wreal out0, out1);
+   wreal tmp[0:1] /* verilator split_var*/;
+   assign tmp[0] = in0;
+   assign tmp[1] = in1;
+   assign out0 = tmp[1];
+   assign out1 = tmp[0];
+endmodule