diff --git a/docs/CONTRIBUTORS b/docs/CONTRIBUTORS index 649e68340..cf0f40070 100644 --- a/docs/CONTRIBUTORS +++ b/docs/CONTRIBUTORS @@ -259,6 +259,7 @@ Srinivasan Venkataramanan Stefan Wallentowitz Stephen Henry Steven Hugg +Stuart Morris Szymon Gizler Sören Tempel Teng Huang diff --git a/src/V3Inline.cpp b/src/V3Inline.cpp index 05714a616..d0a501d03 100644 --- a/src/V3Inline.cpp +++ b/src/V3Inline.cpp @@ -256,6 +256,14 @@ class InlineRelinkVisitor final : public VNVisitor { // STATE std::unordered_set m_renamedInterfaces; // Name of renamed interface variables + std::unordered_set + m_priorInlinedCells; // Cells previously inlined into the module being inlined here. + // Used to recognize VarXRefs whose inlinedDots was stamped by a + // prior V3Inline pass (vs. by V3Begin generate-block unrolling). + std::unordered_set + m_pinSubstitutedXRefs; // VarXRefs created by pin substitution in this relink pass. + // Their dotted/inlinedDots already represent the parent's scope + // and must not be rewritten on the immediate visit. AstNodeModule* const m_modp; // The module we are inlining into const AstCell* const m_cellp; // The cell being inlined size_t m_nPlaceholders = 0; // Unique identifier sequence number for placeholder variables @@ -373,20 +381,47 @@ class InlineRelinkVisitor final : public VNVisitor { AstVarXRef* const newp = new AstVarXRef{nodep->fileline(), xrefp->name(), xrefp->dotted(), nodep->access()}; newp->varp(xrefp->varp()); - // Copy inlinedDots from pin expression - the normal visitor iteration will - // prepend the cell name when this VarXRef is visited later + // The pin expression came from m_modp (the parent we are inlining into), so its + // dotted/inlinedDots already describe a path in m_modp's scope. Record this xref + // so visit(AstVarXRef) leaves it alone on the immediate visit; later inline + // passes will prepend their cell names normally. newp->inlinedDots(xrefp->inlinedDots()); + m_pinSubstitutedXRefs.insert(newp); nodep->replaceWith(newp); VL_DO_DANGLING(nodep->deleteTree(), nodep); - // Note: Don't call iterate(newp) here - the node will be visited during - // normal tree iteration which will apply the inlining transformations } void visit(AstVarXRef* nodep) override { + // VarXRefs just created by pin substitution in this pass already describe a path + // in m_modp's scope (the parent we are inlining into). Leave them untouched on + // this immediate visit; subsequent inline passes will prepend their cell names. + if (m_pinSubstitutedXRefs.erase(nodep)) { + iterateChildren(nodep); + return; + } // Track what scope it was originally under so V3LinkDot can resolve it - nodep->inlinedDots(VString::dot(m_cellp->name(), ".", nodep->inlinedDots())); + const string origInlinedDots = nodep->inlinedDots(); + nodep->inlinedDots(VString::dot(m_cellp->name(), ".", origInlinedDots)); + // If origInlinedDots starts with the name of a previously-inlined cell, this + // VarXRef came from that cell's body and its dotted refers to that child's + // local scope; renaming it against m_renamedInterfaces would wrongly alias it + // to a coincidentally-named var in the current module (#5120). VarXRefs whose + // inlinedDots was stamped by V3Begin generate-block unrolling are unaffected, + // since V3Begin's CellInlines have origModName "__BEGIN__" and don't appear in + // m_priorInlinedCells. + const string::size_type firstDot = origInlinedDots.find('.'); + const string firstSeg + = firstDot == string::npos ? origInlinedDots : origInlinedDots.substr(0, firstDot); + const bool fromPriorInline = m_priorInlinedCells.count(firstSeg); for (string tryname = nodep->dotted(); true;) { if (m_renamedInterfaces.count(tryname)) { - nodep->dotted(m_cellp->name() + "__DOT__" + nodep->dotted()); + // matchIsRenamed: the matched name itself was created by a prior V3Inline + // rename (contains "__DOT__"). When true, we are following the chain of + // renames for the same var across nested inlines, so apply the rename + // even if the VarXRef came from a prior-inlined child. + const bool matchIsRenamed = tryname.find("__DOT__") != string::npos; + if (!fromPriorInline || matchIsRenamed) { + nodep->dotted(m_cellp->name() + "__DOT__" + nodep->dotted()); + } break; } // If foo.bar, and foo is an interface, then need to search again for foo @@ -432,6 +467,16 @@ public: InlineRelinkVisitor(AstNodeModule* cloneModp, AstNodeModule* oldModp, AstCell* cellp) : m_modp{oldModp} , m_cellp{cellp} { + // CellInlines added by V3Begin for generate/named blocks have origModName + // "__BEGIN__"; only those added by prior V3Inline passes carry a real module + // name. Track the latter so visit(AstVarXRef) can distinguish VarXRefs + // originating from previously-inlined children. + for (AstNode* nodep = cloneModp->inlinesp(); nodep; nodep = nodep->nextp()) { + const AstCellInline* const cip = VN_CAST(nodep, CellInline); + if (cip && cip->origModName() != "__BEGIN__") { + m_priorInlinedCells.insert(cip->name()); + } + } iterate(cloneModp); } ~InlineRelinkVisitor() override = default; diff --git a/test_regress/t/t_iface_name_collision.py b/test_regress/t/t_iface_name_collision.py new file mode 100644 index 000000000..ee8fad784 --- /dev/null +++ b/test_regress/t/t_iface_name_collision.py @@ -0,0 +1,16 @@ +#!/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.lint(verilator_flags2=["--lint-only -Werror-MULTIDRIVEN -Werror-UNOPTFLAT"]) + +test.passes() diff --git a/test_regress/t/t_iface_name_collision.v b/test_regress/t/t_iface_name_collision.v new file mode 100644 index 000000000..69fac79bc --- /dev/null +++ b/test_regress/t/t_iface_name_collision.v @@ -0,0 +1,49 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// The code describes a very simple hierarchy of modules. +// The lowest level instantiates avst_interface with name my_avst_if. +// The highest level also instantiates the same interface type +// with the exact same name. The file should lint with no errors +// or warnings other than those disabled by lint_off. +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: CC0-1.0 + +/* verilator lint_off DECLFILENAME */ +/* verilator lint_off UNUSEDSIGNAL */ +interface avst_interface; + logic ready; + logic valid; + + modport sink_mp (output ready, input valid); +endinterface + +module child ( + output logic ready_out +); + avst_interface my_avst_if(); + + assign ready_out = my_avst_if.ready; + assign my_avst_if.ready = 1'b1; // drives child.my_avst_if.ready only +endmodule + +module wrapper ( + avst_interface.sink_mp my_avst_if +); + child child_inst ( + .ready_out (my_avst_if.ready) // sole driver of outer my_avst_if.ready + ); +endmodule + +module top ( + input logic in_valid, + output logic out_ready +); + avst_interface my_avst_if(); + + assign my_avst_if.valid = in_valid; + assign out_ready = my_avst_if.ready; + + wrapper wrapper_inst (.my_avst_if (my_avst_if)); +endmodule