diff --git a/Changes b/Changes index 7d5569a1f..02029ac36 100644 --- a/Changes +++ b/Changes @@ -23,6 +23,7 @@ Verilator 5.039 devel * Add ALWNEVER warning, for `always @*` that never execute (#6291). * Add separate coverage counters for toggles 0->1 and 1->0 (#6086). [Ryszard Rozak, Antmicro Ltd.] * Add error on class 'function static'. +* Add error on force/release non-constant selects. * Add `-DVERILATOR=1` definition to compiler flags when using verilated.mk. * Support member-level triggers for virtual interfaces (#5166) (#6148). [Yilou Wang] * Support unassigned virtual interfaces (#5265) (#6245). [Szymon Gizler, Antmicro Ltd.] diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 49cf22ba6..5bef730b3 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -5469,6 +5469,7 @@ class WidthVisitor final : public VNVisitor { iterateCheckAssign(nodep, "Assign RHS", nodep->rhsp(), FINAL, lhsDTypep); // UINFOTREE(1, nodep, "", "AssignOut"); } + if (VN_IS(nodep, AssignForce)) checkForceReleaseLhs(nodep, nodep->lhsp()); if (AstNode* const controlp = nodep->timingControlp()) { if (VN_IS(m_ftaskp, Func)) { controlp->v3error("Timing controls are not legal in functions. Suggest use a task " @@ -5537,6 +5538,7 @@ class WidthVisitor final : public VNVisitor { userIterateAndNext(nodep->lhsp(), WidthVP{SELF, BOTH}.p()); UASSERT_OBJ(nodep->lhsp()->dtypep(), nodep, "How can LValue be untyped?"); UASSERT_OBJ(nodep->lhsp()->dtypep()->widthSized(), nodep, "How can LValue be unsized?"); + checkForceReleaseLhs(nodep, nodep->lhsp()); } void formatNoStringArg(AstNode* argp, char ch) { @@ -8544,7 +8546,7 @@ class WidthVisitor final : public VNVisitor { const AstArg* const argp = tconnect.second; const AstNode* const pinp = argp->exprp(); if (!pinp) continue; // Argument error we'll find later - if (hasOpenArrayIterateDType(portp->dtypep())) portp->dtypep(pinp->dtypep()); + if (hasOpenArrayDTypeRecurse(portp->dtypep())) portp->dtypep(pinp->dtypep()); } } @@ -8552,7 +8554,7 @@ class WidthVisitor final : public VNVisitor { bool hasOpen = false; for (AstNode* stmtp = nodep->stmtsp(); stmtp; stmtp = stmtp->nextp()) { if (AstVar* const portp = VN_CAST(stmtp, Var)) { - if (portp->isDpiOpenArray() || hasOpenArrayIterateDType(portp->dtypep())) { + if (portp->isDpiOpenArray() || hasOpenArrayDTypeRecurse(portp->dtypep())) { portp->isDpiOpenArray(true); hasOpen = true; } @@ -8560,10 +8562,10 @@ class WidthVisitor final : public VNVisitor { } return hasOpen; } - bool hasOpenArrayIterateDType(AstNodeDType* nodep) { + bool hasOpenArrayDTypeRecurse(AstNodeDType* nodep) { // Return true iff this datatype or child has an openarray if (VN_IS(nodep, UnsizedArrayDType)) return true; - if (nodep->subDTypep()) return hasOpenArrayIterateDType(nodep->subDTypep()->skipRefp()); + if (nodep->subDTypep()) return hasOpenArrayDTypeRecurse(nodep->subDTypep()->skipRefp()); return false; } AstNodeDType* dtypeNotIntAtomOrVecRecurse(AstNodeDType* nodep, bool ranged = false) { @@ -8599,6 +8601,44 @@ class WidthVisitor final : public VNVisitor { } return nodep; } + void checkForceReleaseLhs(AstNode* nodep, AstNode* lhsp) { + // V3Force can't check as vector may have expanded, or propagated constant into index + if (AstNode* const selNodep = selectNonConstantRecurse(lhsp)) + nodep->v3error((VN_IS(nodep, Release) ? "Release"s : "Force"s) + + " left-hand-side must not have variable bit/part select " + "(IEEE 1800-2023 10.6.2)\n" + << nodep->warnContextPrimary() << '\n' + << selNodep->warnOther() << "... Location of non-constant index\n" + << selNodep->warnContextSecondary()); + } + AstNode* selectNonConstantRecurse(AstNode* nodep, bool inSel = false) { + // If node has a non-constant select, return that select + AstNode* resultp = nullptr; + if (AstNodeSel* const anodep = VN_CAST(nodep, NodeSel)) { + resultp = selectNonConstantRecurse(anodep->fromp(), inSel); + if (resultp) return resultp; + resultp = selectNonConstantRecurse(anodep->bitp(), true); + } else if (AstSel* const anodep = VN_CAST(nodep, Sel)) { + resultp = selectNonConstantRecurse(anodep->fromp(), inSel); + if (resultp) return resultp; + resultp = selectNonConstantRecurse(anodep->lsbp(), true); + } else if (AstNodeVarRef* const anodep = VN_CAST(nodep, NodeVarRef)) { + if (inSel && !anodep->varp()->isParam() && !anodep->varp()->isGenVar()) return anodep; + } else { + if (AstNode* const refp = nodep->op1p()) + resultp = selectNonConstantRecurse(refp, inSel); + if (resultp) return resultp; + if (AstNode* const refp = nodep->op2p()) + resultp = selectNonConstantRecurse(refp, inSel); + if (resultp) return resultp; + if (AstNode* const refp = nodep->op3p()) + resultp = selectNonConstantRecurse(refp, inSel); + if (resultp) return resultp; + if (AstNode* const refp = nodep->op4p()) + resultp = selectNonConstantRecurse(refp, inSel); + } + return resultp; + } //---------------------------------------------------------------------- // METHODS - special type detection diff --git a/test_regress/t/t_force_select_bad.out b/test_regress/t/t_force_select_bad.out new file mode 100644 index 000000000..722296d71 --- /dev/null +++ b/test_regress/t/t_force_select_bad.out @@ -0,0 +1,30 @@ +%Error: t/t_force_select_bad.v:24:5: Force left-hand-side must not have variable bit/part select (IEEE 1800-2023 10.6.2) + : ... note: In instance 't' + 24 | force array1[bad_index] = 1'b1; + | ^~~~~ + t/t_force_select_bad.v:24:18: ... Location of non-constant index + 24 | force array1[bad_index] = 1'b1; + | ^~~~~~~~~ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: t/t_force_select_bad.v:25:5: Release left-hand-side must not have variable bit/part select (IEEE 1800-2023 10.6.2) + : ... note: In instance 't' + 25 | release array1[bad_index]; + | ^~~~~~~ + t/t_force_select_bad.v:25:20: ... Location of non-constant index + 25 | release array1[bad_index]; + | ^~~~~~~~~ +%Error: t/t_force_select_bad.v:26:5: Force left-hand-side must not have variable bit/part select (IEEE 1800-2023 10.6.2) + : ... note: In instance 't' + 26 | force vec[bad_index+:1] = 1'b1; + | ^~~~~ + t/t_force_select_bad.v:26:15: ... Location of non-constant index + 26 | force vec[bad_index+:1] = 1'b1; + | ^~~~~~~~~ +%Error: t/t_force_select_bad.v:27:5: Release left-hand-side must not have variable bit/part select (IEEE 1800-2023 10.6.2) + : ... note: In instance 't' + 27 | release vec[bad_index+:1]; + | ^~~~~~~ + t/t_force_select_bad.v:27:17: ... Location of non-constant index + 27 | release vec[bad_index+:1]; + | ^~~~~~~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_force_select_bad.py b/test_regress/t/t_force_select_bad.py new file mode 100755 index 000000000..55203b6c9 --- /dev/null +++ b/test_regress/t/t_force_select_bad.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2025 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('linter') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_force_select_bad.v b/test_regress/t/t_force_select_bad.v new file mode 100644 index 000000000..5b5bdb725 --- /dev/null +++ b/test_regress/t/t_force_select_bad.v @@ -0,0 +1,30 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2025 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +module t; + wire array1[2:1]; + wire [2:-1] vec; + + integer bad_index = 1; + + parameter P_ONE = 1; + + initial begin + force array1[P_ONE] = 1'b1; // ok + release array1[P_ONE]; // ok + force vec[P_ONE+:1] = 1'b1; // ok + release vec[P_ONE+:1]; // ok + + // IEEE 1800-2023 10.6.2 [A force] shall not be a bit-select or a + // part-select of a [non-constant] variable or of a net with a user-defined + // nettype. + force array1[bad_index] = 1'b1; // <---- BAD not constant index + release array1[bad_index]; // <---- BAD not constant index + force vec[bad_index+:1] = 1'b1; // <---- BAD not constant index + release vec[bad_index+:1]; // <---- BAD not constant index + end + +endmodule