From dc26dd601d6e86c71ecccdb969b118a17ab147ea Mon Sep 17 00:00:00 2001 From: Igor Zaworski Date: Fri, 6 Feb 2026 23:20:10 +0100 Subject: [PATCH] Fix internal error - virtual interface not found (#7010) --- src/V3Life.cpp | 2 +- src/V3Sched.cpp | 75 ++++++++----------- src/V3Sched.h | 8 -- src/V3SchedReplicate.cpp | 2 +- src/V3SchedVirtIface.cpp | 43 ++++++----- test_regress/t/t_interface_virtual_unsup.out | 11 --- test_regress/t/t_interface_virtual_unsup.v | 30 -------- .../t_virtual_interface_only_with_assignw.py | 18 +++++ .../t/t_virtual_interface_only_with_assignw.v | 55 ++++++++++++++ 9 files changed, 132 insertions(+), 112 deletions(-) delete mode 100644 test_regress/t/t_interface_virtual_unsup.out delete mode 100644 test_regress/t/t_interface_virtual_unsup.v create mode 100755 test_regress/t/t_virtual_interface_only_with_assignw.py create mode 100644 test_regress/t/t_virtual_interface_only_with_assignw.v diff --git a/src/V3Life.cpp b/src/V3Life.cpp index 00e27e976..573afc79a 100644 --- a/src/V3Life.cpp +++ b/src/V3Life.cpp @@ -171,7 +171,7 @@ public: entr.init(false); } else { if (AstConst* const constp = entr.constNodep()) { - if (!varrefp->varp()->isSigPublic() && !varrefp->varp()->sensIfacep()) { + if (!varrefp->varp()->isSigPublic() && !varrefp->varp()->isVirtIface()) { // Aha, variable is constant; substitute in. // We'll later constant propagate UINFO(4, " replaceconst: " << varrefp); diff --git a/src/V3Sched.cpp b/src/V3Sched.cpp index 248c4fb99..2953353c9 100644 --- a/src/V3Sched.cpp +++ b/src/V3Sched.cpp @@ -86,18 +86,32 @@ void invertAndMergeSenTreeMap( } std::vector -findTriggeredIface(const AstVarScope* vscp, const VirtIfaceTriggers::IfaceSensMap& vifTrigged, +findTriggeredIface(const AstVarScope* vscp, const VirtIfaceTriggers::IfaceMemberSensMap& vifMemberTriggered) { - UASSERT_OBJ(vscp->varp()->sensIfacep(), vscp, "Not an virtual interface trigger"); - std::vector result; - const auto ifaceIt = vifTrigged.find(vscp->varp()->sensIfacep()); - if (ifaceIt != vifTrigged.end()) result.push_back(ifaceIt->second); - for (const auto& memberIt : vifMemberTriggered) { - if (vscp->varp()->sensIfacep() == memberIt.first.m_ifacep) { - result.push_back(memberIt.second); - } + const AstIface* ifacep; + if (vscp->varp()->isVirtIface()) { + // If `vscp->varp()->isVirtIface()` is true then the interface type that viface is pointing + // to is under `VN_AS(vscp->varp()->dtypep(), IfaceRefDType)->ifacep()` + + ifacep = VN_AS(vscp->varp()->dtypep(), IfaceRefDType)->ifacep(); + + // Virtual interface is sensitive to a different interface type than it is a virtual type + // of - this may be a valid behaviour but this function does not expects that + UASSERT_OBJ(vscp->varp()->sensIfacep() == nullptr, vscp, + "Virtual interface has an ambiguous type - " + << vscp->varp()->sensIfacep()->prettyTypeName() + << " != " << ifacep->prettyTypeName()); + } else { + // If `vscp->varp()` is of a non-virtual interface type it has `sensIfacep()` set to + // interface it is sensitive to + ifacep = vscp->varp()->sensIfacep(); } - if (result.empty()) vscp->v3fatalSrc("Did not find virtual interface trigger"); + UASSERT_OBJ(ifacep, vscp, "Variable is not sensitive for any interface"); + std::vector result; + for (const auto& memberIt : vifMemberTriggered) { + if (memberIt.first.m_ifacep == ifacep) result.push_back(memberIt.second); + } + UASSERT_OBJ(!result.empty(), vscp, "Did not find virtual interface trigger"); return result; } @@ -381,10 +395,6 @@ void createFinal(AstNetlist* netlistp, const LogicClasses& logicClasses) { void addVirtIfaceTriggerAssignments(const VirtIfaceTriggers& virtIfaceTriggers, uint32_t vifTriggerIndex, uint32_t vifMemberTriggerIndex, const TriggerKit& trigKit) { - for (const auto& p : virtIfaceTriggers.m_ifaceTriggers) { - trigKit.addExtraTriggerAssignment(p.second, vifTriggerIndex); - ++vifTriggerIndex; - } for (const auto& p : virtIfaceTriggers.m_memberTriggers) { trigKit.addExtraTriggerAssignment(p.second, vifMemberTriggerIndex); ++vifMemberTriggerIndex; @@ -479,9 +489,6 @@ AstNode* createInputCombLoop(AstNetlist* netlistp, AstCFunc* const initFuncp, ? extraTriggers.allocate("DPI export trigger") : std::numeric_limits::max(); const size_t firstVifTriggerIndex = extraTriggers.size(); - for (const auto& p : virtIfaceTriggers.m_ifaceTriggers) { - extraTriggers.allocate("virtual interface: " + p.first->name()); - } const size_t firstVifMemberTriggerIndex = extraTriggers.size(); for (const auto& p : virtIfaceTriggers.m_memberTriggers) { const auto& item = p.first; @@ -516,8 +523,6 @@ AstNode* createInputCombLoop(AstNetlist* netlistp, AstCFunc* const initFuncp, = dpiExportTriggerVscp ? trigKit.newExtraTriggerSenTree(trigKit.vscp(), dpiExportTriggerIndex) : nullptr; - const auto& vifTriggeredIco - = virtIfaceTriggers.makeIfaceToSensMap(trigKit, firstVifTriggerIndex, trigKit.vscp()); const auto& vifMemberTriggeredIco = virtIfaceTriggers.makeMemberToSensMap( trigKit, firstVifMemberTriggerIndex, trigKit.vscp()); @@ -530,9 +535,9 @@ AstNode* createInputCombLoop(AstNetlist* netlistp, AstCFunc* const initFuncp, out.push_back(inputChanged); } if (varp->isWrittenByDpi()) out.push_back(dpiExportTriggered); - if (vscp->varp()->sensIfacep()) { + if (vscp->varp()->isVirtIface()) { std::vector ifaceTriggered - = findTriggeredIface(vscp, vifTriggeredIco, vifMemberTriggeredIco); + = findTriggeredIface(vscp, vifMemberTriggeredIco); out.insert(out.end(), ifaceTriggered.begin(), ifaceTriggered.end()); } }); @@ -721,17 +726,6 @@ void createEval(AstNetlist* netlistp, // //============================================================================ // Helper that builds virtual interface trigger sentrees -VirtIfaceTriggers::IfaceSensMap -VirtIfaceTriggers::makeIfaceToSensMap(const TriggerKit& trigKit, uint32_t vifTriggerIndex, - AstVarScope* trigVscp) const { - std::map map; - for (const auto& p : m_ifaceTriggers) { - map.emplace(p.first, trigKit.newExtraTriggerSenTree(trigVscp, vifTriggerIndex)); - ++vifTriggerIndex; - } - return map; -} - VirtIfaceTriggers::IfaceMemberSensMap VirtIfaceTriggers::makeMemberToSensMap(const TriggerKit& trigKit, uint32_t vifTriggerIndex, AstVarScope* trigVscp) const { @@ -860,9 +854,6 @@ void schedule(AstNetlist* netlistp) { ? extraTriggers.allocate("DPI export trigger") : std::numeric_limits::max(); const uint32_t firstVifTriggerIndex = extraTriggers.size(); - for (const auto& p : virtIfaceTriggers.m_ifaceTriggers) { - extraTriggers.allocate("virtual interface: " + p.first->name()); - } const uint32_t firstVifMemberTriggerIndex = extraTriggers.size(); for (const auto& p : virtIfaceTriggers.m_memberTriggers) { const auto& item = p.first; @@ -915,8 +906,6 @@ void schedule(AstNetlist* netlistp) { ? trigKit.newExtraTriggerSenTree(trigKit.vscp(), dpiExportTriggerIndex) : nullptr; - const auto& vifTriggeredAct - = virtIfaceTriggers.makeIfaceToSensMap(trigKit, firstVifTriggerIndex, trigKit.vscp()); const auto& vifMemberTriggeredAct = virtIfaceTriggers.makeMemberToSensMap( trigKit, firstVifMemberTriggerIndex, trigKit.vscp()); @@ -926,9 +915,9 @@ void schedule(AstNetlist* netlistp) { auto it = actTimingDomains.find(vscp); if (it != actTimingDomains.end()) out = it->second; if (vscp->varp()->isWrittenByDpi()) out.push_back(dpiExportTriggeredAct); - if (vscp->varp()->sensIfacep()) { + if (vscp->varp()->isVirtIface()) { std::vector ifaceTriggered - = findTriggeredIface(vscp, vifTriggeredAct, vifMemberTriggeredAct); + = findTriggeredIface(vscp, vifMemberTriggeredAct); out.insert(out.end(), ifaceTriggered.begin(), ifaceTriggered.end()); } }); @@ -954,8 +943,6 @@ void schedule(AstNetlist* netlistp) { = dpiExportTriggerVscp ? trigKit.newExtraTriggerSenTree(trigVscp, dpiExportTriggerIndex) : nullptr; - const auto& vifTriggered - = virtIfaceTriggers.makeIfaceToSensMap(trigKit, firstVifTriggerIndex, trigVscp); const auto& vifMemberTriggered = virtIfaceTriggers.makeMemberToSensMap(trigKit, firstVifMemberTriggerIndex, trigVscp); @@ -966,9 +953,11 @@ void schedule(AstNetlist* netlistp) { auto it = timingDomains.find(vscp); if (it != timingDomains.end()) out = it->second; if (vscp->varp()->isWrittenByDpi()) out.push_back(dpiExportTriggered); - if (vscp->varp()->sensIfacep()) { + // Sometimes virtual interfaces mix with non-virtual one so, here both have to be + // detected - look `t_virtual_interface_nba_assign` + if (vscp->varp()->sensIfacep() || vscp->varp()->isVirtIface()) { std::vector ifaceTriggered - = findTriggeredIface(vscp, vifTriggered, vifMemberTriggered); + = findTriggeredIface(vscp, vifMemberTriggered); out.insert(out.end(), ifaceTriggered.begin(), ifaceTriggered.end()); } }); diff --git a/src/V3Sched.h b/src/V3Sched.h index 90e25ab59..13255c78c 100644 --- a/src/V3Sched.h +++ b/src/V3Sched.h @@ -338,15 +338,10 @@ class VirtIfaceTriggers final { }; public: - using IfaceSensMap = std::map; using IfaceMemberSensMap = std::map; - std::vector> m_ifaceTriggers; std::vector> m_memberTriggers; - void addIfaceTrigger(const AstIface* ifacep, AstVarScope* vscp) { - m_ifaceTriggers.emplace_back(ifacep, vscp); - } void addMemberTrigger(const AstIface* ifacep, const AstVar* memberp, AstVarScope* vscp) { m_memberTriggers.emplace_back(IfaceMember{ifacep, memberp}, vscp); } @@ -362,9 +357,6 @@ public: IfaceMemberSensMap makeMemberToSensMap(const TriggerKit& trigKit, uint32_t vifTriggerIndex, AstVarScope* trigVscp) const; - IfaceSensMap makeIfaceToSensMap(const TriggerKit& trigKit, uint32_t vifTriggerIndex, - AstVarScope* trigVscp) const; - VL_UNCOPYABLE(VirtIfaceTriggers); VirtIfaceTriggers() = default; VirtIfaceTriggers(VirtIfaceTriggers&&) = default; diff --git a/src/V3SchedReplicate.cpp b/src/V3SchedReplicate.cpp index 979100045..79596533e 100644 --- a/src/V3SchedReplicate.cpp +++ b/src/V3SchedReplicate.cpp @@ -151,7 +151,7 @@ public: , m_vscp{vscp} { // Top level inputs are if (varp()->isPrimaryInish() || varp()->isSigUserRWPublic() || varp()->isWrittenByDpi() - || varp()->sensIfacep()) { + || varp()->sensIfacep() || varp()->isVirtIface()) { addDrivingRegions(INPUT); } // Currently we always execute suspendable processes at the beginning of diff --git a/src/V3SchedVirtIface.cpp b/src/V3SchedVirtIface.cpp index 8220b3ca4..7d19d7c7a 100644 --- a/src/V3SchedVirtIface.cpp +++ b/src/V3SchedVirtIface.cpp @@ -42,6 +42,7 @@ class VirtIfaceVisitor final : public VNVisitor { private: // NODE STATE // AstVarRef::user1() -> bool. Whether it has been visited + // AstMemberSel::user1() -> bool. Whether it has been visited const VNUser1InUse m_user1InUse; // TYPES @@ -64,7 +65,7 @@ private: AstIfaceRefDType* const dtypep = VN_CAST(refp->varp()->dtypep(), IfaceRefDType); const bool writesToVirtIfaceMember = (dtypep && dtypep->isVirtual() && VN_IS(refp->firstAbovep(), MemberSel)); - const bool writesToIfaceSensVar = refp->varp()->sensIfacep(); + const bool writesToIfaceSensVar = refp->varp()->isVirtIface(); return writesToVirtIfaceMember || writesToIfaceSensVar; }); } @@ -86,27 +87,21 @@ private: return new AstVarRef{flp, existingTrigger, VAccess::WRITE}; } - // VISITORS - void visit(AstNodeProcedure* nodep) override { - // Not sure if needed, but be paranoid to match previous behavior as didn't optimize - // before .. - if (VN_IS(nodep, AlwaysPost) && writesToVirtIface(nodep)) { - nodep->foreach([](AstVarRef* refp) { refp->varScopep()->optimizeLifePost(false); }); - } - iterateChildren(nodep); - } - void visit(AstVarRef* const nodep) override { + template + void handleIface(T nodep) { + static_assert(std::is_same::type, + typename std::add_pointer::type>::value + || std::is_same::type, + typename std::add_pointer::type>::value, + "Node has to be of AstVarRef* or AstMemberSel* type"); if (nodep->access().isReadOnly()) return; if (nodep->user1SetOnce()) return; AstIface* ifacep = nullptr; AstVar* memberVarp = nullptr; - if (AstIfaceRefDType* const dtypep = VN_CAST(nodep->varp()->dtypep(), IfaceRefDType)) { - if (dtypep->isVirtual()) { - if (AstMemberSel* const memberSelp = VN_CAST(nodep->firstAbovep(), MemberSel)) { - // Extract the member varp from the MemberSel node - memberVarp = memberSelp->varp(); - ifacep = dtypep->ifacep(); - } + if (nodep->varp()->isVirtIface()) { + if (AstMemberSel* const memberSelp = VN_CAST(nodep->firstAbovep(), MemberSel)) { + ifacep = VN_AS(nodep->varp()->dtypep(), IfaceRefDType)->ifacep(); + memberVarp = memberSelp->varp(); } } else if ((ifacep = nodep->varp()->sensIfacep())) { memberVarp = nodep->varp(); @@ -123,6 +118,18 @@ private: nodep}); } } + + // VISITORS + void visit(AstNodeProcedure* nodep) override { + // Not sure if needed, but be paranoid to match previous behavior as didn't optimize + // before .. + if (VN_IS(nodep, AlwaysPost) && writesToVirtIface(nodep)) { + nodep->foreach([](AstVarRef* refp) { refp->varScopep()->optimizeLifePost(false); }); + } + iterateChildren(nodep); + } + void visit(AstMemberSel* const nodep) override { handleIface(nodep); } + void visit(AstVarRef* const nodep) override { handleIface(nodep); } void visit(AstNode* nodep) override { iterateChildren(nodep); } public: diff --git a/test_regress/t/t_interface_virtual_unsup.out b/test_regress/t/t_interface_virtual_unsup.out deleted file mode 100644 index e4a69ebac..000000000 --- a/test_regress/t/t_interface_virtual_unsup.out +++ /dev/null @@ -1,11 +0,0 @@ -%Error-UNSUPPORTED: t/t_interface_virtual_unsup.v:24:22: Unsupported: Write to virtual interface in if condition - 24 | if (write_data(vif.data)) $write("dummy op"); - | ^~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest -%Error-UNSUPPORTED: t/t_interface_virtual_unsup.v:25:25: Unsupported: Write to virtual interface in loop condition - 25 | while (write_data(vif.data)); - | ^~~ -%Error-UNSUPPORTED: t/t_interface_virtual_unsup.v:26:30: Unsupported: Write to virtual interface in loop condition - 26 | do ; while (write_data(vif.data)); - | ^~~ -%Error: Exiting due to diff --git a/test_regress/t/t_interface_virtual_unsup.v b/test_regress/t/t_interface_virtual_unsup.v deleted file mode 100644 index a95166320..000000000 --- a/test_regress/t/t_interface_virtual_unsup.v +++ /dev/null @@ -1,30 +0,0 @@ -// DESCRIPTION: Verilator: Verilog Test module -// -// This file ONLY is placed under the Creative Commons Public Domain. -// SPDX-FileCopyrightText: 2023 Wilson Snyder -// SPDX-License-Identifier: CC0-1.0 - -// NOTE: Once this is supported, t_interface_virtual_cond is no longer needed - -interface Bus; - logic [15:0] data; -endinterface - -module t; - Bus intf(); - virtual Bus vif = intf; - - function logic write_data(output logic[15:0] data); - data = 'hdead; - return 1; - endfunction - - // verilator lint_off INFINITELOOP - initial begin - if (write_data(vif.data)) $write("dummy op"); - while (write_data(vif.data)); - do ; while (write_data(vif.data)); - for (int i = 0; write_data(vif.data++); i++); - end - -endmodule diff --git a/test_regress/t/t_virtual_interface_only_with_assignw.py b/test_regress/t/t_virtual_interface_only_with_assignw.py new file mode 100755 index 000000000..3ae669156 --- /dev/null +++ b/test_regress/t/t_virtual_interface_only_with_assignw.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2026 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(verilator_flags2=['--binary']) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_virtual_interface_only_with_assignw.v b/test_regress/t/t_virtual_interface_only_with_assignw.v new file mode 100644 index 000000000..9879143d5 --- /dev/null +++ b/test_regress/t/t_virtual_interface_only_with_assignw.v @@ -0,0 +1,55 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2026 by Antmicro. +// SPDX-License-Identifier: CC0-1.0 + +interface sys_if; + logic clk; +endinterface + +interface axi_if; + wire clk; +endinterface + +class sys_config; + virtual sys_if sys_vi; +endclass + +class axi_agent_config; + virtual axi_if axi_vi; + sys_config cfg; + task start_clk(); + fork + forever begin + cfg.sys_vi.clk = 1'b1; + #1; + end + join_none + @(posedge axi_vi.clk); + endtask + task test(); + cfg.sys_vi.clk = 0; + #1; + start_clk(); + $write("*-* All Finished *-*\n"); + $finish; + endtask +endclass + +module axi_tb_top; + sys_if sys_vi(); + axi_if axi_vi(); + assign axi_vi.clk = sys_vi.clk; + sys_config a; + axi_agent_config b; + initial begin + a = new; + b = new; + a.sys_vi = sys_vi; + b.axi_vi = axi_vi; + b.cfg = a; + b.test(); + #3 $stop; + end +endmodule