From 51fcb881d56a155398d9d0dcc4463cb550ee7f3d Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 8 Mar 2025 11:05:58 -0500 Subject: [PATCH] Fix error on out-of-range lvalue part select (#5820). --- Changes | 1 + src/V3AstNodeExpr.h | 1 + src/V3AstNodes.cpp | 17 ++++++++++++++++- src/V3Width.cpp | 29 ++++++++++++++++++++++------- test_regress/t/t_bitsel_lvalue.py | 16 ++++++++++++++++ test_regress/t/t_bitsel_lvalue.v | 20 ++++++++++++++++++++ 6 files changed, 76 insertions(+), 8 deletions(-) create mode 100755 test_regress/t/t_bitsel_lvalue.py create mode 100644 test_regress/t/t_bitsel_lvalue.v diff --git a/Changes b/Changes index 0ef80005d..c4fc2df6f 100644 --- a/Changes +++ b/Changes @@ -27,6 +27,7 @@ Verilator 5.035 devel * Fix invalid code motion over branches (#5811) (#5814). [Geza Lore] * Fix sorting of wide SenItems (#5816). [Geza Lore] * Fix tcmalloc static link and non-22.04 builds (#5817) (#5818). [Geza Lore] +* Fix error on out-of-range lvalue part select (#5820). * Fix UNOPTFLAT warnings with `--coverage-trace` and always_comb (#5821). * Fix function locals in SenExprBuilder (#5822). [Geza Lore] * Fix type_id package scope resolution (#5826). [Krzysztof Bieganski, Antmicro Ltd.] diff --git a/src/V3AstNodeExpr.h b/src/V3AstNodeExpr.h index 5074373aa..3ef5eb4b3 100644 --- a/src/V3AstNodeExpr.h +++ b/src/V3AstNodeExpr.h @@ -542,6 +542,7 @@ public: // Know no children, and hot function, so skip iterator for speed // cppcheck-suppress functionConst void iterateChildren(VNVisitorConst& v) {} + static AstNodeVarRef* varRefLValueRecurse(AstNode* nodep); }; // === Concrete node types ===================================================== diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 62db4bbc9..461d39045 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -824,7 +824,6 @@ AstVar* AstVar::scVarRecurse(AstNode* nodep) { } } else if (AstArraySel* const arraySelp = VN_CAST(nodep, ArraySel)) { if (AstVar* const p = scVarRecurse(arraySelp->fromp())) return p; - if (AstVar* const p = scVarRecurse(arraySelp->bitp())) return p; } return nullptr; } @@ -2484,6 +2483,22 @@ void AstNodeVarRef::dumpJson(std::ostream& str) const { dumpJsonStr(str, "access", access().ascii()); dumpJsonGen(str); } +AstNodeVarRef* AstNodeVarRef::varRefLValueRecurse(AstNode* nodep) { + // Given a (possible) lvalue expression, recurse to find the being-set NodeVarRef, else nullptr + if (AstNodeVarRef* const anodep = VN_CAST(nodep, NodeVarRef)) return anodep; + if (AstNodeSel* const anodep = VN_CAST(nodep, NodeSel)) + return varRefLValueRecurse(anodep->fromp()); + if (AstSel* const anodep = VN_CAST(nodep, Sel)) + return varRefLValueRecurse(anodep->fromp()); + if (AstArraySel* const anodep = VN_CAST(nodep, ArraySel)) + return varRefLValueRecurse(anodep->fromp()); + if (AstMemberSel* const anodep = VN_CAST(nodep, MemberSel)) + return varRefLValueRecurse(anodep->fromp()); + if (AstStructSel* const anodep = VN_CAST(nodep, StructSel)) + return varRefLValueRecurse(anodep->fromp()); + return nullptr; +} + void AstVarXRef::dump(std::ostream& str) const { this->AstNodeVarRef::dump(str); str << ".=" << dotted() << " "; diff --git a/src/V3Width.cpp b/src/V3Width.cpp index e34f07461..83f6e3e1a 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -962,6 +962,7 @@ class WidthVisitor final : public VNVisitor { return; } UASSERT_OBJ(nodep->dtypep(), nodep, "dtype wasn't set"); // by V3WidthSel + if (VN_IS(nodep->lsbp(), Const) && nodep->msbConst() < nodep->lsbConst()) { // Likely impossible given above width check nodep->v3warn(E_UNSUPPORTED, @@ -1027,20 +1028,33 @@ class WidthVisitor final : public VNVisitor { // evaluating type sizes for a generate block condition. We // should only trigger the error if the out-of-range access is // actually generated. + AstNodeVarRef* lrefp = AstNodeVarRef::varRefLValueRecurse(nodep); if (m_doGenerate) { - UINFO(5, "Selection index out of range inside generate." << endl); + UINFO(5, "Selection index out of range inside generate\n"); } else { nodep->v3warn(SELRANGE, "Selection index out of range: " << nodep->msbConst() << ":" << nodep->lsbConst() << " outside " << frommsb << ":" << fromlsb); UINFO(1, " Related node: " << nodep << endl); } - // Extend it. - const int extendTo = nodep->msbConst() + 1; - AstNodeDType* const subDTypep = nodep->findLogicDType( - extendTo, extendTo, nodep->fromp()->dtypep()->numeric()); - widthCheckSized(nodep, "errorless...", nodep->fromp(), subDTypep, EXTEND_EXP, - false /*noerror*/); + if (lrefp) UINFO(9, " Select extend lrefp " << lrefp << endl); + if (lrefp && lrefp->access().isWriteOrRW()) { + // lvarref[X] = ..., the expression assigned is too wide + // WTF to do + // Don't change the width of this lhsp, instead propagate up + // to upper assign/expression the correct width + AstNodeDType* const subDTypep + = nodep->findLogicDType(width, width, nodep->fromp()->dtypep()->numeric()); + widthCheckSized(nodep, "errorless...", nodep->fromp(), subDTypep, EXTEND_EXP, + false /*noerror*/); + } else { + // Extend it + const int extendTo = nodep->msbConst() + 1; + AstNodeDType* const subDTypep = nodep->findLogicDType( + extendTo, extendTo, nodep->fromp()->dtypep()->numeric()); + widthCheckSized(nodep, "errorless...", nodep->fromp(), subDTypep, EXTEND_EXP, + false /*noerror*/); + } } // iterate FINAL is two blocks above // @@ -1055,6 +1069,7 @@ class WidthVisitor final : public VNVisitor { widthCheckSized(nodep, "Extract Range", nodep->lsbp(), selwidthDTypep, EXTEND_EXP, false /*NOWARN*/); } + // if (debug() >= 9) nodep->dumpTree("-seldone "); } } diff --git a/test_regress/t/t_bitsel_lvalue.py b/test_regress/t/t_bitsel_lvalue.py new file mode 100755 index 000000000..147fe6faf --- /dev/null +++ b/test_regress/t/t_bitsel_lvalue.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('simulator') + +test.compile() + +test.passes() diff --git a/test_regress/t/t_bitsel_lvalue.v b/test_regress/t/t_bitsel_lvalue.v new file mode 100644 index 000000000..83359a594 --- /dev/null +++ b/test_regress/t/t_bitsel_lvalue.v @@ -0,0 +1,20 @@ +// 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 + ( + input wire [ 31:0] foo, + output reg [144:0] bar, + output reg [144:0] bar2, + output reg [144:0] bar3, + output reg [144:0] bar4 + ); + // verilator lint_off SELRANGE + assign bar[159:128] = foo; + assign bar2[159] = foo[1]; + assign bar3[159 -: 32] = foo; + assign bar4[128 +: 32] = foo; +endmodule