diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index ee58f63d5..c37796284 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1578,6 +1578,8 @@ class AstNetlist final : public AstNode { // AstConst itself, as AstConst is a very common node and only a small fraction carry this // name. std::unordered_map m_constOrigParamNames; + // Module each node is in, only good while the tree holds still + std::unordered_map m_containingModules; // The model's evaluation entry point functions std::array m_evalFuncps{}; // The trigger dump function of each region if exists, otherwise nullptr @@ -1603,6 +1605,10 @@ public: string astConstOrigParamName(const AstConst* nodep) const; void astConstOrigParamName(const AstConst* nodep, const string& name); void astConstOrigParamNameErase(const AstConst* nodep); + // Find the module a node is in, remembering what it passed on the way. + const AstNodeModule* containingModule(const AstNode* nodep); + // Forget remembered modules, as the tree has moved. + void clearContainingModules() { m_containingModules.clear(); } AstPackage* dollarUnitPkgp() const { return m_dollarUnitPkgp; } void dollarUnitPkgp(AstPackage* const packagep) { m_dollarUnitPkgp = packagep; } AstCFunc* evalFuncp(VEval eval) const { return m_evalFuncps[eval]; } diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 7d5400ef0..da6c698b1 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -1963,6 +1963,16 @@ const char* AstNetlist::broken() const { } return nullptr; } +const AstNodeModule* AstNetlist::containingModule(const AstNode* nodep) { + if (const AstNodeModule* const modp = VN_CAST(nodep, NodeModule)) return modp; + const auto it = m_containingModules.find(nodep); + if (it != m_containingModules.end()) return it->second; + // Only true parents are followed. + AstNode* const abovep = nodep->aboveLoopp(); + const AstNodeModule* const modp = abovep ? containingModule(abovep) : nullptr; + m_containingModules[nodep] = modp; + return modp; +} void AstNetlist::createTopScope(AstScope* scopep) { UASSERT(scopep, "Must not be nullptr"); UASSERT_OBJ(!m_topScopep, scopep, "TopScope already exits"); @@ -1980,6 +1990,7 @@ void AstNetlist::deleteContents() { m_nbaEventp = nullptr; m_nbaEventTriggerp = nullptr; m_topScopep = nullptr; + m_containingModules.clear(); m_evalFuncps.fill(nullptr); m_dumpTriggersFuncps.fill(nullptr); if (op1p()) op1p()->unlinkFrBackWithNext()->deleteTree(); diff --git a/src/V3LinkDotIfaceCapture.cpp b/src/V3LinkDotIfaceCapture.cpp index 7c230108e..686b68bbb 100644 --- a/src/V3LinkDotIfaceCapture.cpp +++ b/src/V3LinkDotIfaceCapture.cpp @@ -190,34 +190,50 @@ bool V3LinkDotIfaceCapture::retargetRefToModule(const CapturedEntry& entry, AstParamTypeDType* const paramTypep = findParamTypeInModule(targetModp, entry.refp->name()); if (!paramTypep) return false; - const auto retarget = [&](AstRefDType* refp) { - if (!refp) return; - refp->refDTypep(paramTypep); - refp->dtypep(paramTypep); - }; - retarget(entry.refp); - for (AstRefDType* const refp : entry.extraRefps) retarget(refp); + retargetRefToParamType(entry.refp, paramTypep); + for (AstRefDType* const refp : entry.extraRefps) retargetRefToParamType(refp, paramTypep); return true; } AstTypedef* const typedefp = findTypedefInModule(targetModp, entry.refp->name()); if (!typedefp) return false; - AstNodeDType* const dtypep = typedefp->subDTypep(); - const auto retarget = [&](AstRefDType* refp) { - if (!refp) return; - refp->typedefp(typedefp); - // An incomplete typedef is still a successful name resolution, but - // must not erase type links that a later width pass can complete. - if (dtypep) { - refp->refDTypep(dtypep); - refp->dtypep(dtypep); - } - }; - retarget(entry.refp); - for (AstRefDType* const refp : entry.extraRefps) retarget(refp); + retargetRefToTypedef(entry.refp, typedefp); + for (AstRefDType* const refp : entry.extraRefps) retargetRefToTypedef(refp, typedefp); return true; } +void V3LinkDotIfaceCapture::retargetRefToParamType(AstRefDType* refp, + AstParamTypeDType* paramTypep) { + // An entry can hold references that were dropped when their subtree died. + if (!refp) return; + UASSERT_OBJ(paramTypep, refp, "Retarget to a null parameter type"); + refp->refDTypep(paramTypep); + refp->dtypep(paramTypep); +} + +void V3LinkDotIfaceCapture::retargetRefToTypedef(AstRefDType* refp, AstTypedef* typedefp) { + // An entry can hold references that were dropped when their subtree died. + if (!refp) return; + UASSERT_OBJ(typedefp, refp, "Retarget to a null typedef"); + refp->typedefp(typedefp); + // An incomplete typedef is still a successful name resolution, but + // must not erase type links that a later width pass can complete. + if (AstNodeDType* const dtypep = typedefp->subDTypep()) { + refp->refDTypep(dtypep); + refp->dtypep(dtypep); + } +} + +bool V3LinkDotIfaceCapture::isCloneOfModule(const AstNodeModule* modp, + const AstNodeModule* templateModp) { + if (!modp || !templateModp || modp == templateModp) return false; + // Only a module that was actually copied has copies, which keeps a copy from + // matching another copy of the same original. + if (!templateModp->parameterizedTemplate()) return false; + // A copy keeps the name the module was written with, whatever it was renamed to. + return modp->origName() == templateModp->origName(); +} + AstNodeModule* V3LinkDotIfaceCapture::findCloneViaHierarchy(AstNodeModule* containingModp, AstNodeModule* deadTargetModp, int depth) { @@ -226,12 +242,7 @@ AstNodeModule* V3LinkDotIfaceCapture::findCloneViaHierarchy(AstNodeModule* conta if (AstCell* const cellp = VN_CAST(stmtp, Cell)) { AstNodeModule* const cellModp = cellp->modp(); if (!cellModp || cellModp->dead()) continue; - // Check if cellModp is a clone of deadTargetModp by comparing - // the template name (part before "__") - const string& cellModName = cellModp->name(); - const string& deadName = deadTargetModp->name(); - const size_t pos = cellModName.find("__"); - if (pos != string::npos && cellModName.substr(0, pos) == deadName) { return cellModp; } + if (isCloneOfModule(cellModp, deadTargetModp)) return cellModp; // Recurse into sub-cells AstNodeModule* const found = findCloneViaHierarchy(cellModp, deadTargetModp, depth + 1); @@ -267,6 +278,13 @@ AstNodeModule* findOwnerModuleIfLive(AstNode* nodep, const LiveNodes& liveNodes) return findOwnerModuleImpl(nodep, &liveNodes); } +// Shared by the callers that all asked this the same way. +AstNodeModule* dyingOwnerOf(AstNode* nodep, const LiveNodes& liveNodes) { + if (!nodep || !liveNodes.count(nodep)) return nullptr; + AstNodeModule* const ownerp = findOwnerModuleIfLive(nodep, liveNodes); + return (ownerp && ownerp->dead()) ? ownerp : nullptr; +} + bool moduleMatchesOwner(const AstNodeModule* modp, const string& ownerName) { if (!modp || ownerName.empty()) return false; return modp->name() == ownerName || modp->origName() == ownerName; @@ -277,31 +295,33 @@ int V3LinkDotIfaceCapture::fixDeadRefs(AstRefDType* refp, AstNodeModule* contain const char* location, const LiveNodes& liveNodes) { int fixed = 0; - // Fix typedefp pointing to dead module + // Check both links, a reference may only have one of them. AstTypedef* const oldTypedefp = refp->typedefp(); - if (oldTypedefp && liveNodes.count(oldTypedefp)) { - AstNodeModule* const typedefModp = findOwnerModuleIfLive(oldTypedefp, liveNodes); - if (typedefModp && typedefModp->dead()) { - AstNodeModule* cloneModp = nullptr; - if (containingModp) { cloneModp = findCloneViaHierarchy(containingModp, typedefModp); } - if (cloneModp) { - const string& tdName = oldTypedefp->name(); - if (AstTypedef* const newTdp = findTypedefInModule(cloneModp, tdName)) { - UINFO(9, "iface capture finalizeCapture (" - << location << "): fixing typedefp refp=" << refp << " dead=" - << typedefModp->name() << " -> " << cloneModp->name()); - refp->typedefp(newTdp); - ++fixed; - } + AstNodeModule* deadModp = dyingOwnerOf(oldTypedefp, liveNodes); + if (!deadModp) deadModp = dyingOwnerOf(refp->refDTypep(), liveNodes); + + if (deadModp && containingModp) { + // The module we are in can be the copy we want. + AstNodeModule* const cloneModp = isCloneOfModule(containingModp, deadModp) + ? containingModp + : findCloneViaHierarchy(containingModp, deadModp); + if (cloneModp) { + // A reference without a typedef still has its own name. + const string& tdName = oldTypedefp ? oldTypedefp->name() : refp->name(); + // Use the same retargets as the ledger so nothing is left behind. + if (AstTypedef* const newTdp = findTypedefInModule(cloneModp, tdName)) { + UINFO(9, "iface capture finalizeCapture (" << location << "): fixing refp=" << refp + << " dead=" << deadModp->name() + << " -> " << cloneModp->name()); + retargetRefToTypedef(refp, newTdp); + ++fixed; } } } - // refDTypep is retargeted for captured refs by resolveCapturedRefs, and no - // non-captured ref resolves refDTypep into a template that then dies, so it - // never survives pointing at a dead module here (verifyNoDeadRefs re-checks). + // Only worth checking when there is no typedef, as that is read first. AstNodeDType* const oldRefDTypep = refp->refDTypep(); - if (oldRefDTypep && liveNodes.count(oldRefDTypep)) { + if (!refp->typedefp() && oldRefDTypep && liveNodes.count(oldRefDTypep)) { AstNodeModule* const targetModp = findOwnerModuleIfLive(oldRefDTypep, liveNodes); UASSERT_OBJ(!targetModp || !targetModp->dead(), refp, "refDTypep of '" << refp->prettyNameQ() << "' points to dead module '" @@ -832,17 +852,9 @@ class TypeTableDeadRefVisitor final : public VNVisitor { // For type table entries, find the first live module that contains // a cell hierarchy leading to the dead target AstNodeModule* containingModp = nullptr; - AstNodeModule* deadTargetModp = nullptr; - // Check BOTH typedefp and refDTypep for dead owners. - // Either (or both) may point to a dead module. - if (refp->typedefp() && m_liveNodes.count(refp->typedefp())) { - AstNodeModule* const tdOwnerp = findOwnerModuleIfLive(refp->typedefp(), m_liveNodes); - if (tdOwnerp && tdOwnerp->dead()) deadTargetModp = tdOwnerp; - } - if (!deadTargetModp && refp->refDTypep() && m_liveNodes.count(refp->refDTypep())) { - AstNodeModule* const rdOwnerp = findOwnerModuleIfLive(refp->refDTypep(), m_liveNodes); - if (rdOwnerp && rdOwnerp->dead()) deadTargetModp = rdOwnerp; - } + // Either (or both) links may point to a dead module. + AstNodeModule* deadTargetModp = dyingOwnerOf(refp->typedefp(), m_liveNodes); + if (!deadTargetModp) deadTargetModp = dyingOwnerOf(refp->refDTypep(), m_liveNodes); if (deadTargetModp) { V3LinkDotIfaceCapture::findLiveCloneOf(deadTargetModp, &containingModp); } diff --git a/src/V3LinkDotIfaceCapture.h b/src/V3LinkDotIfaceCapture.h index 8abb54e93..ea10099aa 100644 --- a/src/V3LinkDotIfaceCapture.h +++ b/src/V3LinkDotIfaceCapture.h @@ -135,6 +135,12 @@ private: static void clearModuleCache(); static AstIfaceRefDType* ifaceRefFromVarDType(AstNodeDType* dtypep); static string extractIfacePortName(const string& dotText); + // True if this module is a copy of that one. + static bool isCloneOfModule(const AstNodeModule* modp, const AstNodeModule* templateModp); + // Point a reference at a typedef and fix its other links. + static void retargetRefToTypedef(AstRefDType* refp, AstTypedef* typedefp); + // Same, for a parameter type. + static void retargetRefToParamType(AstRefDType* refp, AstParamTypeDType* paramTypep); static AstNodeModule* findCloneViaHierarchy(AstNodeModule* containingModp, AstNodeModule* deadTargetModp, int depth = 0); static AstNodeModule* findLiveCloneOf(AstNodeModule* deadTargetModp, diff --git a/src/V3Param.cpp b/src/V3Param.cpp index 8542c9197..2c3929eea 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -2771,6 +2771,8 @@ class ParamVisitor final : public VNVisitor { const auto itm = workQueue.cbegin(); AstNodeModule* const modp = itm->second; workQueue.erase(itm); + // Starting a new module, so what was learned about the last one no longer holds. + v3Global.rootp()->clearContainingModules(); // Process once; note user2 will be cleared on specialization, so we will do the // specialized module if needed @@ -3829,6 +3831,8 @@ void V3Param::param(AstNetlist* rootp) { if (dumpTreeEitherLevel() >= 9) V3LinkDotIfaceCapture::dumpEntries("before V3Param"); { ParamTop{rootp}; } + // The memo is only good while parameterizing, and the tree moves after. + rootp->clearContainingModules(); V3LinkDotIfaceCapture::purgeStaleRefs(); if (dumpTreeEitherLevel() >= 9) V3LinkDotIfaceCapture::dumpEntries("after V3Param"); diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 5a800dd2e..9f5615241 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -2360,6 +2360,19 @@ class WidthVisitor final : public VNVisitor { case VAttrType::DIM_SIZE: { AstNodeDType* const dtypep = fromDTypep(nodep->fromp()); UASSERT_OBJ(dtypep, nodep, "Unsized expression"); + // Only worth asking while parameters are still being worked out. + if (m_paramsOnly) { + // A module that is still being copied does not have its final sizes. + const AstNodeModule* const ownModp = v3Global.rootp()->containingModule(dtypep); + if (ownModp && ownModp->parameterizedTemplate() && !ownModp->dead()) { + UINFO(9, "size deferred, type still on template " << ownModp->name()); + // These queries always give an int, so set that now and let the + // value be worked out once the copy exists. + nodep->dtypeSetInt(); + return; + } + } + if (VN_IS(dtypep, QueueDType) || VN_IS(dtypep, DynArrayDType)) { switch (nodep->attrType()) { case VAttrType::DIM_SIZE: { diff --git a/test_regress/t/t_iface_typedef_param_bits.py b/test_regress/t/t_iface_typedef_param_bits.py new file mode 100755 index 000000000..44d191244 --- /dev/null +++ b/test_regress/t/t_iface_typedef_param_bits.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(v_flags2=["--binary"]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_iface_typedef_param_bits.v b/test_regress/t/t_iface_typedef_param_bits.v new file mode 100644 index 000000000..53452ffbe --- /dev/null +++ b/test_regress/t/t_iface_typedef_param_bits.v @@ -0,0 +1,58 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// Sizes of a type from a parameterized interface must use the specialized +// parameter, not the value the interface was declared with. +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: CC0-1.0 + +package a_pkg; + typedef struct packed { + int unsigned p_a; + } cfg_t; +endpackage + +interface sub_if #(parameter a_pkg::cfg_t cfg = 0); + typedef logic [cfg.p_a-1:0] data_t; + typedef struct packed { + logic [3:0] addr; + data_t data; + } data2_t; +endinterface + +module sub (sub_if io); +endmodule + +module t(); + parameter a_pkg::cfg_t cfg = '{p_a: 16}; + + sub_if #(cfg) sub_io(); + + sub u_sub(.io(sub_io)); + + typedef sub_io.data2_t data2_t; + typedef sub_io.data_t data_t; + + localparam int COUNT = $bits(data2_t); + localparam int DBITS = $bits(data_t); + localparam int DHIGH = $high(data_t); + localparam int DLOW = $low(data_t); + localparam int DLEFT = $left(data_t); + localparam int DRIGHT = $right(data_t); + localparam int DSIZE = $size(data_t); + localparam int DINCR = $increment(data_t); + + initial begin + if (COUNT != 20) $stop; + if (DBITS != 16) $stop; + if (DHIGH != 15) $stop; + if (DLOW != 0) $stop; + if (DLEFT != 15) $stop; + if (DRIGHT != 0) $stop; + if (DSIZE != 16) $stop; + if (DINCR != 1) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule