diff --git a/Changes b/Changes index 0868f7138..9a8c2c443 100644 --- a/Changes +++ b/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 diff --git a/src/V3Width.cpp b/src/V3Width.cpp index b85020e08..5e4adf969 100644 --- a/src/V3Width.cpp +++ b/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(new AstExtendS{nodep->fileline(), nodep}) : static_cast(new AstExtend{nodep->fileline(), nodep})); + newp->didWidth(true); linker.relink(newp); nodep = newp; } diff --git a/src/V3WidthSel.cpp b/src/V3WidthSel.cpp index 12b8d6e66..ddf3ac719 100644 --- a/src/V3WidthSel.cpp +++ b/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, - new AstMul{nodep->fileline(), - new AstConst(nodep->fileline(), AstConst::Unsized32{}, elwidth), subp}, + nodep->fileline(), fromp, newMulConst(nodep->fileline(), 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), - new AstConst(nodep->fileline(), AstConst::Unsized32{}, elwidth)}, + newMulConst(nodep->fileline(), elwidth, newSubLsbOf(lsbp, fromRange)), 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) { - newlsbp = new AstMul{nodep->fileline(), newlsbp, - new AstConst(nodep->fileline(), elwidth)}; - } + if (elwidth != 1) newlsbp = newMulConst(nodep->fileline(), elwidth, newlsbp); AstSel* const newp = new AstSel{nodep->fileline(), fromp, newlsbp, newwidthp}; newp->declRange(fromRange); newp->declElWidth(elwidth); diff --git a/test_regress/t/t_select_mul_extend.pl b/test_regress/t/t_select_mul_extend.pl new file mode 100755 index 000000000..e64ab41be --- /dev/null +++ b/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; diff --git a/test_regress/t/t_select_mul_extend.v b/test_regress/t/t_select_mul_extend.v new file mode 100644 index 000000000..745bac540 --- /dev/null +++ b/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