From 76df8b4a7eb3fa2e384a2a503a3bf967c6904eeb Mon Sep 17 00:00:00 2001 From: Artur Bieniek Date: Mon, 7 Sep 2026 20:04:17 +0200 Subject: [PATCH] Fix multidimensional packed array type handling (#8235) Signed-off-by: Artur Bieniek --- src/V3Width.cpp | 16 +++-- test_regress/t/t_array_packed_sign.v | 61 +++++++++++-------- .../t/t_array_packed_unpacked_bad.out | 14 +++++ test_regress/t/t_array_packed_unpacked_bad.py | 16 +++++ test_regress/t/t_array_packed_unpacked_bad.v | 22 +++++++ 5 files changed, 97 insertions(+), 32 deletions(-) create mode 100644 test_regress/t/t_array_packed_unpacked_bad.out create mode 100755 test_regress/t/t_array_packed_unpacked_bad.py create mode 100644 test_regress/t/t_array_packed_unpacked_bad.v diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 467c908d0..dc8b1d95e 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -230,7 +230,6 @@ class WidthVisitor final : public VNVisitor { bool m_underSExpr = false; // Visiting under a sequence expression bool m_underPackedArray = false; // Visiting under a AstPackArrayDType bool m_underMemberSel = false; // Viting under a MemberSel - bool m_hasNamedType = false; // Packed array is defined using named type AstNode* m_seqUnsupp = nullptr; // Property has unsupported node bool m_hasSExpr = false; // Property has a sequence expression const AstCell* m_cellp = nullptr; // Current cell for arrayed instantiations @@ -2507,8 +2506,6 @@ class WidthVisitor final : public VNVisitor { VL_DO_DANGLING(pushDeletep(basicp), basicp); } } - if (!m_underPackedArray) m_hasNamedType = false; // Outermost dimension - VL_RESTORER(m_hasNamedType); VL_RESTORER(m_underPackedArray); if (VN_IS(nodep, PackArrayDType)) m_underPackedArray = true; // Iterate into subDTypep() to resolve that type and update pointer. @@ -2522,13 +2519,23 @@ class WidthVisitor final : public VNVisitor { nodep->widthFromSub(nodep->subDTypep()); if (nodep->subDTypep()->skipRefp()->isCompound()) adtypep->isCompound(true); } else { + const AstNodeDType* const elemDTypep = nodep->subDTypep()->skipRefp(); + if (!elemDTypep->isIntegralOrPacked()) { + nodep->v3error("Unpacked data type " << elemDTypep->prettyDTypeNameQ() + << " in packed array (IEEE 1800-2023 7.4.1)"); + } const int width = nodep->subDTypep()->width() * nodep->rangep()->elementsConst(); nodep->widthForce(width, width); if (!VL_RESTORER_PREV(m_underPackedArray)) { // Outermost dimension // IEEE 1800-2023 7.4.1 "Packed arrays" says // If a packed array is declared as signed, // then the array viewed as a single vector shall be signed. - if (!m_hasNamedType && nodep->basicp()->isSigned()) { + const AstNodeDType* baseDTypep = nodep->subDTypep(); + while (!VN_IS(baseDTypep, RefDType) && baseDTypep->subDTypep()) { + baseDTypep = baseDTypep->subDTypep(); + } + const AstBasicDType* const basicp = nodep->basicp(); + if (!VN_IS(baseDTypep, RefDType) && basicp && basicp->isSigned()) { nodep->numeric(VSigning::fromBool(true)); } } @@ -2653,7 +2660,6 @@ class WidthVisitor final : public VNVisitor { UINFO(4, "dtWidthed " << nodep); } void visit(AstRefDType* nodep) override { - m_hasNamedType = m_underPackedArray; if (nodep->didWidthAndSet()) return; // This node is a dtype & not both PRELIMed+FINALed nodep->doingWidth(true); if (nodep->typeofp()) { // type(typeofp_expression) diff --git a/test_regress/t/t_array_packed_sign.v b/test_regress/t/t_array_packed_sign.v index 7ee6f23aa..0d2b135a4 100644 --- a/test_regress/t/t_array_packed_sign.v +++ b/test_regress/t/t_array_packed_sign.v @@ -4,6 +4,11 @@ // SPDX-FileCopyrightText: 2026 Yutetsu TAKATSUKASA // SPDX-License-Identifier: CC0-1.0 +// verilog_format: off +`define stop $stop +`define checkd(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0d exp=%0d (%s !== %s)\n", `__FILE__,`__LINE__, (gotv), (expv), `"gotv`", `"expv`"); `stop; end while(0); +// verilog_format: on + // Test to check whether the following spec is properly implemented. // In IEEE 1800-2023 7.4.1 Packed arrays: // If a packed array is declared as signed, then the array viewed as a single @@ -16,44 +21,46 @@ module t; typedef logic signed [1:0][2:0] named_unnamed_t; named_named_t [1:0] named_named; + named_t [1:0][1:0] named_2d; named_unnamed_t [1:0] named_unnamed; logic signed [1:0][1:0][2:0] unnamed; initial begin + logic signed [11:0] whole_result; + logic signed [5:0] slice_result; + logic signed [2:0] element_result; + // Set 1 to MSB(=sign bit) named_named = 12'b100000_000000; + named_2d = 12'b100000_000000; named_unnamed = 12'b100000_000000; unnamed = 12'b100000_000000; - if ($signed((named_named >>> 1) >> 11) != 0) begin - $stop; - end - if ($signed((named_named[1] >>> 1) >> 5) != 0) begin - $stop; - end - if ($signed((named_named[1][1] >>> 1) >> 2) != 1) begin - $stop; - end + whole_result = $signed((named_named >>> 1) >> 11); + `checkd(whole_result, 0); + slice_result = $signed((named_named[1] >>> 1) >> 5); + `checkd(slice_result, 0); + element_result = $signed((named_named[1][1] >>> 1) >> 2); + `checkd(element_result, 1); - if ($signed((named_unnamed >>> 1) >> 11) != 0) begin - $stop; - end - if ($signed((named_unnamed[1] >>> 1) >> 5) != 1) begin - $stop; - end - if ($signed((named_unnamed[1][1] >>> 1) >> 2) != 0) begin - $stop; - end + whole_result = $signed((named_2d >>> 1) >> 11); + `checkd(whole_result, 0); + element_result = $signed((named_2d[1][1] >>> 1) >> 2); + `checkd(element_result, 1); - if ($signed((unnamed >>> 1) >> 11) != 1) begin - $stop; // - end - if ($signed((unnamed[1] >>> 1) >> 5) != 0) begin - $stop; - end - if ($signed((unnamed[1][1] >>> 1) >> 2) != 0) begin - $stop; - end + whole_result = $signed((named_unnamed >>> 1) >> 11); + `checkd(whole_result, 0); + slice_result = $signed((named_unnamed[1] >>> 1) >> 5); + `checkd(slice_result, 1); + element_result = $signed((named_unnamed[1][1] >>> 1) >> 2); + `checkd(element_result, 0); + + whole_result = $signed((unnamed >>> 1) >> 11); + `checkd(whole_result, 1); + slice_result = $signed((unnamed[1] >>> 1) >> 5); + `checkd(slice_result, 0); + element_result = $signed((unnamed[1][1] >>> 1) >> 2); + `checkd(element_result, 0); $write("*-* All Finished *-*\n"); $finish; end diff --git a/test_regress/t/t_array_packed_unpacked_bad.out b/test_regress/t/t_array_packed_unpacked_bad.out new file mode 100644 index 000000000..1bd661f7c --- /dev/null +++ b/test_regress/t/t_array_packed_unpacked_bad.out @@ -0,0 +1,14 @@ +%Error: t/t_array_packed_unpacked_bad.v:18:26: Unpacked data type 'struct{}$unit::unpacked_struct_t' in packed array (IEEE 1800-2023 7.4.1) + : ... note: In instance 't' + 18 | unpacked_struct_t [0:0][0:0] unpacked_struct_array; + | ^ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_array_packed_unpacked_bad.v:19:25: Unpacked data type 'union{}$unit::unpacked_union_t' in packed array (IEEE 1800-2023 7.4.1) + : ... note: In instance 't' + 19 | unpacked_union_t [0:0][0:0] unpacked_union_array; + | ^ +%Error: t/t_array_packed_unpacked_bad.v:21:34: Unpacked data type 'struct{}' in packed array (IEEE 1800-2023 7.4.1) + : ... note: In instance 't' + 21 | typedef struct {logic a;} [0:0][0:0] direct_unpacked_struct_array_t; + | ^ +%Error: Exiting due to diff --git a/test_regress/t/t_array_packed_unpacked_bad.py b/test_regress/t/t_array_packed_unpacked_bad.py new file mode 100755 index 000000000..38cf36b43 --- /dev/null +++ b/test_regress/t/t_array_packed_unpacked_bad.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# 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-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('linter') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_array_packed_unpacked_bad.v b/test_regress/t/t_array_packed_unpacked_bad.v new file mode 100644 index 000000000..0ebb690ac --- /dev/null +++ b/test_regress/t/t_array_packed_unpacked_bad.v @@ -0,0 +1,22 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Antmicro +// SPDX-License-Identifier: CC0-1.0 + +typedef struct { + logic a; + logic b; +} unpacked_struct_t; + +typedef union { + logic a; + logic b; +} unpacked_union_t; + +module t; + unpacked_struct_t [0:0][0:0] unpacked_struct_array; + unpacked_union_t [0:0][0:0] unpacked_union_array; + + typedef struct {logic a;} [0:0][0:0] direct_unpacked_struct_array_t; +endmodule