Fix width extension of unpacked array select (#5095).

Changes

@@ -32,6 +32,7 @@ Verilator 5.025 devel
 * Fix false ASSIGNIN on functions with explicit port map (#5069).
 * Fix DFG assertion with SystemC (#5076). [Geza Lore]
 * Fix macro expansion in strings per 1800-2023 (#5094). [Geza Lore]
+* Fix width extension of unpacked array select (#5095). [Varun Koyyalagunta]
 
 
 Verilator 5.024 2024-04-05

src/V3Width.cpp

@@ -1208,10 +1208,16 @@ class WidthVisitor final : public VNVisitor {
     }
 
     void visit(AstExtend* nodep) override {
-        // Only created by this process, so we know width from here down is correct.
+        // Typically created by this process, so we know width from here down is correct.
+        // Exception is extend added by V3WidthSel - those need iteration
+        if (nodep->didWidthAndSet()) return;
+        userIterateAndNext(nodep->lhsp(), WidthVP{SELF, BOTH}.p());
     }
     void visit(AstExtendS* nodep) override {
-        // Only created by this process, so we know width from here down is correct.
+        // Typically created by this process, so we know width from here down is correct.
+        // Exception is extend added by V3WidthSel - those need iteration
+        if (nodep->didWidthAndSet()) return;
+        userIterateAndNext(nodep->lhsp(), WidthVP{SELF, BOTH}.p());
     }
     void visit(AstConst* nodep) override {
         // The node got setup with the signed/real state of the node.
@@ -6512,6 +6518,7 @@ class WidthVisitor final : public VNVisitor {
             AstNodeExpr* const newp
                 = (doSigned ? static_cast<AstNodeExpr*>(new AstExtendS{nodep->fileline(), nodep})
                             : static_cast<AstNodeExpr*>(new AstExtend{nodep->fileline(), nodep}));
+            newp->didWidth(true);
             linker.relink(newp);
             nodep = newp;
         }

src/V3WidthSel.cpp

@@ -167,6 +167,16 @@ class WidthSelVisitor final : public VNVisitor {
             }
         }
     }
+    AstNodeExpr* newMulConst(FileLine* fl, uint32_t elwidth, AstNodeExpr* indexp) {
+        AstNodeExpr* const extendp = new AstExtend{fl, indexp};
+        extendp->dtypeSetLogicUnsized(
+            32, std::max(V3Number::log2b(elwidth) + 1, indexp->widthMin()), VSigning::UNSIGNED);
+        AstNodeExpr* const mulp
+            = new AstMul{fl, new AstConst{fl, AstConst::Unsized32{}, elwidth},
+                         // Extend needed as index might be e.g. 3 bits but constant e.g. 5 bits
+                         extendp};
+        return mulp;
+    }
 
     AstNodeDType* sliceDType(AstPackArrayDType* nodep, int msb, int lsb) {
         // Return slice needed for msb/lsb, either as original dtype or a new slice dtype
@@ -247,9 +257,7 @@ class WidthSelVisitor final : public VNVisitor {
             // cppcheck-suppress zerodivcond
             const int elwidth = adtypep->width() / fromRange.elements();
             AstSel* const newp = new AstSel{
-                nodep->fileline(), fromp,
+                nodep->fileline(), fromp, newMulConst(nodep->fileline(), elwidth, subp),
-                new AstMul{nodep->fileline(),
-                           new AstConst(nodep->fileline(), AstConst::Unsized32{}, elwidth), subp},
                 new AstConst(nodep->fileline(), AstConst::Unsized32{}, elwidth)};
             newp->declRange(fromRange);
             newp->declElWidth(elwidth);
@@ -428,8 +436,7 @@ class WidthSelVisitor final : public VNVisitor {
             const int elwidth = adtypep->width() / fromRange.elements();
             AstSel* const newp = new AstSel{
                 nodep->fileline(), fromp,
-                new AstMul{nodep->fileline(), newSubLsbOf(lsbp, fromRange),
+                newMulConst(nodep->fileline(), elwidth, newSubLsbOf(lsbp, fromRange)),
-                           new AstConst(nodep->fileline(), AstConst::Unsized32{}, elwidth)},
                 new AstConst(nodep->fileline(), AstConst::Unsized32{}, (msb - lsb + 1) * elwidth)};
             newp->declRange(fromRange);
             newp->declElWidth(elwidth);
@@ -583,10 +590,7 @@ class WidthSelVisitor final : public VNVisitor {
             } else {
                 nodep->v3fatalSrc("Bad Case");
             }
-            if (elwidth != 1) {
+            if (elwidth != 1) newlsbp = newMulConst(nodep->fileline(), elwidth, newlsbp);
-                newlsbp = new AstMul{nodep->fileline(), newlsbp,
-                                     new AstConst(nodep->fileline(), elwidth)};
-            }
             AstSel* const newp = new AstSel{nodep->fileline(), fromp, newlsbp, newwidthp};
             newp->declRange(fromRange);
             newp->declElWidth(elwidth);

test_regress/t/t_select_mul_extend.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 2024 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_select_mul_extend.v

@@ -0,0 +1,98 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2024 by Varun Koyyalagunta.
+// SPDX-License-Identifier: CC0-1.0
+
+module t(/*AUTOARG*/
+   // Inputs
+   clk
+   );
+   input clk;
+
+   integer cyc = 0;
+   reg [63:0] crc;
+   reg [63:0] sum;
+
+   // Take CRC data and apply to testblock inputs
+   wire [31:0] in = crc[31:0];
+
+   /*AUTOWIRE*/
+   // Beginning of automatic wires (for undeclared instantiated-module outputs)
+   wire [31:0]		out;			// From test of Test.v
+   // End of automatics
+
+   Test test(/*AUTOINST*/
+	     // Outputs
+	     .out			(out[31:0]),
+	     // Inputs
+	     .clk			(clk),
+	     .in			(in[31:0]));
+
+   // Aggregate outputs into a single result vector
+   wire [63:0] result = {32'h0, out};
+
+   // Test loop
+   always @ (posedge clk) begin
+`ifdef TEST_VERBOSE
+      $write("[%0t] cyc==%0d crc=%x result=%x sum=%x\n", $time, cyc, crc, result, sum);
+`endif
+      cyc <= cyc + 1;
+      crc <= {crc[62:0], crc[63] ^ crc[2] ^ crc[0]};
+      sum <= result ^ {sum[62:0], sum[63] ^ sum[2] ^ sum[0]};
+      if (cyc == 0) begin
+         // Setup
+         crc <= 64'h5aef0c8d_d70a4497;
+         sum <= '0;
+      end
+      else if (cyc < 10) begin
+         sum <= '0;
+      end
+      else if (cyc < 90) begin
+      end
+      else if (cyc == 99) begin
+         $write("[%0t] cyc==%0d crc=%x sum=%x\n", $time, cyc, crc, sum);
+         if (crc !== 64'hc77bb9b3784ea091) $stop;
+         // What checksum will we end up with (above print should match)
+`define EXPECTED_SUM 64'h4afe43fb79d7b71e
+         if (sum !== `EXPECTED_SUM) $stop;
+         $write("*-* All Finished *-*\n");
+         $finish;
+      end
+   end
+
+endmodule
+
+module Test(/*AUTOARG*/
+   // Outputs
+   out,
+   // Inputs
+   clk, in
+   );
+
+   input clk;
+   input [31:0] in;
+   output reg [31:0] out;
+
+   logic [31:0] cnt = 0;
+   logic [7:0][30:0] q;
+   logic cond = 0;
+
+   always_comb begin
+      for (int i = 0; i < 8; i++) begin
+         if (i == (cond ? (2-cnt)%8 : 0)) begin
+            q[i] = 31'(in);
+         end
+	 else begin
+            q[i] = '0;
+         end
+      end
+   end
+
+   always @(posedge clk) begin
+      cnt <= cnt + 1;
+      cond <= ~cond;
+      out <= {in[31], q[cond ? (3'd2 - cnt[2:0]) : 3'd0]};
+   end
+
+endmodule