diff --git a/docs/guide/warnings.rst b/docs/guide/warnings.rst index d724a834e..eb5d15d5d 100644 --- a/docs/guide/warnings.rst +++ b/docs/guide/warnings.rst @@ -933,6 +933,9 @@ List Of Warnings reference the parameter of a lower module in a way that affects determining the parameters that elaborate that lower module. + An exception is made for IEEE 1800-2023 25.10 for interfaces/modports which appear in a module's + port list, since these are references to interfaces/modports declared at a higher level and are + already specialized. These types of accesses do not require waiving HIERPARAM. .. option:: IFDEPTH diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index cf43250c8..a335e579a 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1876,6 +1876,7 @@ class AstVar final : public AstNode { bool m_isPullup : 1; // Tri1 bool m_isIfaceParent : 1; // dtype is reference to interface present in this module bool m_isInternal : 1; // Internal state, don't add to method pinter + bool m_isIfaceParam : 1; // Parameter belongs to an interface/modport bool m_isDpiOpenArray : 1; // DPI import open array bool m_isHideLocal : 1; // Verilog local bool m_isHideProtected : 1; // Verilog protected @@ -1925,6 +1926,7 @@ class AstVar final : public AstNode { m_isPullup = false; m_isIfaceParent = false; m_isInternal = false; + m_isIfaceParam = false; m_isDpiOpenArray = false; m_isHideLocal = false; m_isHideProtected = false; @@ -2064,6 +2066,7 @@ public: void isStatic(bool flag) { m_isStatic = flag; } void isIfaceParent(bool flag) { m_isIfaceParent = flag; } void isInternal(bool flag) { m_isInternal = flag; } + void isIfaceParam(bool flag) { m_isIfaceParam = flag; } void funcLocal(bool flag) { m_funcLocal = flag; if (flag) m_funcLocalSticky = true; @@ -2133,6 +2136,7 @@ public: } bool isIfaceParent() const { return m_isIfaceParent; } bool isInternal() const { return m_isInternal; } + bool isIfaceParam() const { return m_isIfaceParam; } bool isSignal() const { return varType().isSignal(); } bool isNet() const { return varType().isNet(); } bool isWor() const { return varType().isWor(); } diff --git a/src/V3LinkParse.cpp b/src/V3LinkParse.cpp index fba477e6a..8e05441f8 100644 --- a/src/V3LinkParse.cpp +++ b/src/V3LinkParse.cpp @@ -53,6 +53,7 @@ class LinkParseVisitor final : public VNVisitor { AstVar* m_varp = nullptr; // Variable we're under AstNodeModule* m_valueModp = nullptr; AstNodeModule* m_modp = nullptr; // Current module + bool m_inInterface = false; // True when inside interface declaration AstNodeProcedure* m_procedurep = nullptr; // Current procedure AstNodeFTask* m_ftaskp = nullptr; // Current task AstNodeBlock* m_blockp = nullptr; // Current AstNodeBlock @@ -328,6 +329,9 @@ class LinkParseVisitor final : public VNVisitor { nodep->v3warn(NEWERSTD, "Parameter requires default value, or use IEEE 1800-2009 or later."); } + + // Mark parameters declared inside interfaces + if (nodep->isParam() && m_inInterface) { nodep->isIfaceParam(true); } if (AstParseTypeDType* const ptypep = VN_CAST(nodep->subDTypep(), ParseTypeDType)) { // It's a parameter type. Use a different node type for this. AstNode* dtypep = nodep->valuep(); @@ -632,6 +636,7 @@ class LinkParseVisitor final : public VNVisitor { } VL_RESTORER(m_modp); + VL_RESTORER(m_inInterface); VL_RESTORER(m_anonUdpId); VL_RESTORER(m_genblkAbove); VL_RESTORER(m_genblkNum); @@ -647,6 +652,7 @@ class LinkParseVisitor final : public VNVisitor { // Classes inherit from upper package if (m_modp && nodep->timeunit().isNone()) nodep->timeunit(m_modp->timeunit()); m_modp = nodep; + if (VN_IS(nodep, Iface)) m_inInterface = true; // Start or stay within interface m_anonUdpId = 0; m_genblkAbove = 0; m_genblkNum = 0; diff --git a/src/V3Param.cpp b/src/V3Param.cpp index 665c0f4e7..ed1f6ba55 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -1282,6 +1282,7 @@ class ParamVisitor final : public VNVisitor { // STATE - for current visit position (use VL_RESTORER) AstNodeModule* m_modp; // Module iterating + std::unordered_set m_ifacePortNames; // Interface port names in current module string m_generateHierName; // Generate portion of hierarchy name // METHODS @@ -1312,7 +1313,9 @@ class ParamVisitor final : public VNVisitor { // Iterate the body { VL_RESTORER(m_modp); + VL_RESTORER(m_ifacePortNames); m_modp = modp; + m_ifacePortNames.clear(); iterateChildren(modp); } } @@ -1366,9 +1369,23 @@ class ParamVisitor final : public VNVisitor { void checkParamNotHier(AstNode* valuep) { if (!valuep) return; valuep->foreachAndNext([&](const AstNodeExpr* exprp) { - if (const AstVarXRef* refp = VN_CAST(exprp, VarXRef)) { - refp->v3warn(HIERPARAM, "Parameter values cannot use hierarchical values" - " (IEEE 1800-2023 6.20.2)"); + if (const AstVarXRef* const refp = VN_CAST(exprp, VarXRef)) { + // Allow hierarchical ref to interface params through interface/modport ports + // (always), and also allow to locally declared interfaces if the user has + // waived HIERPARAM. + bool isIfacePortRef = false; + if (refp->varp() && refp->varp()->isIfaceParam()) { + const string dotted = refp->dotted(); + if (!dotted.empty()) { + const string refname = dotted.substr(0, dotted.find('.')); + isIfacePortRef = m_ifacePortNames.count(refname); + } + } + + if (!isIfacePortRef) { + refp->v3warn(HIERPARAM, "Parameter values cannot use hierarchical values" + " (IEEE 1800-2023 6.20.2)"); + } } else if (const AstNodeFTaskRef* refp = VN_CAST(exprp, NodeFTaskRef)) { if (refp->dotted() != "") { refp->v3error("Parameter values cannot call hierarchical functions" @@ -1383,6 +1400,14 @@ class ParamVisitor final : public VNVisitor { // Must do ifaces first, so push to list and do in proper order m_strings.emplace_back(m_generateHierName); nodep->user2p(&m_strings.back()); + + // For interface cells with parameters, specialize first before processing children + if (isIface && VN_CAST(nodep, Cell) && VN_CAST(nodep, Cell)->paramsp()) { + AstCell* const cellp = VN_CAST(nodep, Cell); + AstNodeModule* const srcModp = cellp->modp(); + m_processor.nodeDeparam(cellp, srcModp, m_modp, m_modp->someInstanceName()); + } + // Visit parameters in the instantiation. iterateChildren(nodep); m_cellps.emplace(!isIface, nodep); @@ -1462,6 +1487,8 @@ class ParamVisitor final : public VNVisitor { // Make sure all parameters are constantified void visit(AstVar* nodep) override { if (nodep->user2SetOnce()) return; // Process once + // Build cache of interface port names as we encounter them + if (nodep->isIfaceRef()) { m_ifacePortNames.insert(nodep->name()); } iterateChildren(nodep); if (nodep->isParam()) { checkParamNotHier(nodep->valuep()); @@ -1548,7 +1575,7 @@ class ParamVisitor final : public VNVisitor { else if (const AstCell* const cellp = ifacerefp->cellp()) { if (dotted == cellp->name()) { UINFO(9, "Iface matching scope: " << cellp); - if (ifaceParamReplace(nodep, cellp->paramsp())) { // + if (ifaceParamReplace(nodep, cellp->modp()->stmtsp())) { // return; } } diff --git a/test_regress/t/t_interface_array_parameter_access.v b/test_regress/t/t_interface_array_parameter_access.v index 27ce16119..7b95cb7fe 100644 --- a/test_regress/t/t_interface_array_parameter_access.v +++ b/test_regress/t/t_interface_array_parameter_access.v @@ -11,13 +11,13 @@ interface intf modport modp (input clk, rst); endinterface -module sub (intf.modp the_intf_port [4]); - // verilator lint_off HIERPARAM +module sub (intf.modp the_intf_port [4], intf.modp single_intf_port); localparam intf_foo = the_intf_port[0].FOO; - // verilator lint_on HIERPARAM + localparam single_foo = single_intf_port.FOO; initial begin if (intf_foo != 4) $stop; + if (single_foo != 4) $stop; end endmodule @@ -28,10 +28,12 @@ module t ( input clk; intf the_intf [4] (.*); + intf single_intf (.*); sub the_sub ( - .the_intf_port (the_intf) + .the_intf_port (the_intf), + .single_intf_port(single_intf) ); always @(posedge clk) begin diff --git a/test_regress/t/t_interface_localparam.v b/test_regress/t/t_interface_localparam.v index d6d8a06ad..e7ba1810c 100644 --- a/test_regress/t/t_interface_localparam.v +++ b/test_regress/t/t_interface_localparam.v @@ -35,9 +35,7 @@ module Core( ); // this will constify and valDiv2 will have the default value - // verilator lint_off HIERPARAM localparam valDiv4Upper = intf.valDiv2; - // verilator lint_on HIERPARAM SimpleIntf #(.VAL(68)) core_intf (); diff --git a/test_regress/t/t_interface_param_another_bad.out b/test_regress/t/t_interface_param_another_bad.out index 4d558d4f3..0b09b0b3e 100644 --- a/test_regress/t/t_interface_param_another_bad.out +++ b/test_regress/t/t_interface_param_another_bad.out @@ -1,15 +1,15 @@ -%Error-HIERPARAM: t/t_interface_param_another_bad.v:9:36: Parameter values cannot use hierarchical values (IEEE 1800-2023 6.20.2) +%Error-HIERPARAM: t/t_interface_param_another_bad.v:9:35: Parameter values cannot use hierarchical values (IEEE 1800-2023 6.20.2) : ... note: In instance 't' - 9 | simple_bus #(.PARAMETER(sb_intf.dummy)) simple(); - | ^~~~~ + 9 | simple_bus #(.PARAMETER(sb_intf.dummy)) simple (); + | ^~~~~ ... For error description see https://verilator.org/warn/HIERPARAM?v=latest -%Error: t/t_interface_param_another_bad.v:9:36: Expecting expression to be constant, but variable isn't const: 'dummy' +%Error: t/t_interface_param_another_bad.v:9:35: Expecting expression to be constant, but variable isn't const: 'dummy' : ... note: In instance 't' - 9 | simple_bus #(.PARAMETER(sb_intf.dummy)) simple(); - | ^~~~~ + 9 | simple_bus #(.PARAMETER(sb_intf.dummy)) simple (); + | ^~~~~ ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. -%Error: t/t_interface_param_another_bad.v:9:18: Can't convert defparam value to constant: Param 'PARAMETER' of 'simple' +%Error: t/t_interface_param_another_bad.v:9:17: Can't convert defparam value to constant: Param 'PARAMETER' of 'simple' : ... note: In instance 't' - 9 | simple_bus #(.PARAMETER(sb_intf.dummy)) simple(); - | ^~~~~~~~~ + 9 | simple_bus #(.PARAMETER(sb_intf.dummy)) simple (); + | ^~~~~~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_interface_param_another_bad.v b/test_regress/t/t_interface_param_another_bad.v index 15dea9764..e432a507c 100644 --- a/test_regress/t/t_interface_param_another_bad.v +++ b/test_regress/t/t_interface_param_another_bad.v @@ -5,14 +5,16 @@ // SPDX-License-Identifier: CC0-1.0 module t (); - simple_bus sb_intf(); - simple_bus #(.PARAMETER(sb_intf.dummy)) simple(); - initial begin - $write("*-* All Finished *-*\n"); - $finish; - end + simple_bus sb_intf (); + simple_bus #(.PARAMETER(sb_intf.dummy)) simple (); + initial begin + $write("*-* All Finished *-*\n"); + $finish; + end endmodule -interface simple_bus #(PARAMETER = 0); - logic dummy; +interface simple_bus #( + PARAMETER = 0 +); + logic dummy; endinterface diff --git a/test_regress/t/t_interface_param_dependency.py b/test_regress/t/t_interface_param_dependency.py new file mode 100755 index 000000000..daf5a778c --- /dev/null +++ b/test_regress/t/t_interface_param_dependency.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Test interface parameter dependency resolution +# +# 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_st") + +test.compile() + +test.execute() + +test.passes() diff --git a/test_regress/t/t_interface_param_dependency.v b/test_regress/t/t_interface_param_dependency.v new file mode 100644 index 000000000..dc6beff75 --- /dev/null +++ b/test_regress/t/t_interface_param_dependency.v @@ -0,0 +1,179 @@ +// DESCRIPTION: Test interface parameter dependency resolution +// +// Test that interface/modport parameters can be accessed when the +// interface/modport is an IO port of the module. +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2025 by Paul Swirhun +// SPDX-License-Identifier: CC0-1.0 + +`define stop $stop +`define checkd(gotv, + expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0d exp=%0d\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0); + +interface TEST_IF #( + parameter int FOO = 1, + parameter int BAR = FOO * 10 +); + logic [31:0] data; + modport mp(input data); +endinterface + +module submod_iface ( + output logic [31:0] result, + TEST_IF iface +); + assign result = iface.FOO + iface.BAR; +endmodule + +module submod_modport ( + output logic [31:0] result, + TEST_IF.mp mp +); + assign result = mp.FOO + mp.BAR; +endmodule + +// Test module that asserts interface parameter values - catches dependency bugs +module submod_assert #( + parameter int EXPECTED_FOO = 0, + parameter int EXPECTED_BAR = 0 +) ( + TEST_IF iface +); + initial begin + // Verify the dependent parameter BAR is correctly computed in the module + if (iface.FOO != EXPECTED_FOO) begin + $error("FOO mismatch in module: expected %0d, got %0d", EXPECTED_FOO, iface.FOO); + end + if (iface.BAR != EXPECTED_BAR) begin + $error("BAR dependency failed in module: expected %0d, got %0d", EXPECTED_BAR, iface.BAR); + end + end +endmodule + +// Test parameterized interface chain: module parameter -> interface parameter -> submodule +module param_chain #( + parameter int TOP_PARAM = 3 +) ( + output logic [31:0] result +); + // Interface gets parameter from module parameter + TEST_IF #(.FOO(TOP_PARAM)) chain_iface (); + + // Submodule uses interface (FOO=3, BAR should be 30) + submod_iface chain_sub ( + .result(result), + .iface (chain_iface) + ); + + // Assert the chain works correctly + submod_assert #( + .EXPECTED_FOO(TOP_PARAM), + .EXPECTED_BAR(TOP_PARAM * 10) + ) chain_assert ( + .iface(chain_iface) + ); +endmodule + + +module t; + // Test case 1: FOO specified, BAR should be FOO*10 + TEST_IF #(.FOO(5)) tif_1 (); + + // Test case 2: Both FOO and BAR specified explicitly + TEST_IF #( + .FOO(6), + .BAR(66) + ) tif_2 (); + + // Test case 3: Only BAR specified, FOO should be default + TEST_IF #(.BAR(77)) tif_3 (); + + // Test case 4: Default parameters + TEST_IF tif_4 (); + + logic [8:0][31:0] result; + + // Test interface as port parameter + submod_iface u0 ( + .result(result[0]), + .iface (tif_1) + ); + submod_iface u1 ( + .result(result[1]), + .iface (tif_2) + ); + submod_iface u2 ( + .result(result[2]), + .iface (tif_3) + ); + submod_iface u3 ( + .result(result[3]), + .iface (tif_4) + ); + + // Test modport as port parameter + submod_modport u4 ( + .result(result[4]), + .mp(tif_1) + ); + submod_modport u5 ( + .result(result[5]), + .mp(tif_2) + ); + submod_modport u6 ( + .result(result[6]), + .mp(tif_3) + ); + submod_modport u7 ( + .result(result[7]), + .mp(tif_4) + ); + + // Test that interface parameter dependencies are correctly resolved in modules + submod_assert #( + .EXPECTED_FOO(5), + .EXPECTED_BAR(50) + ) assert1 ( + .iface(tif_1) + ); + + // Test parameterized interface chain: module param -> interface param -> submodule + param_chain #(.TOP_PARAM(4)) chain_test (.result(result[8])); + + // Allow hierarchichal references to locally declared interfaces only when HIERPARAM is waived + /* verilator lint_off HIERPARAM */ + TEST_IF #(.FOO(3)) test_if_local (); + logic [31:0] foo_local_1 = 32'(test_if_local.FOO); + logic [31:0] bar_local_1 = 32'(test_if_local.BAR); + localparam FOO_LOCAL = test_if_local.FOO; + localparam BAR_LOCAL = test_if_local.BAR; + logic [31:0] foo_local_2 = 32'(FOO_LOCAL); + logic [31:0] bar_local_2 = 32'(BAR_LOCAL); + /* verilator lint_on HIERPARAM */ + + initial begin + // Verify modules can access interface parameters correctly + `checkd(result[0], 55); // 5 + 50 + `checkd(result[1], 72); // 6 + 66 + `checkd(result[2], 78); // 1 + 77 (FOO default + BAR explicit) + `checkd(result[3], 11); // 1 + 10 (both defaults, BAR = FOO*10) + + // Verify modport access gives same results + `checkd(result[4], 55); // 5 + 50 + `checkd(result[5], 72); // 6 + 66 + `checkd(result[6], 78); // 1 + 77 + `checkd(result[7], 11); // 1 + 10 + + // Verify parameterized chain works + `checkd(result[8], 44); // 4 + 40 (TOP_PARAM=4, so FOO=4, BAR=40) + + `checkd(foo_local_1, 3); + `checkd(bar_local_1, 30); + `checkd(foo_local_2, 3); + `checkd(bar_local_2, 30); + + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule