Fix slice of unpacked array calculation (Issue #2506) (#2507)

Fixes Issue #2506 by shifting index as ArraySel does

src/V3Width.cpp

@@ -941,17 +941,21 @@ private:
             if (!m_doGenerate) {
                 // Must check bounds before adding a select that truncates the bound
                 // Note we've already subtracted off LSB
-                if ((nodep->declRange().hi() > adtypep->declRange().hi())
+                const int subtracted = adtypep->declRange().lo();
-                    || nodep->declRange().lo() < adtypep->declRange().lo()) {
+                // Add subtracted value to get the original range
+                const VNumRange declRange{nodep->declRange().hi() + subtracted,
+                                          nodep->declRange().lo() + subtracted,
+                                          nodep->declRange().littleEndian()};
+                if ((declRange.hi() > adtypep->declRange().hi())
+                    || declRange.lo() < adtypep->declRange().lo()) {
                     // Other simulators warn too
-                    nodep->v3error("Slice selection index '" << nodep->declRange() << "'"
+                    nodep->v3error("Slice selection index '" << declRange << "'"
                                                              << " outside data type's '"
                                                              << adtypep->declRange() << "'");
-                } else if ((nodep->declRange().littleEndian()
+                } else if ((declRange.littleEndian() != adtypep->declRange().littleEndian())
-                            != adtypep->declRange().littleEndian())
+                           && declRange.hi() != declRange.lo()) {
-                           && nodep->declRange().hi() != nodep->declRange().lo()) {
                     nodep->v3error("Slice selection '"
-                                   << nodep->declRange() << "'"
+                                   << declRange << "'"
                                    << " has backward indexing versus data type's '"
                                    << adtypep->declRange() << "'");
                 }

src/V3WidthSel.cpp

@@ -351,8 +351,9 @@ private:
                 nodep->replaceWith(newp);
                 VL_DO_DANGLING(pushDeletep(nodep), nodep);
             } else {  // Slice
-                AstSliceSel* newp = new AstSliceSel(nodep->fileline(), fromp,
+                AstSliceSel* newp = new AstSliceSel(
-                                                    VNumRange(VNumRange::LeftRight(), msb, lsb));
+                    nodep->fileline(), fromp,
+                    VNumRange(VNumRange::LeftRight(), msb - fromRange.lo(), lsb - fromRange.lo()));
                 nodep->replaceWith(newp);
                 VL_DO_DANGLING(pushDeletep(nodep), nodep);
             }

test_regress/t/t_array_backw_index_bad.out

@@ -1,21 +1,29 @@
-%Error: t/t_array_backw_index_bad.v:14:19: Slice selection '[1:3]' has backward indexing versus data type's '[3:0]'
+%Error: t/t_array_backw_index_bad.v:17:19: Slice selection '[1:3]' has backward indexing versus data type's '[3:0]'
                                          : ... In instance t
-   14 |       array_assign[1:3] = '{32'd4, 32'd3, 32'd2};
+   17 |       array_assign[1:3] = '{32'd4, 32'd3, 32'd2};
       |                   ^
-%Error: t/t_array_backw_index_bad.v:15:20: Slice selection '[3:1]' has backward indexing versus data type's '[0:3]'
+%Error: t/t_array_backw_index_bad.v:18:20: Slice selection '[3:1]' has backward indexing versus data type's '[0:3]'
                                          : ... In instance t
-   15 |       larray_assign[3:1] = '{32'd4, 32'd3, 32'd2};
+   18 |       larray_assign[3:1] = '{32'd4, 32'd3, 32'd2};
       |                    ^
-%Error: t/t_array_backw_index_bad.v:17:19: Slice selection index '[4:3]' outside data type's '[3:0]'
+%Error: t/t_array_backw_index_bad.v:19:20: Slice selection '[4:6]' has backward indexing versus data type's '[6:3]'
                                          : ... In instance t
-   17 |       array_assign[4:3] = '{32'd4, 32'd3};
+   19 |       array_assign2[4:6] = '{32'd4, 32'd3, 32'd2};
       |                    ^
-%Error: t/t_array_backw_index_bad.v:18:19: Slice selection index '[1:-1]' outside data type's '[3:0]'
+%Error: t/t_array_backw_index_bad.v:20:21: Slice selection '[6:4]' has backward indexing versus data type's '[3:6]'
                                          : ... In instance t
-   18 |       array_assign[1:-1] = '{32'd4, 32'd3};
+   20 |       larray_assign2[6:4] = '{32'd4, 32'd3, 32'd2};
       |                     ^
-%Error: t/t_array_backw_index_bad.v:18:28: Assignment pattern missed initializing elements: -1
+%Error: t/t_array_backw_index_bad.v:22:19: Slice selection index '[4:3]' outside data type's '[3:0]'
                                          : ... In instance t
