From 350158c857def29a35f4cb52a5182bb1591338ab Mon Sep 17 00:00:00 2001 From: Artur Bieniek Date: Wed, 24 Jun 2026 11:51:42 +0200 Subject: [PATCH] Fix scheduling of virtual interface method writes (#7641) Fix scheduling of writes in virtual interfaces, there were missing triggers (see added test). Make V3SchedVirtIface handle writes done inside methods called through a virtual interface. The pass first records direct vif.member writes, VIF method calls, and candidate interface member VarScopes. It then walks the methods reachable from those VIF calls, writes to persistent interface variables in those method bodies are treated as VIF writes, and nested calls are followed with the same interface context. Function locals, temps, and events are ignored because they are not persistent interface storage observable through a later VIF read. Triggers are still created only from the intersection of (interface type, member name) writes and matching VarScopes, so unrelated interface variables and interfaces with no virtual access do not get extra triggers. --- src/V3SchedVirtIface.cpp | 67 ++++++++++++++-- .../t/t_virtual_interface_method_sched.py | 18 +++++ .../t/t_virtual_interface_method_sched.v | 79 +++++++++++++++++++ 3 files changed, 158 insertions(+), 6 deletions(-) create mode 100755 test_regress/t/t_virtual_interface_method_sched.py create mode 100644 test_regress/t/t_virtual_interface_method_sched.v diff --git a/src/V3SchedVirtIface.cpp b/src/V3SchedVirtIface.cpp index 4dd95662d..41684cd2b 100644 --- a/src/V3SchedVirtIface.cpp +++ b/src/V3SchedVirtIface.cpp @@ -39,11 +39,18 @@ namespace { class VirtIfaceVisitor final : public VNVisitor { private: // STATE + using IfaceMember = std::pair; + using IfaceCallable = std::pair; + // Set of (iface, member) pairs written through VIF -- defines which members need triggers - std::set> m_vifWrittenMembers; + std::set m_vifWrittenMembers; // All candidate VarScopes of interface members (keyed by interface type + member name) - std::map, std::vector> - m_candidateVscps; + std::map> m_candidateVscps; + std::set> m_seenCandidateVscps; + // VarScope index and callable worklist for VIF method-body writes. + std::map> m_vscpsByVar; + std::vector m_reachableIfaceCallables; + std::set m_seenReachableIfaceCallables; VirtIfaceTriggers m_triggers; // METHODS @@ -70,12 +77,24 @@ private: } iterateChildren(nodep); } + void visit(AstCMethodCall* nodep) override { + if (const AstIfaceRefDType* const dtypep + = VN_CAST(nodep->fromp()->dtypep()->skipRefp(), IfaceRefDType)) { + if (VL_UNCOVERABLE(!dtypep->isVirtual())) { + // Concrete interface method calls are lowered before this pass. + } else { + addReachableIfaceCallable(dtypep->ifaceViaCellp(), nodep->funcp()); + } + } + iterateChildren(nodep); + } void visit(AstVarScope* nodep) override { // Collect candidate VarScopes. sensIfacep() is set on interface members // accessed via any MemberSel (virtual or non-virtual). - if (const AstIface* const ifacep = nodep->varp()->sensIfacep()) { - m_candidateVscps[{ifacep, nodep->varp()->name()}].push_back(nodep); - } + AstVar* const varp = nodep->varp(); + if (varp->isTemp()) return; + m_vscpsByVar[varp].push_back(nodep); + if (const AstIface* const ifacep = varp->sensIfacep()) addCandidateVscp(ifacep, nodep); } void visit(AstNodeProcedure* nodep) override { // Disable lifetime optimization for variables in AlwaysPost blocks @@ -87,6 +106,19 @@ private: } void visit(AstNode* nodep) override { iterateChildren(nodep); } + void addCandidateVscp(const AstIface* const ifacep, AstVarScope* const vscp) { + const IfaceMember member{ifacep, vscp->varp()->name()}; + if (m_seenCandidateVscps.emplace(member, vscp).second) + m_candidateVscps[member].push_back(vscp); + } + + void addReachableIfaceCallable(AstIface* const ifacep, AstCFunc* const funcp) { + const IfaceCallable callable{ifacep, funcp}; + if (m_seenReachableIfaceCallables.emplace(callable).second) { + m_reachableIfaceCallables.push_back(callable); + } + } + // Build final trigger list by intersecting VIF writes with candidate VarScopes void buildTriggers() { for (const auto& written : m_vifWrittenMembers) { @@ -103,6 +135,29 @@ public: // CONSTRUCTORS explicit VirtIfaceVisitor(AstNetlist* nodep) { iterate(nodep); + for (size_t i = 0; i < m_reachableIfaceCallables.size(); ++i) { + const IfaceCallable callable = m_reachableIfaceCallables[i]; + callable.second->foreach([this, callable](AstNodeExpr* const nodep) { + if (AstVarRef* const refp = VN_CAST(nodep, VarRef)) { + // Only persistent interface storage is observable through a VIF read. + UASSERT_OBJ(refp->varScopep(), refp, "No var scope"); + AstVar* const varp = refp->varp(); + if (!refp->access().isWriteOrRW() || varp->isFuncLocal() || varp->isTemp() + || varp->isEvent() || !VN_IS(refp->varScopep()->scopep()->modp(), Iface)) { + return; + } + varp->sensIfacep(callable.first); + m_vifWrittenMembers.emplace(callable.first, varp->name()); + const auto it = m_vscpsByVar.find(varp); + UASSERT_OBJ(it != m_vscpsByVar.end(), varp, + "No VarScope for interface member"); + for (AstVarScope* const vscp : it->second) + addCandidateVscp(callable.first, vscp); + } else if (AstNodeCCall* const callp = VN_CAST(nodep, NodeCCall)) { + addReachableIfaceCallable(callable.first, callp->funcp()); + } + }); + } buildTriggers(); } ~VirtIfaceVisitor() override = default; diff --git a/test_regress/t/t_virtual_interface_method_sched.py b/test_regress/t/t_virtual_interface_method_sched.py new file mode 100755 index 000000000..6ac2815da --- /dev/null +++ b/test_regress/t/t_virtual_interface_method_sched.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=["--timing"]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_virtual_interface_method_sched.v b/test_regress/t/t_virtual_interface_method_sched.v new file mode 100644 index 000000000..48dc01565 --- /dev/null +++ b/test_regress/t/t_virtual_interface_method_sched.v @@ -0,0 +1,79 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Antmicro +// SPDX-License-Identifier: CC0-1.0 + +class msg; + string context_name; + + function void set_context(string name); + context_name = name; + endfunction +endclass + +package helper_pkg; + int package_counter; + + function void bump(); + package_counter++; + endfunction +endpackage + +interface intf(output logic a, output logic b); + msg m; + event e; + + task go(); + helper_pkg::package_counter++; + go_helper(); + go_helper(); + endtask + + task go_helper(); + // verilator no_inline_task + m.set_context("go_helper"); + helper_pkg::bump(); + -> e; + a <= 1; + endtask +endinterface + +class driver; + virtual intf vif; + + function new(virtual intf vif); + this.vif = vif; + endfunction + + task go(); + vif.go(); + endtask +endclass + +module t; + wire a; + wire b; + virtual intf vif; + + intf i(a, b); + + initial begin + driver d; + vif = t.i; + t.i.m = new; + d = new(t.i); + d.go(); + end + + always @(posedge a) begin + vif.b <= 1; + end + + always @(*) begin + if (a && b) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end +endmodule