From a2d4b90b52bace1d43dee986df028375e05c8294 Mon Sep 17 00:00:00 2001 From: em2machine <92717390+em2machine@users.noreply.github.com> Date: Mon, 25 May 2026 17:48:31 -0400 Subject: [PATCH] Fix dearray varref scope error (#7530) (#7602) Fixes #7530. --- src/V3Ast.cpp | 9 ++ src/V3Inst.cpp | 82 ++++++++++++++++++- test_regress/t/t_interface_array_class_new.py | 18 ++++ test_regress/t/t_interface_array_class_new.v | 42 ++++++++++ 4 files changed, 150 insertions(+), 1 deletion(-) create mode 100755 test_regress/t/t_interface_array_class_new.py create mode 100644 test_regress/t/t_interface_array_class_new.v diff --git a/src/V3Ast.cpp b/src/V3Ast.cpp index 423d530d5..9fe36df38 100644 --- a/src/V3Ast.cpp +++ b/src/V3Ast.cpp @@ -1698,6 +1698,15 @@ static VCastable computeCastableImp(const AstNodeDType* toDtp, const AstNodeDTyp if (upcast) return VCastable::COMPATIBLE; if (downcast) return VCastable::DYNAMIC_CLASS; return VCastable::INCOMPATIBLE; + } else if (const AstIfaceRefDType* const toIfp = VN_CAST(toDtp, IfaceRefDType)) { + // Two interface refs are compatible if they point at the same interface + // module (and modport, if any). Pointer-equality on the dtype isn't + // enough since every cell binding clones the dtype. + const AstIfaceRefDType* const fromIfp = VN_CAST(fromDtp, IfaceRefDType); + if (fromIfp && toIfp->ifaceViaCellp() == fromIfp->ifaceViaCellp() + && (!toIfp->modportp() || toIfp->modportp() == fromIfp->modportp())) { + return VCastable::COMPATIBLE; + } } return castable; } diff --git a/src/V3Inst.cpp b/src/V3Inst.cpp index 2d9b443dd..5a0f9279c 100644 --- a/src/V3Inst.cpp +++ b/src/V3Inst.cpp @@ -172,6 +172,64 @@ public: } }; +//###################################################################### +// Replace any leftover VarRef to an interface-array var that the cell +// dearrayer just deleted. The dearrayer handles VarRefs in pin / array-select +// / array-assign contexts itself; this rewriter catches the rest (a class +// new() arg, a function call arg, etc) by rebuilding an array literal over +// the per-element vars. Runs once, only if anything was actually deleted. + +class InstDeOrphanVisitor final : public VNVisitor { + // STATE + const std::unordered_map>& m_dearrayed; + + static bool isDearrayerHandled(const AstNode* backp) { + // VarRefs under these are rewritten by the cell dearrayer itself. + return VN_IS(backp, Pin) || VN_IS(backp, ArraySel) || VN_IS(backp, SliceSel) + || VN_IS(backp, NodeAssign); + } + + // Build a (possibly nested) InitArray matching dtp's array nesting; leaves + // are VarRefs to perElem, consumed in flat row-major order. + static AstNodeExpr* buildPattern(FileLine* flp, AstNodeDType* dtp, + const std::vector& perElem, size_t& idxr, + const VAccess& access) { + AstNodeDType* const skipDtp = dtp->skipRefp(); + if (AstUnpackArrayDType* const arrp = VN_CAST(skipDtp, UnpackArrayDType)) { + AstInitArray* const initp = new AstInitArray{flp, arrp, nullptr}; + const int elems = arrp->elementsConst(); + for (int i = 0; i < elems; ++i) { + initp->addIndexValuep(static_cast(i), + buildPattern(flp, arrp->subDTypep(), perElem, idxr, access)); + } + return initp; + } + UASSERT(idxr < perElem.size(), "buildPattern outran per-element vars"); + return new AstVarRef{flp, perElem[idxr++], access}; + } + + // VISITORS + void visit(AstVarRef* nodep) override { + const auto it = m_dearrayed.find(nodep->varp()); + if (it == m_dearrayed.end()) return; + if (isDearrayerHandled(nodep->backp())) return; + size_t idx = 0; + AstNodeExpr* const newp = buildPattern(nodep->fileline(), it->first->dtypep(), it->second, + idx, nodep->access()); + nodep->replaceWith(newp); + VL_DO_DANGLING(pushDeletep(nodep), nodep); + } + void visit(AstNode* nodep) override { iterateChildren(nodep); } + +public: + InstDeOrphanVisitor(AstNode* rootp, + const std::unordered_map>& dearrayed) + : m_dearrayed{dearrayed} { + iterate(rootp); + } + ~InstDeOrphanVisitor() override = default; +}; + //###################################################################### class InstDeVisitor final : public VNVisitor { @@ -181,6 +239,10 @@ private: const AstRange* m_cellRangep = nullptr; // Outer range; nullptr for non-arrayed cells int m_instSelNum = 0; // Row-major flat index for 1D-compat pin expansion InstDeModVarVisitor m_deModVars; // State of variables for current cell module + // Iface-array vars deleted by visit(AstCell), mapped to their per-element + // replacements in row-major order. The post-pass uses this to fix up any + // leftover VarRefs (see InstDeOrphanVisitor). + std::unordered_map> m_dearrayedIfaceVars; // VISITORS void visit(AstVar* nodep) override { @@ -261,6 +323,10 @@ private: } const bool isIface = origIfaceRefp && !origIfaceRefp->isVirtual(); + // Collected in row-major order for the orphan VarRef rewriter. + std::vector perElemVarps; + if (isIface) perElemVarps.reserve(totalElems); + std::vector idx(ndim, 0); for (int n = 0; n < totalElems; ++n) { // Unflatten n into a row-major cartesian index; outer dim most significant. @@ -290,6 +356,11 @@ private: // Interface instantiation: also clone the IfaceRef in the parent module. if (isIface) { + // Cache the interface module on the dtype so ifaceViaCellp() + // still works after we clear cellp and delete the cell. + if (!origIfaceRefp->ifacep()) { + origIfaceRefp->ifacep(VN_AS(nodep->modp(), Iface)); + } origIfaceRefp->cellp(nullptr); AstVar* const varNewp = ifaceVarp->cloneTree(false); AstIfaceRefDType* const ifaceRefp = origIfaceRefp->cloneTree(false); @@ -300,6 +371,7 @@ private: varNewp->origName(varNewp->origName() + suffix); varNewp->dtypep(ifaceRefp); newp->addNextHere(varNewp); + perElemVarps.emplace_back(varNewp); if (debug() == 9) { varNewp->dumpTree("- newintf: "); cout << '\n'; @@ -316,6 +388,10 @@ private: // Done. Delete original m_cellRangep = nullptr; if (isIface) { + // Hand the per-element vars to the orphan-VarRef rewriter. + if (!perElemVarps.empty()) { + m_dearrayedIfaceVars.emplace(ifaceVarp, std::move(perElemVarps)); + } ifaceVarp->unlinkFrBack(); VL_DO_DANGLING(pushDeletep(ifaceVarp), ifaceVarp); } @@ -709,7 +785,11 @@ private: public: // CONSTRUCTORS - explicit InstDeVisitor(AstNetlist* nodep) { iterate(nodep); } + explicit InstDeVisitor(AstNetlist* nodep) { + iterate(nodep); + // Skip when nothing was deleted; designs without iface arrays pay nothing. + if (!m_dearrayedIfaceVars.empty()) { InstDeOrphanVisitor{nodep, m_dearrayedIfaceVars}; } + } ~InstDeVisitor() override = default; }; diff --git a/test_regress/t/t_interface_array_class_new.py b/test_regress/t/t_interface_array_class_new.py new file mode 100755 index 000000000..46d1fe4c0 --- /dev/null +++ b/test_regress/t/t_interface_array_class_new.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('simulator') + +test.compile(verilator_flags2=['--binary']) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_interface_array_class_new.v b/test_regress/t/t_interface_array_class_new.v new file mode 100644 index 000000000..c52ccc502 --- /dev/null +++ b/test_regress/t/t_interface_array_class_new.v @@ -0,0 +1,42 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// Passing a real interface array as a class new() argument bound to a +// virtual interface array formal parameter. Exercises the cell dearrayer's +// orphan-VarRef fixup in V3Inst. +// +// 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 checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0) +// verilog_format: on + +interface AXI_BUS_DV; +endinterface + +package tb_axi_xbar_pkg; + class axi_xbar_monitor #( + parameter int unsigned NoMasters + ); + int captured_count; + function new(virtual AXI_BUS_DV axi_masters_vif[NoMasters-1:0]); + captured_count = NoMasters; + endfunction + endclass +endpackage + +module t; + localparam int unsigned TbNumMasters = 32'd6; + + AXI_BUS_DV master_monitor_dv[TbNumMasters-1:0] (); + + initial begin + static tb_axi_xbar_pkg::axi_xbar_monitor #(.NoMasters(TbNumMasters)) + monitor = new(master_monitor_dv); + `checkh(monitor.captured_count, 32'd6); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule