From e27613ed452e22a80facd9f1baa16ececa695d89 Mon Sep 17 00:00:00 2001 From: Paul Swirhun <63216497+paul-demo@users.noreply.github.com> Date: Fri, 31 Oct 2025 20:06:26 -0700 Subject: [PATCH] Fix interface parameter access in parameter map (#6587) (#6621) (#6623) Co-authored-by: Paul Swirhun --- src/V3Param.cpp | 40 ++++-- test_regress/t/t_interface_param_dependency.v | 125 +++++++++++++++++- test_regress/t/t_interface_parameter_access.v | 7 + 3 files changed, 161 insertions(+), 11 deletions(-) diff --git a/src/V3Param.cpp b/src/V3Param.cpp index ed1f6ba55..4a197ec8d 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -1366,20 +1366,22 @@ class ParamVisitor final : public VNVisitor { m_iterateModule = false; } + // Extract the base reference name from a dotted VarXRef (e.g., "iface.FOO" -> "iface") + string getRefBaseName(const AstVarXRef* refp) { + const string dotted = refp->dotted(); + if (dotted.empty()) return ""; + return dotted.substr(0, dotted.find('.')); + } + void checkParamNotHier(AstNode* valuep) { if (!valuep) return; valuep->foreachAndNext([&](const AstNodeExpr* exprp) { 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); - } + const string refname = getRefBaseName(refp); + isIfacePortRef = !refname.empty() && m_ifacePortNames.count(refname); } if (!isIfacePortRef) { @@ -1395,6 +1397,23 @@ class ParamVisitor final : public VNVisitor { }); } + // Check if cell parameters reference interface ports + bool cellParamsReferenceIfacePorts(AstCell* cellp) { + if (!cellp->paramsp()) return false; + + for (AstNode* nodep = cellp->paramsp(); nodep; nodep = nodep->nextp()) { + if (AstPin* const pinp = VN_CAST(nodep, Pin)) { + if (AstNode* const exprp = pinp->exprp()) { + if (const AstVarXRef* const refp = VN_CAST(exprp, VarXRef)) { + const string refname = getRefBaseName(refp); + if (!refname.empty() && m_ifacePortNames.count(refname)) return true; + } + } + } + } + return false; + } + // A generic visitor for cells and class refs void visitCellOrClassRef(AstNode* nodep, bool isIface) { // Must do ifaces first, so push to list and do in proper order @@ -1402,10 +1421,13 @@ class ParamVisitor final : public VNVisitor { nodep->user2p(&m_strings.back()); // For interface cells with parameters, specialize first before processing children + // Only do early specialization if parameters don't reference interface ports 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()); + if (!cellParamsReferenceIfacePorts(cellp)) { + AstNodeModule* const srcModp = cellp->modp(); + m_processor.nodeDeparam(cellp, srcModp, m_modp, m_modp->someInstanceName()); + } } // Visit parameters in the instantiation. diff --git a/test_regress/t/t_interface_param_dependency.v b/test_regress/t/t_interface_param_dependency.v index dc6beff75..f90d24a59 100644 --- a/test_regress/t/t_interface_param_dependency.v +++ b/test_regress/t/t_interface_param_dependency.v @@ -33,8 +33,7 @@ module submod_modport ( assign result = mp.FOO + mp.BAR; endmodule -// Test module that asserts interface parameter values - catches dependency bugs -module submod_assert #( +module submod_assert2 #( parameter int EXPECTED_FOO = 0, parameter int EXPECTED_BAR = 0 ) ( @@ -51,6 +50,128 @@ module submod_assert #( end 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 +); + // Make a new interface with inherited parameters and pass it to a submodule + // with inherited expectations. + TEST_IF #( + .FOO(iface.FOO), + .BAR(iface.BAR) + ) iface2 (); + + // Test mixed parameters: constant + interface port reference + TEST_IF #( + .FOO(7), // Constant parameter + .BAR(iface.FOO) // References interface port + ) iface_mixed (); + + // Test specifying only FOO parameter (BAR should use default calculation) + TEST_IF #( + .FOO(iface.FOO) // Only FOO specified, BAR = FOO * 10 + ) iface_foo_only (); + + // Test specifying only BAR parameter (FOO should use default) + TEST_IF #( + .BAR(iface.BAR) // Only BAR specified, FOO = default (1) + ) iface_bar_only (); + + // Test no parameters specified (both should use defaults) + TEST_IF iface_defaults (); + + submod_assert2 #( + .EXPECTED_FOO(EXPECTED_FOO), + .EXPECTED_BAR(EXPECTED_BAR) + ) u_submod_assert2 ( + .iface(iface2) + ); + + // Test the mixed parameter interface + submod_assert2 #( + .EXPECTED_FOO(7), + .EXPECTED_BAR(EXPECTED_FOO) // BAR should get iface.FOO value + ) u_mixed_assert ( + .iface(iface_mixed) + ); + + // Test FOO-only interface + submod_assert2 #( + .EXPECTED_FOO(EXPECTED_FOO), + .EXPECTED_BAR(EXPECTED_FOO * 10) // BAR = FOO * 10 + ) u_foo_only_assert ( + .iface(iface_foo_only) + ); + + // Test BAR-only interface + submod_assert2 #( + .EXPECTED_FOO(1), // FOO = default + .EXPECTED_BAR(EXPECTED_BAR) // BAR = specified value + ) u_bar_only_assert ( + .iface(iface_bar_only) + ); + + // Test defaults interface + submod_assert2 #( + .EXPECTED_FOO(1), // FOO = default + .EXPECTED_BAR(10) // BAR = FOO * 10 = 1 * 10 + ) u_defaults_assert ( + .iface(iface_defaults) + ); + + 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 + + // Verify all interface instances have correct parameter values + if (iface2.FOO != EXPECTED_FOO) begin + $error("iface2.FOO mismatch: expected %0d, got %0d", EXPECTED_FOO, iface2.FOO); + end + if (iface2.BAR != EXPECTED_BAR) begin + $error("iface2.BAR mismatch: expected %0d, got %0d", EXPECTED_BAR, iface2.BAR); + end + + if (iface_mixed.FOO != 7) begin + $error("iface_mixed.FOO mismatch: expected 7, got %0d", iface_mixed.FOO); + end + if (iface_mixed.BAR != EXPECTED_FOO) begin + $error("iface_mixed.BAR mismatch: expected %0d, got %0d", EXPECTED_FOO, iface_mixed.BAR); + end + + if (iface_foo_only.FOO != EXPECTED_FOO) begin + $error("iface_foo_only.FOO mismatch: expected %0d, got %0d", EXPECTED_FOO, + iface_foo_only.FOO); + end + if (iface_foo_only.BAR != EXPECTED_FOO * 10) begin + $error("iface_foo_only.BAR mismatch: expected %0d, got %0d", EXPECTED_FOO * 10, + iface_foo_only.BAR); + end + + if (iface_bar_only.FOO != 1) begin + $error("iface_bar_only.FOO mismatch: expected 1, got %0d", iface_bar_only.FOO); + end + if (iface_bar_only.BAR != EXPECTED_BAR) begin + $error("iface_bar_only.BAR mismatch: expected %0d, got %0d", EXPECTED_BAR, + iface_bar_only.BAR); + end + + if (iface_defaults.FOO != 1) begin + $error("iface_defaults.FOO mismatch: expected 1, got %0d", iface_defaults.FOO); + end + if (iface_defaults.BAR != 10) begin + $error("iface_defaults.BAR mismatch: expected 10, got %0d", iface_defaults.BAR); + end + end +endmodule + // Test parameterized interface chain: module parameter -> interface parameter -> submodule module param_chain #( parameter int TOP_PARAM = 3 diff --git a/test_regress/t/t_interface_parameter_access.v b/test_regress/t/t_interface_parameter_access.v index cd3851c48..c7dd80d4b 100644 --- a/test_regress/t/t_interface_parameter_access.v +++ b/test_regress/t/t_interface_parameter_access.v @@ -70,6 +70,7 @@ endmodule module testmod + #(parameter SOME_PARAM = 789) ( input clk, test_if.mp intf, @@ -77,6 +78,8 @@ module testmod test_if.mp intf_array [1:0] ); + test_if #(.FOO (intf.FOO)) some_other_intf (); + // verilator lint_off HIERPARAM localparam THE_FOO = intf.FOO; localparam THE_OTHER_FOO = intf_no_mp.FOO; @@ -91,6 +94,10 @@ module testmod $display("%%Error: THE_FOO = %0d", THE_FOO); $stop; end + if (some_other_intf.FOO != 5) begin + $display("%%Error: some_other_intf.FOO = %0d", some_other_intf.FOO); + $stop; + end if (THE_OTHER_FOO != 5) begin $display("%%Error: THE_OTHER_FOO = %0d", THE_OTHER_FOO); $stop;