Fix typedef through a parameterized sub-interface (#7943 repair) (#8124) (#8128)

This commit is contained in:
em2machine
2026-09-23 14:32:23 -04:00
committed by GitHub
parent e6ecf3a014
commit f82cdbeaa7
8 changed files with 184 additions and 56 deletions
+6
View File
@@ -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<const AstConst*, string> m_constOrigParamNames;
// Module each node is in, only good while the tree holds still
std::unordered_map<const AstNode*, const AstNodeModule*> m_containingModules;
// The model's evaluation entry point functions
std::array<AstCFunc*, VEval::_ENUM_END> 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]; }
+11
View File
@@ -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();
+68 -56
View File
@@ -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);
}
+6
View File
@@ -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,
+4
View File
@@ -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");
+13
View File
@@ -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: {
+18
View File
@@ -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()
@@ -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