diff --git a/src/V3Param.cpp b/src/V3Param.cpp index 23124a371..56f664b2f 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -2303,11 +2303,120 @@ class ParamClassRefDTypeRelinkVisitor final : public VNVisitor { AstClass* m_classp = nullptr; std::unordered_map m_paramTypeClassMap; + // Owner-tracking state for the post-clone REFDTYPE typedef retargeting + // (see retargetRefDType()). m_ownerModp is the most recently entered + // AstNodeModule (or AstClass). m_origNameToClone / m_ambiguous are + // rebuilt lazily on the first AstRefDType visit per owner; the + // m_ownerMapForp cache key avoids rebuilding when the owner is unchanged + // (e.g. across siblings under the same parent). + AstNodeModule* m_ownerModp = nullptr; + AstNodeModule* m_ownerMapForp = nullptr; + std::unordered_map m_origNameToClone; + std::unordered_set m_ambiguous; + + void ensureOwnerMap() { + if (m_ownerMapForp == m_ownerModp) return; + m_ownerMapForp = m_ownerModp; + m_origNameToClone.clear(); + m_ambiguous.clear(); + if (!m_ownerModp) return; + // Skip bare/generic class templates: their bodies are not + // authoritative for any specialization, and their self-typedefs + // (e.g. `typedef cls#(P,Q) this_type;` inside a parameterized class) + // would otherwise be treated as ground truth and cause REFDTYPEs in + // the body to be retargeted at the bare template. + if (AstClass* const ownerClassp = VN_CAST(m_ownerModp, Class)) { + if (ownerClassp->hasGParam()) return; + } + for (AstNode* sp = m_ownerModp->stmtsp(); sp; sp = sp->nextp()) { + AstTypedef* const tdp = VN_CAST(sp, Typedef); + if (!tdp) continue; + AstClassRefDType* const crdtp = VN_CAST(tdp->subDTypep(), ClassRefDType); + if (!crdtp || !crdtp->classp()) continue; + AstClass* const targetp = crdtp->classp(); + // A typedef pointing at a still-generic template is a forward + // declaration / unresolved reference, not ground truth. + if (targetp->hasGParam()) continue; + const std::string origName + = targetp->origName().empty() ? targetp->name() : targetp->origName(); + const auto pair = m_origNameToClone.emplace(origName, targetp); + // Two locally-resolved typedefs pointing at different + // specializations of the same template -> ambiguous; we cannot + // safely guess which one a stripped REFDTYPE meant. + if (!pair.second && pair.first->second != targetp) m_ambiguous.insert(origName); + } + } + + // Re-resolve REFDTYPE.typedefp/refDTypep using the containing module's + // own resolved typedef chain. The eager retargeting in deepCloneModule + // blindly retargets every captured entry to whichever sibling clone is + // being created, so when a referenced parameterized class is cloned more + // than once (e.g. driver -> driver__I5 + driver__I6), the second sibling + // stomps the bindings made by the first. By this point each owner's + // typedefs have been correctly bound (via classRefDeparam), so we use + // those local typedefs as ground truth. + void retargetRefDType(AstRefDType* refp) { + if (!m_ownerModp) return; + AstTypedef* const oldTdp = refp->typedefp(); + if (!oldTdp) return; + AstClass* const oldOwnerp = VN_CAST(V3LinkDotIfaceCapture::findOwnerModule(oldTdp), Class); + if (!oldOwnerp) return; + ensureOwnerMap(); + if (m_origNameToClone.empty()) return; + const std::string origName + = oldOwnerp->origName().empty() ? oldOwnerp->name() : oldOwnerp->origName(); + if (m_ambiguous.count(origName)) { + // The owner has two locally-resolved typedefs pointing at + // different specializations of the same template, e.g.: + // typedef driver #(.IW(5)) drv5_t; + // typedef driver #(.IW(6)) drv6_t; + // A stripped REFDTYPE only carries the origName, so we cannot + // tell which sibling it meant; leave it alone. + UINFO(9, "post-param REFDTYPE retarget skipped (ambiguous owner '" << origName + << "'): " << refp); + return; + } + const auto it = m_origNameToClone.find(origName); + if (it == m_origNameToClone.end()) return; + AstClass* const correctClassp = it->second; + if (correctClassp == oldOwnerp) return; + AstTypedef* const newTdp + = V3LinkDotIfaceCapture::findTypedefInModule(correctClassp, refp->name()); + if (!newTdp) { + // The owner map says correctClassp is the right specialization, + // but it does not declare a typedef named refp->name(). Caveats + // of findTypedefInModule (e.g. typedef declared in a base class, + // not the specialization we resolved to) can land us here. + UINFO(9, "post-param REFDTYPE retarget skipped (no typedef '" + << refp->name() << "' in " << correctClassp->name() << "): " << refp); + return; + } + // typedefp / classOrPackagep / refDTypep must agree. If newTdp lacks + // a subDTypep we cannot keep all three in sync, so skip the entire + // retarget rather than leaving the REFDTYPE half-updated. + if (!newTdp->subDTypep()) { + UINFO(9, "post-param REFDTYPE retarget skipped (newTdp has null subDTypep): " << refp); + return; + } + refp->typedefp(newTdp); + refp->classOrPackagep(correctClassp); + refp->refDTypep(newTdp->subDTypep()); + UINFO(9, "post-param REFDTYPE re-resolve: " << refp << " from " << oldOwnerp->name() + << " to " << correctClassp->name()); + } + public: explicit ParamClassRefDTypeRelinkVisitor(AstNetlist* netlistp) { iterate(netlistp); } + void visit(AstNodeModule* nodep) override { + VL_RESTORER(m_ownerModp); + m_ownerModp = nodep; + iterateChildren(nodep); + } void visit(AstClass* nodep) override { VL_RESTORER(m_classp); + VL_RESTORER(m_ownerModp); m_classp = nodep; + m_ownerModp = nodep; for (AstNode* stmtp = nodep->stmtsp(); stmtp; stmtp = stmtp->nextp()) { if (AstParamTypeDType* const ptp = VN_CAST(stmtp, ParamTypeDType)) { m_paramTypeClassMap[ptp] = nodep; @@ -2317,29 +2426,38 @@ public: } void visit(AstRefDType* nodep) override { iterateChildren(nodep); - AstParamTypeDType* const paramtypep = VN_CAST(nodep->refDTypep(), ParamTypeDType); - if (!paramtypep) return; - const auto it = m_paramTypeClassMap.find(paramtypep); - if (it == m_paramTypeClassMap.end()) return; - AstClass* const origClassp = it->second; - if (!origClassp->hasGParam()) return; // only relink refs to original param classes - if (origClassp->user3p()) return; // will not get removed, no need to relink - AstClass* const parametrizedClassp = VN_CAST(origClassp->user4p(), Class); - if (!parametrizedClassp) return; - const string paramName = paramtypep->name(); - AstParamTypeDType* newParamTypep = nullptr; - for (AstNode* stmtp = parametrizedClassp->stmtsp(); stmtp; stmtp = stmtp->nextp()) { - if (AstParamTypeDType* const ptp = VN_CAST(stmtp, ParamTypeDType)) { - if (ptp->name() == paramName) { - newParamTypep = ptp; - break; + // Redirect refs whose refDTypep points at a stale ParamTypeDType in + // a generic class template that has been replaced by a parametrized + // clone. + if (AstParamTypeDType* const paramtypep = VN_CAST(nodep->refDTypep(), ParamTypeDType)) { + const auto it = m_paramTypeClassMap.find(paramtypep); + if (it != m_paramTypeClassMap.end()) { + AstClass* const origClassp = it->second; + if (origClassp->hasGParam() && !origClassp->user3p()) { + if (AstClass* const parametrizedClassp + = VN_CAST(origClassp->user4p(), Class)) { + const string paramName = paramtypep->name(); + AstParamTypeDType* newParamTypep = nullptr; + for (AstNode* stmtp = parametrizedClassp->stmtsp(); stmtp; + stmtp = stmtp->nextp()) { + if (AstParamTypeDType* const ptp = VN_CAST(stmtp, ParamTypeDType)) { + if (ptp->name() == paramName) { + newParamTypep = ptp; + break; + } + } + } + if (newParamTypep) { + nodep->refDTypep(newParamTypep); + nodep->classOrPackagep(parametrizedClassp); + } + } } } } - if (newParamTypep) { - nodep->refDTypep(newParamTypep); - nodep->classOrPackagep(parametrizedClassp); - } + // Retarget REFDTYPE.typedefp using the current owner's own + // resolved typedefs as ground truth (handles sibling-clone stomping). + retargetRefDType(nodep); } void visit(AstNode* nodep) override { iterateChildren(nodep); } }; diff --git a/test_regress/t/t_class_param_module_typedef.py b/test_regress/t/t_class_param_module_typedef.py new file mode 100755 index 000000000..c87a1e49f --- /dev/null +++ b/test_regress/t/t_class_param_module_typedef.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('vlt') + +test.compile(verilator_flags2=["--binary"]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_class_param_module_typedef.v b/test_regress/t/t_class_param_module_typedef.v new file mode 100644 index 000000000..e4632d09a --- /dev/null +++ b/test_regress/t/t_class_param_module_typedef.v @@ -0,0 +1,39 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// Regression test for a name-resolution bug: +// A parameterized module that re-typedefs a typedef out of a parameterized +// class (`typedef cls#(...)::name name;`) used to spuriously fail with +// "Reference to '' before declaration (IEEE 1800-2023 6.18)" when +// the parameterized class had 2+ parameters and the enclosing module had +// any module parameter. Reduced from pulp-platform/axi +// tb_axi_iw_converter.sv:182. +// +// 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 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 pkg; + class rand_master #(parameter int AW = 32, parameter int DW = 32); + typedef logic [AW-1:0] addr_t; + endclass +endpackage + +module t #(parameter int unused = 1); + // AW and DW are both overridden to non-default values so that a regression + // in parameter propagation through the typedef chain shows up as a width + // mismatch instead of silently succeeding. + typedef pkg::rand_master #(.AW(17), .DW(64)) rand_master_t; + typedef rand_master_t::addr_t addr_t; + initial begin + static addr_t a = '0; + `checkd($bits(addr_t), 17); + `checkd($bits(a), 17); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_class_param_nested_typedef.py b/test_regress/t/t_class_param_nested_typedef.py new file mode 100755 index 000000000..c87a1e49f --- /dev/null +++ b/test_regress/t/t_class_param_nested_typedef.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('vlt') + +test.compile(verilator_flags2=["--binary"]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_class_param_nested_typedef.v b/test_regress/t/t_class_param_nested_typedef.v new file mode 100644 index 000000000..a69e7366c --- /dev/null +++ b/test_regress/t/t_class_param_nested_typedef.v @@ -0,0 +1,72 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// Regression test for a parameterized-class typedef-chain bug. +// +// A parameterized class M re-typedefs a nested type from another +// parameterized class D (M::beat_t = M::driver_t::beat_t). When M and D +// are each instantiated with two distinct parameter values, the type of +// an M::beat_t local must resolve to the matching specialization of beat +// for the enclosing M instantiation. Previously the second sibling clone +// stomped the typedefp binding on the first sibling's REFDTYPE, producing +// a spurious "Function Argument expects ... beat__I5, got ... beat__I6" +// error. +// +// Reduced from axi_test::axi_rand_master usage in pulp-platform/axi +// (tb_axi_xbar with NumMasters>1 or NumSlaves>1). +// +// 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 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 pkg; + + class beat #(parameter int IW = 8); + logic [IW-1:0] id; + endclass + + class driver #(parameter int IW = 8); + typedef beat #(.IW(IW)) beat_t; + // Verify the beat handed to us has the IW we were specialized for. + task send(input beat_t b); + `checkd($bits(b.id), IW); + `checkd(b.id, IW'(IW)); + endtask + endclass + + class master #(parameter int IW = 8); + typedef driver #(.IW(IW)) driver_t; + typedef driver_t::beat_t beat_t; + + driver_t drv; + function new(driver_t d); drv = d; endfunction + + task run(); + automatic beat_t b = new; + // The width of beat_t.id must follow this master's IW. If the + // typedefp on the M::beat_t REFDTYPE is stomped by a sibling clone, + // $bits here will not match IW (or compilation will fail outright). + `checkd($bits(b.id), IW); + b.id = IW'(IW); + drv.send(b); + endtask + endclass + +endpackage + +module t; + pkg::master #(.IW(5)) ma; + pkg::master #(.IW(6)) mb; + pkg::driver #(.IW(5)) da; + pkg::driver #(.IW(6)) db; + initial begin + da = new; ma = new(da); ma.run(); + db = new; mb = new(db); mb.run(); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule