Fix broken support of unassigned virtual interfaces (#6253) (#6338)

* Fix broken support of unassigned virtual interfaces

Unassigned virtual interface support added by #6245 is broken - PR marks
dead module as alive - we can't do that as once a module is dead it
needs to remain dead because earlier steps (e.g. port resolution) have
already been skipped.

This commit handles unassigned virtual interfaces at the beginning of
first pass of LinkDot (so it is never marked as dead, and no linking
steps are getting skipped).

Fixes #6253.

* Apply suggestions from code review

Co-authored-by: Wilson Snyder <wsnyder@wsnyder.org>

* Apply 'make format'

* Revert add of redundant iterateChildren() call

* Add original test case

---------

Co-authored-by: Wilson Snyder <wsnyder@wsnyder.org>
Co-authored-by: github action <action@example.com>
This commit is contained in:
Szymon Gizler 2025-08-28 15:03:46 +02:00 committed by GitHub
parent 1043a6c6be
commit 8868d459a2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 164 additions and 18 deletions

View File

@ -533,22 +533,9 @@ public:
continue;
} else if (ifacerefp->ifaceViaCellp()->dead()
|| !existsNodeSym(ifacerefp->ifaceViaCellp())) {
// virtual interface never assigned any actual interface
ifacerefp->ifacep()->dead(false);
varp->dtypep(ifacerefp->dtypep());
// Create dummy cell to keep the virtual interface alive
// (later stages assume that non-dead interface has associated cell)
AstCell* const ifacecellp
= new AstCell{ifacerefp->ifacep()->fileline(),
ifacerefp->ifacep()->fileline(),
ifacerefp->ifacep()->name() + "__02E" + varp->name(),
ifacerefp->ifaceName(),
nullptr,
nullptr,
nullptr};
ifacecellp->modp(ifacerefp->ifacep());
VSymEnt* const symp = new VSymEnt{&m_syms, ifacecellp};
ifacerefp->ifacep()->user1p(symp);
UASSERT_OBJ(false, ifacerefp->ifaceViaCellp(),
"Interface referenced by virtual iface is dead. "
"Dead interfaces have required linking steps skipped");
}
VSymEnt* const ifaceSymp = getNodeSym(ifacerefp->ifaceViaCellp());
VSymEnt* ifOrPortSymp = ifaceSymp;
@ -874,6 +861,14 @@ LinkDotState* LinkDotState::s_errorThisp = nullptr;
//======================================================================
class LinkDotFindVisitor final : public VNVisitor {
// NODE STATE
// Cleared on global
// *::user1p() -> See LinkDotState
// *::user2p() -> See LinkDotState
// *::user3() // bool. Node stored in m_virtualIfaces
// *::user4() -> See LinkDotState
const VNUser3InUse m_inuser3;
// STATE
LinkDotState* const m_statep; // State to pass between visitors, including symbol table
AstNodeModule* m_classOrPackagep = nullptr; // Current package
@ -891,6 +886,8 @@ class LinkDotFindVisitor final : public VNVisitor {
int m_modBlockNum = 0; // Begin block number in module, 0=none seen
int m_modWithNum = 0; // With block number, 0=none seen
int m_modArgNum = 0; // Arg block number for randomize(), 0=none seen
std::vector<AstIface*> m_virtIfaces; // interfaces used as virtual,
// needed for handleUnvisitedVirtIfaces()
// METHODS
void makeImplicitNew(AstClass* nodep) {
@ -986,6 +983,14 @@ class LinkDotFindVisitor final : public VNVisitor {
}
void visit(AstTypeTable*) override {} // FindVisitor::
void visit(AstConstPool*) override {} // FindVisitor::
void visit(AstIfaceRefDType* nodep) override {
if (m_statep->forPrimary() && nodep->isVirtual() && nodep->ifacep()
&& !nodep->ifacep()->user3()) {
m_virtIfaces.push_back(nodep->ifacep());
nodep->ifacep()->user3(true);
}
iterateChildren(nodep);
}
void visit(AstNodeModule* nodep) override { // FindVisitor::
// Called on top module from Netlist, other modules from the cell creating them,
// and packages
@ -1821,12 +1826,36 @@ class LinkDotFindVisitor final : public VNVisitor {
void visit(AstNode* nodep) override { iterateChildren(nodep); } // FindVisitor::
void handleUnvisitedVirtIfaces() {
AstNodeModule* const top = v3Global.rootp()->modulesp();
m_curSymp = m_statep->getNodeSym(top);
for (AstIface* ifacep : m_virtIfaces) {
if (!ifacep->user4()) { // virtual interface never assigned any actual interface
// Create dummy cell to keep the virtual interface alive
// (later stages assume that non-dead interface has associated cell)
AstCell* const ifacecellp = new AstCell{ifacep->fileline(),
ifacep->fileline(),
"__VdummyInstOf" + ifacep->name(),
ifacep->name(),
nullptr,
nullptr,
nullptr};
ifacecellp->modp(ifacep);
top->addStmtsp(ifacecellp);
iterate(ifacecellp);
}
}
}
public:
// CONSTRUCTORS
LinkDotFindVisitor(AstNetlist* rootp, LinkDotState* statep)
: m_statep{statep} {
UINFO(4, __FUNCTION__ << ": ");
iterate(rootp);
if (!m_virtIfaces.empty()) handleUnvisitedVirtIfaces();
}
~LinkDotFindVisitor() override = default;
};
@ -2877,7 +2906,7 @@ class LinkDotResolveVisitor final : public VNVisitor {
void visit(AstTypeTable*) override {}
void visit(AstConstPool*) override {}
void visit(AstNodeModule* nodep) override {
if (nodep->dead() || !m_statep->existsNodeSym(nodep)) return;
if (nodep->dead()) return;
LINKDOT_VISIT_START();
UINFO(8, indent() << "visit " << nodep);
VL_RESTORER(m_genericIfaceModule);

View File

@ -0,0 +1,5 @@
%Error: t/t_interface_virtual_missing_bad.v:9:12: Cannot find file containing interface: 'foo'
9 | virtual foo vif;
| ^~~
... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance.
%Error: Exiting due to

View File

@ -0,0 +1,16 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2025 by Wilson Snyder. 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-License-Identifier: LGPL-3.0-only OR Artistic-2.0
import vltest_bootstrap
test.scenarios('linter')
test.lint(fails=True, expect_filename=test.golden_filename)
test.passes()

View File

@ -0,0 +1,15 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2025 by Antmicro.
// SPDX-License-Identifier: CC0-1.0
module t (/*AUTOARG*/);
virtual foo vif;
initial begin
$write("*-* All Finished *-*\n");
$finish;
end
endmodule

View File

@ -6,7 +6,7 @@
// See also t_interface_virtual.v
interface QBus;
interface QBus();
endinterface
module t (/*AUTOARG*/);

View File

@ -0,0 +1,18 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2025 by Wilson Snyder. 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-License-Identifier: LGPL-3.0-only OR Artistic-2.0
import vltest_bootstrap
test.scenarios('simulator')
test.compile()
test.execute()
test.passes()

View File

@ -0,0 +1,27 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2025 by Antmicro.
// SPDX-License-Identifier: CC0-1.0
interface QBus(input logic k);
logic data;
endinterface
class cls;
virtual QBus vif1;
function foo(virtual QBus vif2);
vif2.data = 1;
endfunction
endclass
module t (/*AUTOARG*/);
cls bar;
initial begin
$write("*-* All Finished *-*\n");
$finish;
end
endmodule

View File

@ -0,0 +1,16 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2025 by Wilson Snyder. 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-License-Identifier: LGPL-3.0-only OR Artistic-2.0
import vltest_bootstrap
test.scenarios('simulator')
test.compile()
test.passes()

View File

@ -0,0 +1,20 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2025 by Wilson Snyder.
// SPDX-License-Identifier: CC0-1.0
interface stream_ifc #(
) (
input logic clk_i
);
endinterface
package pkg;
class stream_driver #();
virtual stream_ifc stream;
endclass
endpackage
module t;
endmodule