-   18 |       array_assign[1:-1] = '{32'd4, 32'd3};
+   22 |       array_assign[4:3] = '{32'd4, 32'd3};
+      |                   ^
+%Error: t/t_array_backw_index_bad.v:23:19: Slice selection index '[1:-1]' outside data type's '[3:0]'
+                                         : ... In instance t
+   23 |       array_assign[1:-1] = '{32'd4, 32'd3};
+      |                   ^
+%Error: t/t_array_backw_index_bad.v:23:28: Assignment pattern missed initializing elements: -1
+                                         : ... In instance t
+   23 |       array_assign[1:-1] = '{32'd4, 32'd3};
       |                            ^~
 %Error: Exiting due to

test_regress/t/t_array_backw_index_bad.v

@@ -10,14 +10,21 @@ module t (/*AUTOARG*/);
 
    logic [31:0] larray_assign [0:3];
 
+   logic [31:0] array_assign2 [6:3];
+
+   logic [31:0] larray_assign2 [3:6];
    initial begin
       array_assign[1:3] = '{32'd4, 32'd3, 32'd2};
       larray_assign[3:1] = '{32'd4, 32'd3, 32'd2};
+      array_assign2[4:6] = '{32'd4, 32'd3, 32'd2};
+      larray_assign2[6:4] = '{32'd4, 32'd3, 32'd2};
 
       array_assign[4:3] = '{32'd4, 32'd3};
       array_assign[1:-1] = '{32'd4, 32'd3};
       array_assign[1:1] = '{32'd4};  // Ok
       larray_assign[1:1] = '{32'd4};  // Ok
+      array_assign2[4:4] = '{32'd4};  // Ok
+      larray_assign2[4:4] = '{32'd4};  // Ok
 
       $write("*-* All Finished *-*\n");
       $finish;

test_regress/t/t_unpacked_slice_range.pl

@@ -0,0 +1,21 @@
+#!/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(
+    );
+
+execute(
+    check_finished => 1,
+    );
+
+ok(1);
+1;

test_regress/t/t_unpacked_slice_range.v

@@ -0,0 +1,63 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2020 by Yutetsu TAKATSUKASA
+
+module t (
+   clk
+   );
+   input clk;
+
+   int c = 0;
+
+   t2 #(0) i_0(.*);
+   t2 #(-1) i_1(.*); // lo is -1, hi is 5
+   t2 #(-4) i_2(.*); // lo is -4, hi is 1
+   t2 #(-10) i_3(.*); // lo is -10, hi is -4
+   t2 #(+1) i_4(.*); // lo is 1, hi is 7
+   t2 #(+4) i_5(.*); // lo is 4, hi is 10
+   t2 #(+10) i_6(.*); // lo is 10, hi is 16
+
+   always @(posedge clk) begin
+      c <= c + 1;
+      if (c == 5) begin
+         $write("*-* All Finished *-*\n");
+         $finish;
+      end
+   end
+endmodule
+
+module t2 #(parameter ORIGIN = 0) (input wire clk, input int c);
+   localparam WIDTH = 7;
+   localparam OFFSET = 3;
+   localparam FULL_LO = ORIGIN;
+   localparam FULL_HI = ORIGIN + WIDTH - 1;
+   localparam PART_LO = FULL_LO + OFFSET;
+   localparam PART_HI = FULL_HI;
+   logic unpack_sig0 [FULL_LO:FULL_HI];
+   logic unpack_sig1 [PART_LO:PART_HI];
+   logic unpack_sig2 [FULL_HI:FULL_LO];
+   logic unpack_sig3 [PART_HI:PART_LO];
+   initial $display("%m ORIGIN:%d [%d:%d] [%d:%d]", ORIGIN, FULL_LO, FULL_HI, PART_LO, PART_HI);
+
+   always @(posedge clk) begin
+      unpack_sig0[PART_LO] <= 1'b1;
+      unpack_sig1[PART_LO] <= 1'b1;
+      unpack_sig0 [PART_LO+1:FULL_HI] <= unpack_sig0[PART_LO:FULL_HI-1];
+      unpack_sig1 [PART_LO+1:PART_HI] <= unpack_sig1[PART_LO:PART_HI-1];
+      unpack_sig2[PART_LO] <= 1'b1;
+      unpack_sig3[PART_LO] <= 1'b1;
+      unpack_sig2 [FULL_HI:PART_LO+1] <= unpack_sig2[FULL_HI-1:PART_LO];
+      unpack_sig3 [PART_HI:PART_LO+1] <= unpack_sig3[PART_HI-1:PART_LO];
+   end
+
+   always @(posedge clk) begin
+      if (c >= 4) begin
+         if (!unpack_sig0[FULL_HI] || !unpack_sig1[PART_HI]) $stop;
+         if (!unpack_sig2[FULL_HI] || !unpack_sig3[PART_HI]) $stop;
+      end else begin
+         if (unpack_sig0[FULL_HI] || unpack_sig1[PART_HI]) $stop;
+         if (unpack_sig2[FULL_HI] || unpack_sig3[PART_HI]) $stop;
+      end
+   end
+endmodule