From b926fcb7628903f418f8b435aac954560ba40d52 Mon Sep 17 00:00:00 2001 From: Yutetsu TAKATSUKASA Date: Sat, 10 Jan 2026 09:06:37 +0900 Subject: [PATCH] Improve signedness of packed array (#6901) (#6902) * Tests: Add a test whether signedness of a packed array is properly implemented. * Fix signedness of a packed array when named type is not used. * Fix signedness of the entire packed array. --- src/V3Width.cpp | 16 +++++++ src/V3WidthSel.cpp | 6 +++ test_regress/t/t_array_packed_sign.py | 18 ++++++++ test_regress/t/t_array_packed_sign.v | 60 +++++++++++++++++++++++++++ 4 files changed, 100 insertions(+) create mode 100755 test_regress/t/t_array_packed_sign.py create mode 100644 test_regress/t/t_array_packed_sign.v diff --git a/src/V3Width.cpp b/src/V3Width.cpp index e3dd541af..0e0a64867 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -216,6 +216,8 @@ class WidthVisitor final : public VNVisitor { WidthVP* m_vup = nullptr; // Current node state bool m_underFork = false; // Visiting under a fork bool m_underSExpr = false; // Visiting under a sequence expression + bool m_underPackedArray = false; // Visiting under a AstPackArrayDType + 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 @@ -2044,8 +2046,13 @@ 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. nodep->refDTypep(iterateEditMoveDTypep(nodep, nodep->subDTypep())); + // Cleanup array size userIterateAndNext(nodep->rangep(), WidthVP{SELF, BOTH}.p()); nodep->dtypep(nodep); // The array itself, not subDtype @@ -2056,6 +2063,14 @@ class WidthVisitor final : public VNVisitor { } else { 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()) { + nodep->numeric(VSigning::fromBool(true)); + } + } } UINFO(4, "dtWidthed " << nodep); } @@ -2177,6 +2192,7 @@ 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/src/V3WidthSel.cpp b/src/V3WidthSel.cpp index e6abd704a..1a4a38223 100644 --- a/src/V3WidthSel.cpp +++ b/src/V3WidthSel.cpp @@ -266,6 +266,12 @@ class WidthSelVisitor final : public VNVisitor { newp->declRange(fromRange); newp->declElWidth(elwidth); newp->dtypeFrom(adtypep->subDTypep()); // Need to strip off array reference + if (VN_IS(adtypep->subDTypep(), BasicDType)) { + // IEEE 1800-2023 7.4.1 Packed arrays says: + // The individual elements of the array are unsigned + // unless they are of a named type declared as signed. + newp->dtypep()->numeric(VSigning::fromBool(false)); + } UINFOTREE(9, newp, "", "SELBTn"); nodep->replaceWith(newp); VL_DO_DANGLING(pushDeletep(nodep), nodep); diff --git a/test_regress/t/t_array_packed_sign.py b/test_regress/t/t_array_packed_sign.py new file mode 100755 index 000000000..d4f986441 --- /dev/null +++ b/test_regress/t/t_array_packed_sign.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 +# 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 + +import vltest_bootstrap + +test.scenarios('simulator') + +test.compile() + +test.execute() + +test.passes() diff --git a/test_regress/t/t_array_packed_sign.v b/test_regress/t/t_array_packed_sign.v new file mode 100644 index 000000000..16a7772da --- /dev/null +++ b/test_regress/t/t_array_packed_sign.v @@ -0,0 +1,60 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2026 Yutetsu TAKATSUKASA +// SPDX-License-Identifier: CC0-1.0 + +// 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 +// vector shall be signed. The individual elements of the array are unsigned +// unless they are of a named type declared as signed. + +module t; + typedef logic signed [2:0] named_t; + typedef named_t [1:0] named_named_t; + typedef logic signed [1:0][2:0] named_unnamed_t; + + named_named_t [1:0] named_named; + named_unnamed_t [1:0] named_unnamed; + logic signed [1:0][1:0][2:0] unnamed; + + initial begin + // Set 1 to MSB(=sign bit) + named_named = 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 + + 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 + + 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 + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule