Fix HIERPARAM to be suppressed for interface ports (#6587) (#6609)

Co-authored-by: Paul Swirhun <paulswirhun@gmail.com>
This commit is contained in:
Paul Swirhun 2025-10-31 12:49:30 -07:00 committed by GitHub
parent 922223a9c3
commit 10935ee031
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 266 additions and 27 deletions

View File

@ -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

View File

@ -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(); }

View File

@ -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;

View File

@ -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<std::string> 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;
}
}

View File

@ -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

View File

@ -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 ();

View File

@ -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

View File

@ -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

View File

@ -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()

View File

@ -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