From 680ef8dda931d540446bb0b486afdeb85239325a Mon Sep 17 00:00:00 2001 From: em2machine <92717390+em2machine@users.noreply.github.com> Date: Sat, 6 Jun 2026 17:55:47 +0200 Subject: [PATCH] Fix for HIERPARAM - relax checking (#7570) (#7690) --- src/V3AstAttr.h | 18 ++++++ src/V3Param.cpp | 29 ++++++---- test_regress/t/t_interface_hierparam_bits.py | 18 ++++++ test_regress/t/t_interface_hierparam_bits.v | 61 ++++++++++++++++++++ 4 files changed, 115 insertions(+), 11 deletions(-) create mode 100755 test_regress/t/t_interface_hierparam_bits.py create mode 100644 test_regress/t/t_interface_hierparam_bits.v diff --git a/src/V3AstAttr.h b/src/V3AstAttr.h index e912bae20..c12627886 100644 --- a/src/V3AstAttr.h +++ b/src/V3AstAttr.h @@ -370,6 +370,24 @@ public: // clang-format on return names[m_e]; } + // True for attributes that read an operand's type rather than its value, such as $bits. + bool isTypeQuery() const { + switch (m_e) { + case DIM_BITS: + case DIM_BITS_OR_NUMBER: + case DIM_DIMENSIONS: + case DIM_HIGH: + case DIM_INCREMENT: + case DIM_LEFT: + case DIM_LOW: + case DIM_RIGHT: + case DIM_SIZE: + case DIM_UNPK_DIMENSIONS: + case TYPEID: + case TYPENAME: return true; + default: return false; + } + } VAttrType() : m_e{ILLEGAL} {} // cppcheck-suppress noExplicitConstructor diff --git a/src/V3Param.cpp b/src/V3Param.cpp index fe2273d98..063c55d3a 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -2722,10 +2722,14 @@ class ParamVisitor final : public VNVisitor { // LCOV_EXCL_STOP } - void checkParamNotHier(AstNode* valuep) { - if (!valuep) return; - valuep->foreachAndNext([&](const AstNodeExpr* exprp) { - if (const AstVarXRef* const refp = VN_CAST(exprp, VarXRef)) { + // Flag hierarchical refs in a parameter value. Single top-down pass. + void checkParamNotHierRecurse(AstNode* nodep, bool underTypeQuery = false) { + for (; nodep; nodep = nodep->nextp()) { + // Refs read only for their type ($bits etc.) are allowed + const AstAttrOf* const attrp = VN_CAST(nodep, AttrOf); + const bool childUnderQuery + = underTypeQuery || (attrp && attrp->attrType().isTypeQuery()); + if (const AstVarXRef* const refp = VN_CAST(nodep, VarXRef)) { // Allow hierarchical ref to interface params through interface/modport ports // or local interface instances bool isIfaceRef = false; @@ -2735,18 +2739,21 @@ class ParamVisitor final : public VNVisitor { = !refname.empty() && (m_ifacePortNames.count(refname) || m_ifaceInstCells.count(refname)); } - - if (!isIfaceRef) { + if (!isIfaceRef && !underTypeQuery) { refp->v3warn(HIERPARAM, "Parameter values cannot use hierarchical values" " (IEEE 1800-2023 6.20.2)"); } - } else if (const AstNodeFTaskRef* refp = VN_CAST(exprp, NodeFTaskRef)) { + } else if (const AstNodeFTaskRef* const refp = VN_CAST(nodep, NodeFTaskRef)) { if (refp->dotted() != "") { refp->v3error("Parameter values cannot call hierarchical functions" " (IEEE 1800-2023 6.20.2)"); } } - }); + checkParamNotHierRecurse(nodep->op1p(), childUnderQuery); + checkParamNotHierRecurse(nodep->op2p(), childUnderQuery); + checkParamNotHierRecurse(nodep->op3p(), childUnderQuery); + checkParamNotHierRecurse(nodep->op4p(), childUnderQuery); + } } // Deparameterize and constify nested interface cells within ifaceModp. @@ -2888,7 +2895,7 @@ class ParamVisitor final : public VNVisitor { iterateChildren(nodep); } void visit(AstCell* nodep) override { - checkParamNotHier(nodep->paramsp()); + checkParamNotHierRecurse(nodep->paramsp()); if (VN_IS(nodep->modp(), Iface)) m_ifaceInstCells.emplace(nodep->name(), nodep); visitCellOrClassRef(nodep, VN_IS(nodep->modp(), Iface)); } @@ -2896,7 +2903,7 @@ class ParamVisitor final : public VNVisitor { if (nodep->ifacep()) visitCellOrClassRef(nodep, true); } void visit(AstClassRefDType* nodep) override { - checkParamNotHier(nodep->paramsp()); + checkParamNotHierRecurse(nodep->paramsp()); visitCellOrClassRef(nodep, false); } void visit(AstClassOrPackageRef* nodep) override { @@ -2913,7 +2920,7 @@ class ParamVisitor final : public VNVisitor { if (nodep->isIfaceRef()) { m_ifacePortNames.insert(nodep->name()); } iterateChildren(nodep); if (nodep->isParam()) { - checkParamNotHier(nodep->valuep()); + checkParamNotHierRecurse(nodep->valuep()); if (!nodep->valuep() && !VN_IS(m_modp, Class)) { nodep->v3error("Parameter without default value is never given value" << " (IEEE 1800-2023 6.20.1): " << nodep->prettyNameQ()); diff --git a/test_regress/t/t_interface_hierparam_bits.py b/test_regress/t/t_interface_hierparam_bits.py new file mode 100755 index 000000000..46d1fe4c0 --- /dev/null +++ b/test_regress/t/t_interface_hierparam_bits.py @@ -0,0 +1,18 @@ +#!/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('simulator') + +test.compile(verilator_flags2=['--binary']) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_interface_hierparam_bits.v b/test_regress/t/t_interface_hierparam_bits.v new file mode 100644 index 000000000..47e58e753 --- /dev/null +++ b/test_regress/t/t_interface_hierparam_bits.v @@ -0,0 +1,61 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// $bits() of interface member signals used as a child module parameter value. +// $bits reads the type, not the hierarchical value, so it is legal in a +// parameter (IEEE 1800-2023 6.20.2 forbids hierarchical values only). +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: CC0-1.0 + +// verilog_format: off +`define stop $stop +`define checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0) +// verilog_format: on + +interface axi_if #( + parameter int ID_W = 8, + parameter int ADDR_W = 32 +); + logic [ID_W-1:0] AWID; + logic [ADDR_W-1:0] AWADDR; + logic [7:0] AWLEN; +endinterface + +module chkmod #( + parameter int PAYLOAD_WIDTH = 1, + parameter int EXPECT = 1 +); + initial `checkh(PAYLOAD_WIDTH, EXPECT); +endmodule + +module bridge #( + parameter int EXPECT = 1 +) ( + axi_if axi +); + // $bits of a concat of interface members + chkmod #( + .PAYLOAD_WIDTH($bits({axi.AWID, axi.AWADDR, axi.AWLEN})), + .EXPECT(EXPECT) + ) u_concat (); + // $bits of a single interface member + chkmod #( + .PAYLOAD_WIDTH($bits(axi.AWADDR)), + .EXPECT(EXPECT - 12 - 8) + ) u_single (); +endmodule + +module t; + axi_if #(.ID_W(12), .ADDR_W(64)) if0 (); // 12 + 64 + 8 = 84 + axi_if #(.ID_W(12), .ADDR_W(16)) if1 (); // 12 + 16 + 8 = 36 + + bridge #(.EXPECT(84)) dut0 (.axi(if0)); + bridge #(.EXPECT(36)) dut1 (.axi(if1)); + + initial begin + #1; + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule