From face700f2932f7eed012ad3846e589cef0501425 Mon Sep 17 00:00:00 2001 From: Pawel Kojma Date: Sat, 28 Feb 2026 15:55:06 +0100 Subject: [PATCH] Improve assignment-compatibility type check (#2843) (#5666) (#7052) --- src/V3AstNodeDType.h | 1 + src/V3Width.cpp | 109 ++++++++++++++++-- .../t/t_assignment_compatibility_bad.out | 64 ++++++++++ .../t/t_assignment_compatibility_bad.py | 16 +++ .../t/t_assignment_compatibility_bad.v | 35 ++++++ test_regress/t/t_assignment_pin_bad.out | 12 ++ test_regress/t/t_assignment_pin_bad.py | 16 +++ test_regress/t/t_assignment_pin_bad.v | 32 +++++ test_regress/t/t_fourstate_assign_bad.out | 10 +- test_regress/t/t_mem_slice_bad.out | 4 + 10 files changed, 287 insertions(+), 12 deletions(-) create mode 100644 test_regress/t/t_assignment_compatibility_bad.out create mode 100755 test_regress/t/t_assignment_compatibility_bad.py create mode 100644 test_regress/t/t_assignment_compatibility_bad.v create mode 100644 test_regress/t/t_assignment_pin_bad.out create mode 100755 test_regress/t/t_assignment_pin_bad.py create mode 100644 test_regress/t/t_assignment_pin_bad.v diff --git a/src/V3AstNodeDType.h b/src/V3AstNodeDType.h index 4d5bd6046..928402b2b 100644 --- a/src/V3AstNodeDType.h +++ b/src/V3AstNodeDType.h @@ -129,6 +129,7 @@ public: // Ideally an IEEE $typename virtual string prettyDTypeName(bool) const { return prettyTypeName(); } string prettyDTypeNameQ() const { return "'" + prettyDTypeName(false) + "'"; } + string stateDTypeName() const { return this->isFourstate() ? "(4-state)" : "(2-state)"; } // // Changing the width may confuse the data type resolution, so must clear // TypeTable cache after use. diff --git a/src/V3Width.cpp b/src/V3Width.cpp index a2e050928..aa6be6838 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -5797,13 +5797,12 @@ class WidthVisitor final : public VNVisitor { // IEEE 1800-2023 7.6: For unpacked arrays to be assignment compatible, // the element types shall be equivalent (IEEE 1800-2023 6.22.2). - // Check specifically for 2-state vs 4-state mismatch for unpacked array - // to unpacked array assignments, as this is a common IEEE compliance issue. // Note: Streaming operators and string literals have implicit conversion rules. if (nodep->rhsp()->dtypep()) { // May be null on earlier errors const AstNodeDType* const lhsDtp = lhsDTypep->skipRefp(); const AstNodeDType* const rhsDtp = nodep->rhsp()->dtypep()->skipRefp(); - // Only check unpacked array to unpacked array assignments + // Only check if number of states match for unpacked array to unpacked array + // assignments const bool lhsIsUnpackArray = VN_IS(lhsDtp, UnpackArrayDType) || VN_IS(lhsDtp, DynArrayDType) || VN_IS(lhsDtp, QueueDType) || VN_IS(lhsDtp, AssocArrayDType); @@ -5812,15 +5811,17 @@ class WidthVisitor final : public VNVisitor { || VN_IS(rhsDtp, QueueDType) || VN_IS(rhsDtp, AssocArrayDType); if (lhsIsUnpackArray && rhsIsUnpackArray) { if (lhsDtp->isFourstate() != rhsDtp->isFourstate()) { - nodep->v3error( - "Assignment between 2-state and 4-state types requires " - "equivalent element types (IEEE 1800-2023 6.22.2, 7.6)\n" - << nodep->warnMore() << "... LHS type: " << lhsDtp->prettyDTypeNameQ() - << (lhsDtp->isFourstate() ? " (4-state)" : " (2-state)") << "\n" - << nodep->warnMore() << "... RHS type: " << rhsDtp->prettyDTypeNameQ() - << (rhsDtp->isFourstate() ? " (4-state)" : " (2-state)")); + nodep->v3error("Assignment between 2-state and 4-state types requires " + "equivalent element types (IEEE 1800-2023 6.22.2, 7.6)\n" + << nodep->warnMore() + << "... Left-hand type: " << lhsDtp->prettyDTypeNameQ() + << lhsDtp->stateDTypeName() << "\n" + << nodep->warnMore() << "... Right-hand type: " + << rhsDtp->prettyDTypeNameQ() << rhsDtp->stateDTypeName()); } } + checkUnpackedArrayAssignmentCompatible( + nodep, VN_CAST(nodep->lhsp(), NodeVarRef), VN_CAST(nodep->rhsp(), NodeVarRef)); } iterateCheckAssign(nodep, "Assign RHS", nodep->rhsp(), FINAL, lhsDTypep); @@ -6433,6 +6434,11 @@ class WidthVisitor final : public VNVisitor { << (exprArrayp ? "" : " not") << " an array. (IEEE 1800-2023 7.6)"); UINFO(1, " Related lo: " << modDTypep); UINFO(1, " Related hi: " << conDTypep); + } else { + checkUnpackedArrayAssignmentCompatible( + nodep, nodep->modVarp(), VN_CAST(nodep->exprp(), NodeVarRef)); + UINFO(1, " Related lo: " << modDTypep); + UINFO(1, " Related hi: " << conDTypep); } iterateCheckAssign(nodep, "pin connection", nodep->exprp(), FINAL, subDTypep); } @@ -8055,6 +8061,89 @@ class WidthVisitor final : public VNVisitor { } return false; } + // Checks whether two types are assignment-compatible according to IEEE 1800-2023 7.6 + // Currently, this function only supports variables, which are of following types: + // - Fixed-size unpacked array + // - Dynamic unpacked array + // - Associative array + template + void checkUnpackedArrayAssignmentCompatible(const AstNode* nodep, const T* const lhsRefp, + const N* const rhsRefp) { + static_assert( + (std::is_same::value || std::is_same::value) + && (std::is_same::value || std::is_same::value), + "Unsupported types provided."); + if (!lhsRefp || !rhsRefp) return; + string lhsName, rhsName; + if (VN_IS(nodep, Pin)) { + lhsName = std::string{"Pin"}; + rhsName = std::string{"Expression"}; + } else { + lhsName = std::string{"Left-hand"}; + rhsName = std::string{"Right-hand"}; + } + + const AstNodeDType* const lhsDtp = lhsRefp->dtypep()->skipRefp(); + const AstNodeDType* const rhsDtp = rhsRefp->dtypep()->skipRefp(); + const bool isLhsAggregate = lhsDtp->isAggregateType(); + const bool isRhsAggregate = rhsDtp->isAggregateType(); + if (!isLhsAggregate && !isRhsAggregate) return; + if (isLhsAggregate ^ isRhsAggregate) { + nodep->v3error( + "Illegal assignment: types are not assignment compatible (IEEE 1800-2023 7.6)\n" + << nodep->warnMore() << "... " << lhsName << " data type: " + << lhsDtp->prettyDTypeNameQ() << " " << lhsDtp->stateDTypeName() << "\n" + << nodep->warnMore() << "... " << rhsName << " data type: " + << rhsDtp->prettyDTypeNameQ() << " " << rhsDtp->stateDTypeName() << "\n"); + return; + } else if (VN_IS(lhsDtp, QueueDType) && VN_IS(rhsDtp, EmptyQueueDType)) { + return; + } + std::pair lhsDim = lhsDtp->dimensions(false), + rhsDim = rhsDtp->dimensions(false); + // Check if unpacked array dimensions are matching + if (lhsDim.second != rhsDim.second) { + nodep->v3error("Illegal assignment: Unmatched number of unpacked dimensions " + << "(" << lhsDim.second << " vs " << rhsDim.second << ")"); + return; + } + + const AstNodeDType* lhsDtpIterp = lhsDtp; + const AstNodeDType* rhsDtpIterp = rhsDtp; + // Sizes of fixed-size arrays should be the same + // Dynamic-sized arrays are always assignable + for (uint32_t dim = 0; dim < rhsDim.second; dim++) { + if (const AstNodeArrayDType* rhsArrayp = VN_CAST(rhsDtpIterp, NodeArrayDType)) { + if (const AstNodeArrayDType* lhsArrayp = VN_CAST(lhsDtpIterp, NodeArrayDType)) { + if (lhsArrayp->elementsConst() != rhsArrayp->elementsConst()) { + nodep->v3error("Illegal assignment: Unmatched array sizes in dimension " + << dim << " " << "(" << lhsArrayp->elementsConst() << " vs " + << rhsArrayp->elementsConst() << ")"); + return; + } + } + } + // Associative arrays are compatible only with each other + if (VN_IS(lhsDtpIterp, AssocArrayDType) ^ VN_IS(rhsDtpIterp, AssocArrayDType)) { + nodep->v3error("Illegal assignment: Associative arrays are assignment compatible " + "only with associative arrays (IEEE 1800-2023 7.6)"); + + return; + } + lhsDtpIterp = lhsDtpIterp->subDTypep(); + rhsDtpIterp = rhsDtpIterp->subDTypep(); + } + // Element types of source and target shall be equivalent + if (!isEquivalentDType(lhsDtpIterp, rhsDtpIterp)) { + nodep->v3error("Illegal assignment: Array element types are not equivalent (IEEE " + "1800-2023 6.22.2)\n" + << nodep->warnMore() << "... " << lhsName << " data type: " + << lhsDtp->prettyDTypeNameQ() << " " << lhsDtp->stateDTypeName() << "\n" + << nodep->warnMore() << "... " << rhsName + << " data type: " << rhsDtp->prettyDTypeNameQ() << " " + << rhsDtp->stateDTypeName() << "\n"); + } + } void checkClassAssign(const AstNode* nodep, const char* side, AstNode* rhsp, AstNodeDType* const lhsDTypep) { UASSERT_OBJ(rhsp->dtypep(), rhsp, "Node has no type"); diff --git a/test_regress/t/t_assignment_compatibility_bad.out b/test_regress/t/t_assignment_compatibility_bad.out new file mode 100644 index 000000000..fdb138927 --- /dev/null +++ b/test_regress/t/t_assignment_compatibility_bad.out @@ -0,0 +1,64 @@ +%Error: t/t_assignment_compatibility_bad.v:25:24: Illegal assignment: Unmatched array sizes in dimension 0 (3 vs 2) + : ... note: In instance 't' + 25 | logic unpackedF[3] = unpackedA; + | ^~~~~~~~~ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_assignment_compatibility_bad.v:26:31: Assignment between 2-state and 4-state types requires equivalent element types (IEEE 1800-2023 6.22.2, 7.6) + : ... note: In instance 't' + : ... Left-hand type: 'bit$[0:1]'(2-state) + : ... Right-hand type: 'logic$[0:1]'(4-state) + 26 | bit unpackedG[2] = unpackedB[0:1]; + | ^ +%Error: t/t_assignment_compatibility_bad.v:28:20: Illegal assignment: Unmatched array sizes in dimension 0 (3 vs 2) + : ... note: In instance 't' + 28 | assign unpackedB = unpackedA; + | ^ +%Error: t/t_assignment_compatibility_bad.v:29:20: Illegal assignment: Unmatched number of unpacked dimensions (1 vs 2) + : ... note: In instance 't' + 29 | assign unpackedB = unpackedC; + | ^ +%Error: t/t_assignment_compatibility_bad.v:30:20: Illegal assignment: Unmatched array sizes in dimension 0 (4 vs 3) + : ... note: In instance 't' + 30 | assign unpackedD = unpackedC; + | ^ +%Error: t/t_assignment_compatibility_bad.v:31:20: Assignment between 2-state and 4-state types requires equivalent element types (IEEE 1800-2023 6.22.2, 7.6) + : ... note: In instance 't' + : ... Left-hand type: 'struct{}$unit::struct_t$[0:3][0:1]'(2-state) + : ... Right-hand type: 'logic$[0:3][0:1]'(4-state) + 31 | assign unpackedE = unpackedD; + | ^ +%Error: t/t_assignment_compatibility_bad.v:31:20: Illegal assignment: Array element types are not equivalent (IEEE 1800-2023 6.22.2) + : ... note: In instance 't' + : ... Left-hand data type: 'struct{}$unit::struct_t$[0:3][0:1]' (2-state) + : ... Right-hand data type: 'logic$[0:3][0:1]' (4-state) + 31 | assign unpackedE = unpackedD; + | ^ +%Warning-WIDTHEXPAND: t/t_assignment_compatibility_bad.v:31:20: Operator ASSIGNW expects 64 bits on the Assign RHS, but Assign RHS's VARREF 'unpackedD' generates 1 bits. + : ... note: In instance 't' + 31 | assign unpackedE = unpackedD; + | ^ + ... For warning description see https://verilator.org/warn/WIDTHEXPAND?v=latest + ... Use "/* verilator lint_off WIDTHEXPAND */" and lint_on around source to disable this message. +%Error: t/t_assignment_compatibility_bad.v:32:23: Illegal assignment: types are not assignment compatible (IEEE 1800-2023 7.6) + : ... note: In instance 't' + : ... Left-hand data type: 'logic' (4-state) + : ... Right-hand data type: 'logic$[0:1]' (4-state) + 32 | assign nonAggregate = unpackedA; + | ^ +%Error: t/t_assignment_compatibility_bad.v:33:20: Assignment between 2-state and 4-state types requires equivalent element types (IEEE 1800-2023 6.22.2, 7.6) + : ... note: In instance 't' + : ... Left-hand type: 'logic$[0:1]'(4-state) + : ... Right-hand type: 'logic$[string]'(2-state) + 33 | assign unpackedA = assocArrayA; + | ^ +%Error: t/t_assignment_compatibility_bad.v:33:20: Illegal assignment: Associative arrays are assignment compatible only with associative arrays (IEEE 1800-2023 7.6) + : ... note: In instance 't' + 33 | assign unpackedA = assocArrayA; + | ^ +%Error: t/t_assignment_compatibility_bad.v:34:17: Illegal assignment: Array element types are not equivalent (IEEE 1800-2023 6.22.2) + : ... note: In instance 't' + : ... Left-hand data type: 'logic$[$]' (2-state) + : ... Right-hand data type: 'bit$[$]' (2-state) + 34 | assign queueA = queueB; + | ^ +%Error: Exiting due to diff --git a/test_regress/t/t_assignment_compatibility_bad.py b/test_regress/t/t_assignment_compatibility_bad.py new file mode 100755 index 000000000..f3bbcad9d --- /dev/null +++ b/test_regress/t/t_assignment_compatibility_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: 2025 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('vlt') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_assignment_compatibility_bad.v b/test_regress/t/t_assignment_compatibility_bad.v new file mode 100644 index 000000000..9cbba8569 --- /dev/null +++ b/test_regress/t/t_assignment_compatibility_bad.v @@ -0,0 +1,35 @@ +// DESCRIPTION: Verilator: Verilog Test module for SystemVerilog +// +// Assignment compatibility test. +// +// This file ONLY is placed under the Creative Commons Public Domain +// SPDX-FileCopyrightText: 2026 Antmicro +// SPDX-License-Identifier: CC0-1.0 + +typedef struct packed { + int a; + int b; +} struct_t; + +module t; + + logic unpackedA[2]; + logic unpackedB[3]; + logic unpackedC[3][2]; + logic unpackedD[4][2]; + struct_t unpackedE[4][2]; + logic nonAggregate; + logic assocArrayA[string]; + logic queueA[$]; + bit queueB[$]; + logic unpackedF[3] = unpackedA; + bit unpackedG[2] = unpackedB[0:1]; + + assign unpackedB = unpackedA; + assign unpackedB = unpackedC; + assign unpackedD = unpackedC; + assign unpackedE = unpackedD; + assign nonAggregate = unpackedA; + assign unpackedA = assocArrayA; + assign queueA = queueB; +endmodule diff --git a/test_regress/t/t_assignment_pin_bad.out b/test_regress/t/t_assignment_pin_bad.out new file mode 100644 index 000000000..c29062b73 --- /dev/null +++ b/test_regress/t/t_assignment_pin_bad.out @@ -0,0 +1,12 @@ +%Error: t/t_assignment_pin_bad.v:29:19: Illegal input port connection 'b', mismatch between port which is not an array, and expression which is an array. (IEEE 1800-2023 7.6) + : ... note: In instance 't' + 29 | test1 i_test1 (.b); + | ^ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_assignment_pin_bad.v:30:19: Illegal assignment: Array element types are not equivalent (IEEE 1800-2023 6.22.2) + : ... note: In instance 't' + : ... Pin data type: 'bit$[0:1]' (2-state) + : ... Expression data type: 'logic$[0:1]' (4-state) + 30 | test2 i_test2 (.b); + | ^ +%Error: Exiting due to diff --git a/test_regress/t/t_assignment_pin_bad.py b/test_regress/t/t_assignment_pin_bad.py new file mode 100755 index 000000000..3160d0589 --- /dev/null +++ b/test_regress/t/t_assignment_pin_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: 2024 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('vlt') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_assignment_pin_bad.v b/test_regress/t/t_assignment_pin_bad.v new file mode 100644 index 000000000..2d0eddae9 --- /dev/null +++ b/test_regress/t/t_assignment_pin_bad.v @@ -0,0 +1,32 @@ +// DESCRIPTION: Verilator: Verilog Test module for SystemVerilog +// +// This file ONLY is placed under the Creative Commons Public Domain +// SPDX-FileCopyrightText: 2026 Antmicro +// SPDX-License-Identifier: CC0-1.0 + +module test1 ( + input logic b +); + logic do_something; + assign do_something = b; +endmodule + +module test2 ( + input bit b[2] +); + bit do_something[2]; + assign do_something = b; +endmodule + +module t ( + input logic a[2] // unpacked array +); + + logic b[2]; + + assign b = a; + + test1 i_test1 (.b); + test2 i_test2 (.b); + +endmodule diff --git a/test_regress/t/t_fourstate_assign_bad.out b/test_regress/t/t_fourstate_assign_bad.out index a43b8027e..284018659 100644 --- a/test_regress/t/t_fourstate_assign_bad.out +++ b/test_regress/t/t_fourstate_assign_bad.out @@ -1,8 +1,14 @@ %Error: t/t_fourstate_assign_bad.v:23:16: Assignment between 2-state and 4-state types requires equivalent element types (IEEE 1800-2023 6.22.2, 7.6) : ... note: In instance 't' - : ... LHS type: 'bit[7:0]$[3:0]' (2-state) - : ... RHS type: 'logic[7:0]$[3:0]' (4-state) + : ... Left-hand type: 'bit[7:0]$[3:0]'(2-state) + : ... Right-hand type: 'logic[7:0]$[3:0]'(4-state) 23 | arr_2state = arr_4state; | ^ ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_fourstate_assign_bad.v:23:16: Illegal assignment: Array element types are not equivalent (IEEE 1800-2023 6.22.2) + : ... note: In instance 't' + : ... Left-hand data type: 'bit[7:0]$[3:0]' (2-state) + : ... Right-hand data type: 'logic[7:0]$[3:0]' (4-state) + 23 | arr_2state = arr_4state; + | ^ %Error: Exiting due to diff --git a/test_regress/t/t_mem_slice_bad.out b/test_regress/t/t_mem_slice_bad.out index 3aa57d7f1..3d62544dd 100644 --- a/test_regress/t/t_mem_slice_bad.out +++ b/test_regress/t/t_mem_slice_bad.out @@ -23,4 +23,8 @@ : ... note: In instance 't' 51 | active_command4[7:0] <= command_A4[8:0]; | ^ +%Error: t/t_mem_slice_bad.v:56:28: Illegal assignment: Unmatched array sizes in dimension 0 (9 vs 8) + : ... note: In instance 't' + 56 | active_command5[8:0] = command_A5[7:0]; + | ^ %Error: Exiting due to