luke
/
verilator
mirror of https://github.com/verilator/verilator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix wrong $bits() for parameterized interface struct typedefs (#7218) (#7219)

This commit is contained in:
em2machine 2026-03-09 22:32:13 -04:00 committed by GitHub
parent 1f67080a1f
commit 1b2b8afdc1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 321 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
src/V3Param.cpp
View File

@ -800,7 +800,8 @@ class ParamProcessor final {
const string lastCompBase
= (braPos == string::npos) ? lastComp : lastComp.substr(0, braPos);
if (lastComp != cloneCellp->name() && lastCompBase != cloneCellp->name()) return false;
if (lastDot == string::npos) return true; // No parent portion to verify
// No parent portion to verify - startModp itself must be the expected parent
if (lastDot == string::npos) return startModp == expectModp;
const string parentPath = cellPath.substr(0, lastDot);
const AstNodeModule* const resolvedp
= V3LinkDotIfaceCapture::followCellPath(startModp, parentPath);
@ -869,6 +870,7 @@ class ParamProcessor final {
<< (correctModp ? correctModp->name() : "<null>")
<< endl);
if (!correctModp || correctModp->dead()) return;
if (correctModp->parameterizedTemplate()) return;
bool fixed = false;
if (refp->typedefp()) {
@ -1292,8 +1294,10 @@ class ParamProcessor final {
if (resolvedp && (VN_IS(resolvedp, StructDType) || VN_IS(resolvedp, UnionDType))) {
AstNodeModule* const ownerModp
= V3LinkDotIfaceCapture::findOwnerModule(resolvedp);
// Skip if owned by a parameterized template (not yet specialized)
if (ownerModp && ownerModp->parameterizedTemplate()) {
// Skip if owned by a parameterized interface (template or not
// yet cloned). hasGParam() covers interfaces that haven't
// been cloned yet (parameterizedTemplate() not set).
if (ownerModp && VN_IS(ownerModp, Iface) && ownerModp->hasGParam()) {
skipWidthForTemplateStruct = true;
V3Stats::addStatSum("Param, Template struct width skips", 1);
UINFO(9, "SKIP-WIDTH-TEMPLATE: struct="
@ -1776,6 +1780,28 @@ class ParamProcessor final {
}
public:
// After an interface cell inside parentModp has been deparameterized
// (rewired from template to clone), retarget REFDTYPEs that still
// reference the old template's types so that $bits(iface_typedef)
// evaluates with the clone's widths.
void retargetIfaceRefs(AstNodeModule* parentModp, const string& cellName) {
AstNodeModule* const correctModp
= V3LinkDotIfaceCapture::followCellPath(parentModp, cellName);
if (!correctModp || correctModp->dead() || correctModp->parameterizedTemplate()) return;
V3LinkDotIfaceCapture::forEach([&](const V3LinkDotIfaceCapture::CapturedEntry& entry) {
if (!entry.refp || entry.cloneCellPath.empty()) return;
if (entry.cellPath != cellName) return;
// Identity check: only retarget REFDTYPEs that actually live
// inside parentModp. Multiple clones share origName so name
// matching alone would let one clone overwrite another's refs.
if (V3LinkDotIfaceCapture::findOwnerModule(entry.refp) != parentModp) return;
if (retargetRefToModule(entry, correctModp)) {
UINFO(9, "retargetIfaceRefs: " << entry.refp << " -> "
<< correctModp->prettyNameQ() << endl);
}
});
}
AstNodeModule* nodeDeparam(AstNode* nodep, AstNodeModule* srcModp, AstNodeModule* modp,
const string& someInstanceName) {
// Return new or reused de-parameterized module
@ -1895,7 +1921,11 @@ public:
// STATE - across all visitors
std::vector<AstClass*> m_paramClasses; // Parameterized classes
std::vector<AstDot*> m_dots; // Dot references to process
std::multimap<int, AstNodeModule*> m_workQueueNext; // Modules left to process
// Work queue keyed by (!isIface, level) so interfaces are always processed
// before non-interfaces. This ensures interface clones have their types
// properly widthed before any module that references those types.
using WQKey = std::pair<bool, int>;
std::multimap<WQKey, AstNodeModule*> m_workQueueNext; // Modules left to process
// Map from AstNodeModule to set of all AstNodeModules that instantiates it.
std::unordered_map<AstNodeModule*, std::unordered_set<AstNodeModule*>> m_parentps;
};
@ -1930,7 +1960,7 @@ class ParamVisitor final : public VNVisitor {
void processWorkQ() {
UASSERT(!m_iterateModule, "Should not nest");
std::multimap<int, AstNodeModule*> workQueue;
std::multimap<ParamState::WQKey, AstNodeModule*> workQueue;
m_generateHierName = "";
m_iterateModule = true;
@ -2005,10 +2035,21 @@ class ParamVisitor final : public VNVisitor {
if (VN_IS(srcModp, Iface)) {
logTemplateLeakRefs(modp, srcModp, "after queued nodeDeparam", cellp);
// After the interface cell is rewired to its clone,
// retarget REFDTYPEs in the parent module that still
// reference the template interface's types.
if (V3LinkDotIfaceCapture::enabled()) {
if (const AstCell* const modCellp = VN_CAST(cellp, Cell)) {
if (newModp != srcModp) {
m_processor.retargetIfaceRefs(modp, modCellp->name());
}
}
}
}
// Add the (now potentially specialized) child module to the work queue
workQueue.emplace(newModp->level(), newModp);
workQueue.emplace(ParamState::WQKey{!VN_IS(newModp, Iface), newModp->level()},
newModp);
// Add to the hierarchy registry
m_state.m_parentps[newModp].insert(modp);
@ -2194,6 +2235,14 @@ class ParamVisitor final : public VNVisitor {
// destructively widthing the template with default (zero)
// values. See t_interface_nested_struct_param.v.
m_processor.nodeDeparam(cellp, srcModp, m_modp, m_modp->someInstanceName());
// After the interface cell is rewired to its clone,
// retarget REFDTYPEs in the parent module that still
// reference the template interface's types. This ensures
// $bits(iface_typedef) evaluates correctly when
// widthParamsEdit runs on subsequent lparams.
if (V3LinkDotIfaceCapture::enabled() && cellp->modp() != srcModp) {
m_processor.retargetIfaceRefs(m_modp, cellp->name());
}
}
}
@ -2225,7 +2274,8 @@ class ParamVisitor final : public VNVisitor {
UINFO(4, " MOD-under-MOD. " << nodep);
// Delay until current module is done.
// processWorkQ() (which we are returning to) will process nodep later
m_state.m_workQueueNext.emplace(nodep->level(), nodep);
m_state.m_workQueueNext.emplace(
ParamState::WQKey{!VN_IS(nodep, Iface), nodep->level()}, nodep);
return;
}
@ -2233,7 +2283,8 @@ class ParamVisitor final : public VNVisitor {
if (nodep->isTop() // Tops
|| VN_IS(nodep, Class) // Moved classes
|| VN_IS(nodep, Package)) { // Likewise haven't done wrapTopPackages yet
m_state.m_workQueueNext.emplace(nodep->level(), nodep);
m_state.m_workQueueNext.emplace(
ParamState::WQKey{!VN_IS(nodep, Iface), nodep->level()}, nodep);
processWorkQ();
}
}

18
test_regress/t/t_iface_nested_width2.py Executable file
View File

@ -0,0 +1,18 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Localparam with package function call using interface param
#
# 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()

242
test_regress/t/t_iface_nested_width2.v Normal file
View File

@ -0,0 +1,242 @@
// DESCRIPTION: Verilator: Localparam with package function call using interface param
//
// 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
// verilog_format: off
`define stop $stop
`define checkd(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0d exp=%0d (%s !== %s)\n", `__FILE__,`__LINE__, (gotv), (expv), `"gotv`", `"expv`"); `stop; end while(0);
// verilog_format: on
package utils;
function automatic int f_bits (int val);
return (val <= 1) ? 1 : $clog2(val);
endfunction
endpackage
package soc;
// Configuration structure
typedef struct packed {
// CB Configuration
int unsigned CBRids;
int unsigned CBPids;
int unsigned CBFnum;
// Coarse parameters
int unsigned Tile;
int unsigned NumBb;
// CCA Parameters
int unsigned NumDd;
// CC Parameters
int unsigned DDNumW;
int unsigned DDNumWT;
int unsigned DDIds; // check this one
} cfg_t;
endpackage
package cb;
typedef struct packed {
int unsigned Rids; // ring id assigned to port
int unsigned Pids; // packet id
int unsigned Fnum; // flit number
int unsigned XdatSize; // raw packet data size
} cfg_t;
endpackage
package bba;
typedef logic [11:0] csr_index_t;
typedef logic [1:0] cmd_bnum_t;
typedef logic [3:0] rq_cmd_t;
typedef logic [3:0] rs_cmd_t;
typedef union packed {
rq_cmd_t rq;
rs_cmd_t rs;
} cmd_t;
endpackage: bba
interface bb_types_if #(parameter soc::cfg_t cfg=0)();
//
typedef logic [$clog2(cfg.NumDd)-1:0] bb_index_t;
typedef logic [$clog2(cfg.DDIds)-1:0] trans_id_t;
typedef struct packed {
logic hdr_vld;
logic [1:0] bnum;
logic is_rq;
bba::cmd_t cmd;
bb_index_t cc_index;
trans_id_t trans_id;
} cmd_meta_t;
typedef union packed {
logic [cfg.DDNumWT*64-1:0] raw;
logic [cfg.DDNumWT-1:0][63:0] raw_a;
} cmd_data_t;
typedef struct packed {
cmd_meta_t meta;
cmd_data_t d;
} cmd_beat_t;
endinterface
interface cb_port_vc_if #(parameter cb::cfg_t cfg=0)();
typedef logic [$clog2(cfg.Rids)-1:0] rid_t;
typedef logic [$clog2(cfg.Pids)-1:0] pid_t;
typedef logic [$clog2(cfg.Fnum)-1:0] fnum_t;
typedef logic [cfg.XdatSize-1:0] xdat_t;
typedef struct packed {
xdat_t dat;
fnum_t fnum;
logic first;
logic last;
rid_t dest_rid;
} tx_beat_t;
tx_beat_t tx_beat;
endinterface
module simple_mem #(
parameter int p_num_buffered_rq = 16,
parameter int p_storage_size = 65536
)(
bb_types_if bb_types,
// CB Interface
cb_port_vc_if cb_vc0_io,
cb_port_vc_if cb_vc1_io
);
typedef bb_types.cmd_beat_t cmd_beat_t;
logic flit_ot_vld_d, flit_ot_vld_q;
cmd_beat_t flit_ot_d, flit_ot_q;
always_comb begin
cb_vc1_io.tx_beat = 'b0;
if(flit_ot_vld_q) begin
cb_vc1_io.tx_beat.dat = flit_ot_q;
end
end
endmodule
module xm_top #(
parameter soc::cfg_t soc_cfg=0)(
// Interface to SMM from CB
cb_port_vc_if cb_vc0_io,
cb_port_vc_if cb_vc1_io
);
bb_types_if #(soc_cfg) bb_types();
simple_mem #() simple_mem(
.bb_types(bb_types),
.cb_vc0_io(cb_vc0_io),
.cb_vc1_io(cb_vc1_io)
);
endmodule
module tile_top #(
parameter soc::cfg_t soc_cfg=0
)(
);
bb_types_if #(soc_cfg) bb_types();
typedef bb_types.cmd_beat_t cmd_beat_t;
localparam cb::cfg_t cb_cfg = '{
Rids : soc_cfg.CBRids,
Pids : soc_cfg.CBPids,
Fnum : soc_cfg.CBFnum,
XdatSize:$bits(cmd_beat_t)
};
cb_port_vc_if#(cb_cfg) cb_tm_vc0_io();
cb_port_vc_if#(cb_cfg) cb_tm_vc1_io();
xm_top #(
.soc_cfg(soc_cfg)
) xm_top(
.cb_vc0_io(cb_tm_vc0_io),
.cb_vc1_io(cb_tm_vc1_io)
);
endmodule
module soc_top #(parameter soc::cfg_t soc_cfg=0)(
);
genvar a;
localparam int p_num_tile = soc_cfg.Tile;
bb_types_if #(soc_cfg) bb_types();
typedef bb_types.cmd_beat_t cmd_beat_t;
// CB and RBUS
localparam cb::cfg_t cb_cfg = '{
Rids : soc_cfg.CBRids,
Pids : soc_cfg.CBPids,
Fnum : soc_cfg.CBFnum,
XdatSize:$bits(cmd_beat_t)
};
// cb interfaces (HTM dead ends)
cb_port_vc_if#(cb_cfg) cb_vc0_io();
cb_port_vc_if#(cb_cfg) cb_vc1_io();
// HTM
xm_top #(
.soc_cfg(soc_cfg)
) xm_top(
.cb_vc0_io(cb_vc0_io),
.cb_vc1_io(cb_vc1_io)
);
generate
for(a=0; a<p_num_tile; a++) begin : gen_tiles
tile_top #(
.soc_cfg(soc_cfg)
) tile(
);
end
endgenerate
endmodule
module t();
parameter soc::cfg_t soc_cfg = '{
// Cluster Bus
CBRids : 16,
CBPids : 32,
CBFnum : 4,
// Coarse parameters
Tile : 4,
NumBb : 1,
// CCA Parameters
NumDd : 3,
// CC Parameters
DDNumW : 4,
DDNumWT : 8,
DDIds : 5
};
soc_top #(
.soc_cfg(soc_cfg)
) soc_top();
// Check that cb_cfg.XdatSize is correctly computed as $bits(cmd_beat_t) = 525
// for both soc_top and tile_top paths
initial begin
`checkd(soc_top.cb_cfg.XdatSize, 525);
`checkd(soc_top.gen_tiles[0].tile.cb_cfg.XdatSize, 525);
`checkd(soc_top.gen_tiles[1].tile.cb_cfg.XdatSize, 525);
`checkd(soc_top.gen_tiles[2].tile.cb_cfg.XdatSize, 525);
`checkd(soc_top.gen_tiles[3].tile.cb_cfg.XdatSize, 525);
$display("PASS: All cb_cfg.XdatSize = 525");
end
endmodule

4
test_regress/t/t_paramgraph_iface_template_nested_stats.py
View File

@ -21,9 +21,9 @@ test.compile(v_flags2=["--binary --stats"])
test.file_grep(test.stats, r'IfaceCapture, Entries total\s+(\d+)', 21)
test.file_grep(test.stats, r'IfaceCapture, Entries template\s+(\d+)', 11)
test.file_grep(test.stats, r'IfaceCapture, Entries cloned\s+(\d+)', 10)
test.file_grep(test.stats, r'IfaceCapture, Ledger fixups in V3Param\s+(\d+)', 8)
test.file_grep(test.stats, r'IfaceCapture, Ledger fixups in V3Param\s+(\d+)', 7)
test.file_grep(test.stats, r'IfaceCapture, Wrong-clone refs fixed\s+(\d+)', 8)
test.file_grep(test.stats, r'IfaceCapture, Dead refs fixed in modules\s+(\d+)', 3)
test.file_grep(test.stats, r'IfaceCapture, Dead refs fixed in modules\s+(\d+)', 4)
test.execute()