From dfb3a2a3edae30c5dde6ee8bafa597962c79004b Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Mon, 23 Feb 2026 01:49:59 +0100 Subject: [PATCH 01/10] Sched: Conditionally fire VIF triggers only on value change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Virtual interface triggers were previously set unconditionally to 1 on every write to a VIF signal, even when the value was not changing. This caused infinite ICO/NBA convergence loops when combinational logic continuously writes the same value to a VIF signal — a common pattern when using continuous assign statements that propagate both request and response signals across an interface boundary. Fix: when the VIF write is the LHS of an assignment node, read the old value before the write and compare it to the RHS (new value). The trigger is only set if the value actually changes: trigger = trigger | (old_vif_value != new_value_expr) If the parent node is not a recognized assignment (e.g. the VIF signal is written via some other mechanism), fall back to the original unconditional trigger behavior to preserve correctness. Co-Authored-By: Claude Sonnet 4.6 --- src/V3SchedVirtIface.cpp | 55 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 5 deletions(-) diff --git a/src/V3SchedVirtIface.cpp b/src/V3SchedVirtIface.cpp index 7d19d7c7a..218ae3497 100644 --- a/src/V3SchedVirtIface.cpp +++ b/src/V3SchedVirtIface.cpp @@ -109,13 +109,58 @@ private: if (ifacep && memberVarp) { FileLine* const flp = nodep->fileline(); + + // Try to find the RHS of the parent assignment so we can make the trigger + // conditional on the value actually changing. This avoids spurious ICO/NBA + // re-evaluations when combinational logic unconditionally writes the same value + // to a virtual interface signal (e.g. via continuous assign statements). + AstNodeExpr* oldValReadp = nullptr; + AstNodeExpr* newValExprp = nullptr; + if (const AstNodeAssign* const parentAssignp + = VN_CAST(nodep->backp(), NodeAssign)) { + if (parentAssignp->lhsp() == static_cast(nodep)) { + // Clone nodep as a READ to capture the old value before the write. + // T is AstVarRef* or AstMemberSel*, both have access() setters. + T const clonedNodep = static_cast(nodep->cloneTree(false)); + clonedNodep->access(VAccess::READ); + oldValReadp = clonedNodep; + // Clone the RHS as new value expression + AstNode* const clonedRhs = parentAssignp->rhsp()->cloneTree(false); + newValExprp = VN_CAST(clonedRhs, NodeExpr); + if (!newValExprp) VL_DO_DANGLING(clonedRhs->deleteTree(), clonedRhs); + } + } + VNRelinker relinker; nodep->unlinkFrBack(&relinker); - relinker.relink(new AstExprStmt{ - flp, - new AstAssign{flp, createVirtIfaceMemberTriggerRefp(flp, ifacep, memberVarp), - new AstConst{flp, AstConst::BitTrue{}}}, - nodep}); + + // Create the trigger write ref first (this ensures the trigger varscope exists) + AstVarRef* const trigWriteRefp + = createVirtIfaceMemberTriggerRefp(flp, ifacep, memberVarp); + // Now get the trigger varscope for the read ref (guaranteed to exist now) + AstVarScope* const trigVscp = m_triggers.findMemberTrigger(ifacep, memberVarp); + UASSERT(trigVscp, "Trigger varscope should exist after createVirtIfaceMemberTriggerRefp"); + + AstNodeStmt* triggerStmtp = nullptr; + if (oldValReadp && newValExprp) { + // Conditional trigger: only fire if the value is actually changing. + // Generated: trigger = trigger | (old_vif_read != new_value_expr) + // This avoids infinite ICO/NBA convergence loops when combinational logic + // repeatedly writes the same value to a virtual interface signal. + AstVarRef* const trigReadp = new AstVarRef{flp, trigVscp, VAccess::READ}; + AstNodeExpr* const changedExprp = new AstNeq{flp, oldValReadp, newValExprp}; + AstNodeExpr* const triggerValp + = new AstOr{flp, trigReadp, changedExprp}; + triggerStmtp = new AstAssign{flp, trigWriteRefp, triggerValp}; + } else { + // Fall back to unconditional trigger if we can't determine the new value + if (oldValReadp) VL_DO_DANGLING(oldValReadp->deleteTree(), oldValReadp); + if (newValExprp) VL_DO_DANGLING(newValExprp->deleteTree(), newValExprp); + triggerStmtp = new AstAssign{flp, trigWriteRefp, + new AstConst{flp, AstConst::BitTrue{}}}; + } + + relinker.relink(new AstExprStmt{flp, triggerStmtp, nodep}); } } From dd7d8dbd93812ea71ba6ee8a676964996c50bf8c Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Mon, 23 Feb 2026 13:03:53 +0100 Subject: [PATCH 02/10] Tests: Add regression test for VIF trigger convergence loop Add t_interface_virtual_sched_ico_loop which reproduces the infinite NBA convergence loop caused by unconditional VIF trigger firing. A driver interface accessed via virtual interface in a class, connected to a DUT interface via bidirectional continuous assigns, with combinational response logic writing back through the VIF path causes a loop. Without the fix in dfb3a2a3e ("Sched: Conditionally fire VIF triggers only on value change"), this test fails with DIDNOTCONVERGE. Co-Authored-By: Claude Opus 4.6 --- .../t/t_interface_virtual_sched_ico_loop.py | 18 +++ .../t/t_interface_virtual_sched_ico_loop.v | 120 ++++++++++++++++++ 2 files changed, 138 insertions(+) create mode 100644 test_regress/t/t_interface_virtual_sched_ico_loop.py create mode 100644 test_regress/t/t_interface_virtual_sched_ico_loop.v diff --git a/test_regress/t/t_interface_virtual_sched_ico_loop.py b/test_regress/t/t_interface_virtual_sched_ico_loop.py new file mode 100644 index 000000000..3c44ca40f --- /dev/null +++ b/test_regress/t/t_interface_virtual_sched_ico_loop.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 Michael Rogenmoser +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('simulator') + +test.compile(verilator_flags2=["--binary", "--timing", "-Wno-UNOPTFLAT"]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_interface_virtual_sched_ico_loop.v b/test_regress/t/t_interface_virtual_sched_ico_loop.v new file mode 100644 index 000000000..0648d69e2 --- /dev/null +++ b/test_regress/t/t_interface_virtual_sched_ico_loop.v @@ -0,0 +1,120 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// Regression test for virtual interface trigger convergence. +// When combinational logic continuously writes the same value to a virtual +// interface signal (via continuous assign), the VIF trigger must only fire +// on actual value changes. Otherwise, the ICO/NBA scheduling loop never +// converges. +// +// This reproduces the pattern from the AXI bus_compare testbench: +// 1. A DV (driver) interface instance is accessed via virtual interface +// in a class (drv_if) +// 2. A plain interface instance (dut_if) is connected to drv_if via +// continuous assigns in both directions (request & response) +// 3. Combinational logic (always_comb) reads from dut_if members (extracted +// into struct-like variables) and writes response variables back +// 4. Response variables are assigned back to dut_if, which flow to drv_if +// 5. Writing to drv_if fires VIF triggers unconditionally, causing +// re-evaluation of all VIF-dependent logic in the ICO loop +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Michael Rogenmoser +// SPDX-License-Identifier: CC0-1.0 + +// Simple bus interface with request and response signals +interface BusIf #( + parameter int DW = 8 +) ( + input logic clk +); + logic req_valid; + logic req_ready; + logic [DW-1:0] req_data; +endinterface + +// Driver class that holds a virtual interface reference. +// This is what makes drv_if a "virtual interface" in Verilator's eyes. +class Driver #( + parameter int DW = 8 +); + virtual BusIf #(.DW(DW)) vif; + + function new(virtual BusIf #(.DW(DW)) vif); + this.vif = vif; + endfunction + + task reset(); + vif.req_valid <= 1'b0; + vif.req_data <= '0; + endtask + + task send(input logic [DW-1:0] data); + vif.req_valid <= 1'b1; + vif.req_data <= data; + @(posedge vif.clk); + while (!vif.req_ready) @(posedge vif.clk); + vif.req_valid <= 1'b0; + endtask +endclass + +module t; + + logic clk = 0; + int cyc = 0; + + // Driver interface (like AXI_BUS_DV) -- used via virtual interface in class + BusIf #(.DW(8)) drv_if (.clk(clk)); + + // DUT interface (like AXI_BUS) -- plain interface + BusIf #(.DW(8)) dut_if (.clk(clk)); + + // Instantiate driver class with virtual interface handle + Driver #(.DW(8)) drv = new(drv_if); + + // --- Bidirectional continuous assigns (like `AXI_ASSIGN(dut_if, drv_if)) --- + // Request direction: drv_if -> dut_if + assign dut_if.req_valid = drv_if.req_valid; + assign dut_if.req_data = drv_if.req_data; + // Response direction: dut_if -> drv_if (WRITES TO VIF!) + assign drv_if.req_ready = dut_if.req_ready; + + // --- Extract signals from dut_if (like AXI_ASSIGN_TO_REQ) --- + logic ext_valid; + logic [7:0] ext_data; + assign ext_valid = dut_if.req_valid; + assign ext_data = dut_if.req_data; + + // --- Combinational response logic (like always_comb in tb_axi_bus_compare) --- + logic rsp_ready; + always_comb begin + rsp_ready = 1'b1; // Always accept + end + + // --- Write response back to dut_if (like AXI_ASSIGN_FROM_RESP) --- + assign dut_if.req_ready = rsp_ready; + + // --- Testbench stimulus using the driver class --- + initial begin + drv.reset(); + @(posedge clk); + @(posedge clk); + // These writes to drv_if (VIF) trigger VIF triggers. The response + // path writes back to drv_if.req_ready with the same value (1'b1), + // which should NOT re-trigger infinitely. + drv.send(8'hAB); + @(posedge clk); + // Send same value again + drv.send(8'hAB); + @(posedge clk); + drv.send(8'h00); + @(posedge clk); + @(posedge clk); + $write("*-* All Finished *-*\n"); + $finish; + end + + initial begin + repeat (30) #5ns clk = ~clk; + end + +endmodule From 420d36d57eed4eefa741556a02a990aceada3fd2 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Mon, 23 Feb 2026 14:03:01 +0100 Subject: [PATCH 03/10] Sched: Simplify VIF conditional trigger code Remove unnecessary static_casts (cloneTree returns same type), redundant VN_CAST on RHS (always NodeExpr), and dead cleanup code. Co-Authored-By: Claude Opus 4.6 --- src/V3SchedVirtIface.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/V3SchedVirtIface.cpp b/src/V3SchedVirtIface.cpp index 218ae3497..0fd2ce11b 100644 --- a/src/V3SchedVirtIface.cpp +++ b/src/V3SchedVirtIface.cpp @@ -118,16 +118,13 @@ private: AstNodeExpr* newValExprp = nullptr; if (const AstNodeAssign* const parentAssignp = VN_CAST(nodep->backp(), NodeAssign)) { - if (parentAssignp->lhsp() == static_cast(nodep)) { + if (parentAssignp->lhsp() == nodep) { // Clone nodep as a READ to capture the old value before the write. - // T is AstVarRef* or AstMemberSel*, both have access() setters. - T const clonedNodep = static_cast(nodep->cloneTree(false)); + T const clonedNodep = nodep->cloneTree(false); clonedNodep->access(VAccess::READ); oldValReadp = clonedNodep; // Clone the RHS as new value expression - AstNode* const clonedRhs = parentAssignp->rhsp()->cloneTree(false); - newValExprp = VN_CAST(clonedRhs, NodeExpr); - if (!newValExprp) VL_DO_DANGLING(clonedRhs->deleteTree(), clonedRhs); + newValExprp = parentAssignp->rhsp()->cloneTree(false); } } @@ -154,8 +151,6 @@ private: triggerStmtp = new AstAssign{flp, trigWriteRefp, triggerValp}; } else { // Fall back to unconditional trigger if we can't determine the new value - if (oldValReadp) VL_DO_DANGLING(oldValReadp->deleteTree(), oldValReadp); - if (newValExprp) VL_DO_DANGLING(newValExprp->deleteTree(), newValExprp); triggerStmtp = new AstAssign{flp, trigWriteRefp, new AstConst{flp, AstConst::BitTrue{}}}; } From bff747c61c724ed9ed2fe8eb113b79d8e9e177ff Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Mon, 23 Feb 2026 14:03:45 +0100 Subject: [PATCH 04/10] Update contributor list --- docs/CONTRIBUTORS | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/CONTRIBUTORS b/docs/CONTRIBUTORS index 34b80c77f..39a74dcee 100644 --- a/docs/CONTRIBUTORS +++ b/docs/CONTRIBUTORS @@ -176,6 +176,7 @@ Max Wipfli Michael Bedford Taylor Michael Bikovitsky Michael Killough +Michael Rogenmoser Michal Czyz Michaël Lefebvre Mike Popoloski From 59d443e6bab70f45684e4613ca65ebb7985dcce7 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Mon, 23 Feb 2026 14:47:54 +0100 Subject: [PATCH 05/10] Minor fix --- test_regress/t/t_interface_virtual_sched_ico_loop.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_regress/t/t_interface_virtual_sched_ico_loop.py b/test_regress/t/t_interface_virtual_sched_ico_loop.py index 3c44ca40f..b495c8802 100644 --- a/test_regress/t/t_interface_virtual_sched_ico_loop.py +++ b/test_regress/t/t_interface_virtual_sched_ico_loop.py @@ -11,7 +11,7 @@ import vltest_bootstrap test.scenarios('simulator') -test.compile(verilator_flags2=["--binary", "--timing", "-Wno-UNOPTFLAT"]) +test.compile(verilator_flags2=["--binary", "-Wno-UNOPTFLAT"]) test.execute() From 174b63a3b74762daf98cd09dd1bb1b79a6cdf169 Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Mon, 23 Feb 2026 15:20:08 +0100 Subject: [PATCH 06/10] Sched: Restrict conditional VIF trigger to continuous assigns The conditional trigger optimization (comparing old vs new value) was being applied to all assignment types, causing circular ordering dependencies for procedural assigns. Restrict it to AstAssignW only, which is the only context where infinite ICO convergence loops occur. Also remove the trigger self-read (trigger | ...) to avoid self-loops in the ordering graph. Co-Authored-By: Claude Opus 4.6 --- src/V3SchedVirtIface.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/V3SchedVirtIface.cpp b/src/V3SchedVirtIface.cpp index 0fd2ce11b..8cbf4c0dc 100644 --- a/src/V3SchedVirtIface.cpp +++ b/src/V3SchedVirtIface.cpp @@ -118,7 +118,13 @@ private: AstNodeExpr* newValExprp = nullptr; if (const AstNodeAssign* const parentAssignp = VN_CAST(nodep->backp(), NodeAssign)) { - if (parentAssignp->lhsp() == nodep) { + // Only apply conditional trigger for continuous assigns (AstAssignW). + // These are the only context where repeatedly writing the same value + // causes infinite ICO convergence loops. Procedural assigns (blocking + // and non-blocking) are clocked and don't have this problem; applying + // the conditional trigger to them introduces circular ordering + // dependencies that the scheduler cannot resolve. + if (VN_IS(parentAssignp, AssignW) && parentAssignp->lhsp() == nodep) { // Clone nodep as a READ to capture the old value before the write. T const clonedNodep = nodep->cloneTree(false); clonedNodep->access(VAccess::READ); @@ -131,24 +137,18 @@ private: VNRelinker relinker; nodep->unlinkFrBack(&relinker); - // Create the trigger write ref first (this ensures the trigger varscope exists) + // Create the trigger write ref (this also ensures the trigger varscope exists) AstVarRef* const trigWriteRefp = createVirtIfaceMemberTriggerRefp(flp, ifacep, memberVarp); - // Now get the trigger varscope for the read ref (guaranteed to exist now) - AstVarScope* const trigVscp = m_triggers.findMemberTrigger(ifacep, memberVarp); - UASSERT(trigVscp, "Trigger varscope should exist after createVirtIfaceMemberTriggerRefp"); AstNodeStmt* triggerStmtp = nullptr; if (oldValReadp && newValExprp) { // Conditional trigger: only fire if the value is actually changing. - // Generated: trigger = trigger | (old_vif_read != new_value_expr) + // Generated: trigger = (old_vif_read != new_value_expr) // This avoids infinite ICO/NBA convergence loops when combinational logic // repeatedly writes the same value to a virtual interface signal. - AstVarRef* const trigReadp = new AstVarRef{flp, trigVscp, VAccess::READ}; AstNodeExpr* const changedExprp = new AstNeq{flp, oldValReadp, newValExprp}; - AstNodeExpr* const triggerValp - = new AstOr{flp, trigReadp, changedExprp}; - triggerStmtp = new AstAssign{flp, trigWriteRefp, triggerValp}; + triggerStmtp = new AstAssign{flp, trigWriteRefp, changedExprp}; } else { // Fall back to unconditional trigger if we can't determine the new value triggerStmtp = new AstAssign{flp, trigWriteRefp, From 50983e0727e681af289a26b6a09ba327c576a06f Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Mon, 23 Feb 2026 19:08:25 +0100 Subject: [PATCH 07/10] Sched: Use gated triggers to prevent VIF convergence loops Replace the conditional VIF trigger approach (which compared old vs new values) with a gated trigger mechanism. Each VIF member trigger now has an "already fired" gate variable that prevents the trigger from re-firing on subsequent convergence iterations within the same eval call. The gate flags are cleared at the start of each eval. This avoids the infinite convergence loops without introducing circular ordering dependencies that the conditional approach caused for procedural assigns. Co-Authored-By: Claude Opus 4.6 --- src/V3Sched.cpp | 62 +++++++++++++++++++++++++++++----------- src/V3Sched.h | 6 ++++ src/V3SchedTrigger.cpp | 28 ++++++++++++++++++ src/V3SchedVirtIface.cpp | 50 ++++---------------------------- 4 files changed, 84 insertions(+), 62 deletions(-) diff --git a/src/V3Sched.cpp b/src/V3Sched.cpp index bef68595f..d2fd08832 100644 --- a/src/V3Sched.cpp +++ b/src/V3Sched.cpp @@ -396,13 +396,23 @@ void createFinal(AstNetlist* netlistp, const LogicClasses& logicClasses) { //============================================================================ // Helper that creates virtual interface trigger resets -void addVirtIfaceTriggerAssignments(const VirtIfaceTriggers& virtIfaceTriggers, - uint32_t vifTriggerIndex, uint32_t vifMemberTriggerIndex, - const TriggerKit& trigKit) { +std::vector addVirtIfaceTriggerAssignments( + AstNetlist* netlistp, const VirtIfaceTriggers& virtIfaceTriggers, uint32_t vifTriggerIndex, + uint32_t vifMemberTriggerIndex, const TriggerKit& trigKit, const std::string& tag) { + AstScope* const scopeTopp = netlistp->topScopep()->scopep(); + std::vector gateVscps; for (const auto& p : virtIfaceTriggers.m_memberTriggers) { - trigKit.addExtraTriggerAssignment(p.second, vifMemberTriggerIndex); + // Create an "already fired" gate variable for this VIF trigger. + // This prevents the same trigger from re-firing on subsequent convergence + // iterations when the VIF member is written with the same value. + const std::string gateName = "__Vvif_" + tag + "_fired_" + + p.first.m_ifacep->name() + "_" + p.first.m_memberp->name(); + AstVarScope* const gatep = scopeTopp->createTemp(gateName, 1); + gateVscps.push_back(gatep); + trigKit.addGatedExtraTriggerAssignment(p.second, vifMemberTriggerIndex, gatep); ++vifMemberTriggerIndex; } + return gateVscps; } // Order the combinational logic to create the settle loop @@ -471,11 +481,16 @@ void createSettle(AstNetlist* netlistp, AstCFunc* const initFuncp, SenExprBuilde //============================================================================ // Order the replicated combinational logic to create the 'ico' region -AstNode* createInputCombLoop(AstNetlist* netlistp, AstCFunc* const initFuncp, - SenExprBuilder& senExprBuilder, LogicByScope& logic, - const VirtIfaceTriggers& virtIfaceTriggers) { +struct IcoResult final { + AstNode* stmtsp = nullptr; + std::vector vifGateVscps; +}; + +IcoResult createInputCombLoop(AstNetlist* netlistp, AstCFunc* const initFuncp, + SenExprBuilder& senExprBuilder, LogicByScope& logic, + const VirtIfaceTriggers& virtIfaceTriggers) { // Nothing to do if no combinational logic is sensitive to top level inputs - if (logic.empty()) return nullptr; + if (logic.empty()) return {}; // SystemC only: Any top level inputs feeding a combinational logic must be marked, // so we can make them sc_sensitive @@ -516,8 +531,9 @@ AstNode* createInputCombLoop(AstNetlist* netlistp, AstCFunc* const initFuncp, if (dpiExportTriggerVscp) { trigKit.addExtraTriggerAssignment(dpiExportTriggerVscp, dpiExportTriggerIndex); } - addVirtIfaceTriggerAssignments(virtIfaceTriggers, firstVifTriggerIndex, - firstVifMemberTriggerIndex, trigKit); + const auto icoGateVscps = addVirtIfaceTriggerAssignments( + netlistp, virtIfaceTriggers, firstVifTriggerIndex, firstVifMemberTriggerIndex, trigKit, + "ico"); // Remap sensitivities remapSensitivities(logic, trigKit.mapVec()); @@ -574,7 +590,7 @@ AstNode* createInputCombLoop(AstNetlist* netlistp, AstCFunc* const initFuncp, // Add the first iteration trigger to the trigger computation function trigKit.addExtraTriggerAssignment(icoLoop.firstIterp, firstIterationTrigger, false); - return icoLoop.stmtsp; + return {icoLoop.stmtsp, icoGateVscps}; } //============================================================================ @@ -600,7 +616,9 @@ void createEval(AstNetlist* netlistp, // const EvalKit& obsKit, // const EvalKit& reactKit, // AstCFunc* postponedFuncp, // - TimingKit& timingKit // + TimingKit& timingKit, // + const std::vector& icoGateVscps, // + const std::vector& actGateVscps // ) { FileLine* const flp = netlistp->fileline(); @@ -795,6 +813,14 @@ void createEval(AstNetlist* netlistp, // if (v3Global.opt.profExec()) funcp->addStmtsp(AstCStmt::profExecSectionPush(flp, "eval")); + // Clear the VIF trigger gate flags so triggers can fire in this eval + for (AstVarScope* const gatep : icoGateVscps) { + funcp->addStmtsp(util::setVar(gatep, 0)); + } + for (AstVarScope* const gatep : actGateVscps) { + funcp->addStmtsp(util::setVar(gatep, 0)); + } + // Start with the ico loop, if any if (icoLoop) funcp->addStmtsp(icoLoop); @@ -926,8 +952,9 @@ void schedule(AstNetlist* netlistp) { } // Step 7: Create input combinational logic loop - AstNode* const icoLoopp = createInputCombLoop(netlistp, staticp, senExprBuilder, - logicReplicas.m_ico, virtIfaceTriggers); + const IcoResult icoResult = createInputCombLoop(netlistp, staticp, senExprBuilder, + logicReplicas.m_ico, virtIfaceTriggers); + AstNode* const icoLoopp = icoResult.stmtsp; if (v3Global.opt.stats()) V3Stats::statsStage("sched-create-ico"); // Step 8: Create the triggers @@ -963,8 +990,9 @@ void schedule(AstNetlist* netlistp) { if (dpiExportTriggerVscp) { trigKit.addExtraTriggerAssignment(dpiExportTriggerVscp, dpiExportTriggerIndex); } - addVirtIfaceTriggerAssignments(virtIfaceTriggers, firstVifTriggerIndex, - firstVifMemberTriggerIndex, trigKit); + const auto actGateVscps = addVirtIfaceTriggerAssignments( + netlistp, virtIfaceTriggers, firstVifTriggerIndex, firstVifMemberTriggerIndex, trigKit, + "act"); if (v3Global.opt.stats()) V3Stats::statsStage("sched-create-triggers"); // Note: Experiments so far show that running the Act (or Ico) regions on @@ -1081,7 +1109,7 @@ void schedule(AstNetlist* netlistp) { // Step 14: Bolt it all together to create the '_eval' function createEval(netlistp, icoLoopp, trigKit, actKit, nbaKit, obsKit, reactKit, postponedFuncp, - timingKit); + timingKit, icoResult.vifGateVscps, actGateVscps); // Step 15: Add neccessary evaluation before awaits if (AstCCall* const readyp = timingKit.createReady(netlistp)) { diff --git a/src/V3Sched.h b/src/V3Sched.h index 870d6d4a9..e5dc2930e 100644 --- a/src/V3Sched.h +++ b/src/V3Sched.h @@ -312,6 +312,12 @@ public: // Set then extra trigger bit at 'index' to the value of 'vscp', then set 'vscp' to 0 void addExtraTriggerAssignment(AstVarScope* vscp, uint32_t index, bool clear = true) const; + + // Like addExtraTriggerAssignment, but gates the trigger bit on '!gatep'. + // Sets: trigBit = vscp & ~gatep; gatep = gatep | vscp; vscp = 0; + // This prevents the same trigger from firing more than once per convergence loop. + void addGatedExtraTriggerAssignment(AstVarScope* vscp, uint32_t index, + AstVarScope* gatep) const; }; // Everything needed for combining timing with static scheduling. diff --git a/src/V3SchedTrigger.cpp b/src/V3SchedTrigger.cpp index 428f77457..13c84c5b8 100644 --- a/src/V3SchedTrigger.cpp +++ b/src/V3SchedTrigger.cpp @@ -426,6 +426,34 @@ void TriggerKit::addExtraTriggerAssignment(AstVarScope* vscp, uint32_t index, bo m_compVecp->addStmtsp(setp); } +void TriggerKit::addGatedExtraTriggerAssignment(AstVarScope* vscp, uint32_t index, + AstVarScope* gatep) const { + index += m_nSenseWords * WORD_SIZE; + const uint32_t wordIndex = index / WORD_SIZE; + const uint32_t bitIndex = index % WORD_SIZE; + FileLine* const flp = vscp->fileline(); + // Set the trigger bit, gated by the 'already fired' flag: + // trigBit = vscp & ~gatep + AstVarRef* const refp = new AstVarRef{flp, m_vscp, VAccess::WRITE}; + AstNodeExpr* const wordp = new AstArraySel{flp, refp, static_cast(wordIndex)}; + AstNodeExpr* const trigLhsp = new AstSel{flp, wordp, static_cast(bitIndex), 1}; + AstNodeExpr* const vscpReadp = new AstVarRef{flp, vscp, VAccess::READ}; + AstNodeExpr* const gateReadp = new AstVarRef{flp, gatep, VAccess::READ}; + AstNodeExpr* const trigRhsp = new AstAnd{flp, vscpReadp, new AstNot{flp, gateReadp}}; + AstNode* const setp = new AstAssign{flp, trigLhsp, trigRhsp}; + // Update the gate: gatep = gatep | vscp + setp->addNext(new AstAssign{flp, new AstVarRef{flp, gatep, VAccess::WRITE}, + new AstOr{flp, new AstVarRef{flp, gatep, VAccess::READ}, + new AstVarRef{flp, vscp, VAccess::READ}}}); + // Clear the input variable + setp->addNext(new AstAssign{flp, new AstVarRef{flp, vscp, VAccess::WRITE}, + new AstConst{flp, AstConst::BitFalse{}}}); + if (AstNode* const nodep = m_compVecp->stmtsp()) { + setp->addNext(setp, nodep->unlinkFrBackWithNext()); + } + m_compVecp->addStmtsp(setp); +} + TriggerKit::TriggerKit(const std::string& name, bool slow, uint32_t nSenseWords, uint32_t nExtraWords, uint32_t nPreWords, std::unordered_map, size_t> senItem2TrigIdx, diff --git a/src/V3SchedVirtIface.cpp b/src/V3SchedVirtIface.cpp index 8cbf4c0dc..7d19d7c7a 100644 --- a/src/V3SchedVirtIface.cpp +++ b/src/V3SchedVirtIface.cpp @@ -109,53 +109,13 @@ private: if (ifacep && memberVarp) { FileLine* const flp = nodep->fileline(); - - // Try to find the RHS of the parent assignment so we can make the trigger - // conditional on the value actually changing. This avoids spurious ICO/NBA - // re-evaluations when combinational logic unconditionally writes the same value - // to a virtual interface signal (e.g. via continuous assign statements). - AstNodeExpr* oldValReadp = nullptr; - AstNodeExpr* newValExprp = nullptr; - if (const AstNodeAssign* const parentAssignp - = VN_CAST(nodep->backp(), NodeAssign)) { - // Only apply conditional trigger for continuous assigns (AstAssignW). - // These are the only context where repeatedly writing the same value - // causes infinite ICO convergence loops. Procedural assigns (blocking - // and non-blocking) are clocked and don't have this problem; applying - // the conditional trigger to them introduces circular ordering - // dependencies that the scheduler cannot resolve. - if (VN_IS(parentAssignp, AssignW) && parentAssignp->lhsp() == nodep) { - // Clone nodep as a READ to capture the old value before the write. - T const clonedNodep = nodep->cloneTree(false); - clonedNodep->access(VAccess::READ); - oldValReadp = clonedNodep; - // Clone the RHS as new value expression - newValExprp = parentAssignp->rhsp()->cloneTree(false); - } - } - VNRelinker relinker; nodep->unlinkFrBack(&relinker); - - // Create the trigger write ref (this also ensures the trigger varscope exists) - AstVarRef* const trigWriteRefp - = createVirtIfaceMemberTriggerRefp(flp, ifacep, memberVarp); - - AstNodeStmt* triggerStmtp = nullptr; - if (oldValReadp && newValExprp) { - // Conditional trigger: only fire if the value is actually changing. - // Generated: trigger = (old_vif_read != new_value_expr) - // This avoids infinite ICO/NBA convergence loops when combinational logic - // repeatedly writes the same value to a virtual interface signal. - AstNodeExpr* const changedExprp = new AstNeq{flp, oldValReadp, newValExprp}; - triggerStmtp = new AstAssign{flp, trigWriteRefp, changedExprp}; - } else { - // Fall back to unconditional trigger if we can't determine the new value - triggerStmtp = new AstAssign{flp, trigWriteRefp, - new AstConst{flp, AstConst::BitTrue{}}}; - } - - relinker.relink(new AstExprStmt{flp, triggerStmtp, nodep}); + relinker.relink(new AstExprStmt{ + flp, + new AstAssign{flp, createVirtIfaceMemberTriggerRefp(flp, ifacep, memberVarp), + new AstConst{flp, AstConst::BitTrue{}}}, + nodep}); } } From 3e401ac384a0b45105b7c1801da94c7321f19f3a Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Mon, 23 Feb 2026 19:10:54 +0100 Subject: [PATCH 08/10] Remove unneeded warning suppression --- test_regress/t/t_interface_virtual_sched_ico_loop.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test_regress/t/t_interface_virtual_sched_ico_loop.py b/test_regress/t/t_interface_virtual_sched_ico_loop.py index b495c8802..68e52200a 100644 --- a/test_regress/t/t_interface_virtual_sched_ico_loop.py +++ b/test_regress/t/t_interface_virtual_sched_ico_loop.py @@ -9,9 +9,9 @@ import vltest_bootstrap -test.scenarios('simulator') +test.scenarios("simulator") -test.compile(verilator_flags2=["--binary", "-Wno-UNOPTFLAT"]) +test.compile(verilator_flags2=["--binary"]) test.execute() From 9797979470b3769ab3aa1f9c5974e9d65151945c Mon Sep 17 00:00:00 2001 From: Michael Rogenmoser Date: Mon, 23 Feb 2026 23:45:00 +0100 Subject: [PATCH 09/10] Tests: Fix SPDX copyright in VIF trigger convergence test Co-Authored-By: Claude Opus 4.6 --- test_regress/t/t_interface_virtual_sched_ico_loop.py | 2 +- test_regress/t/t_interface_virtual_sched_ico_loop.v | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test_regress/t/t_interface_virtual_sched_ico_loop.py b/test_regress/t/t_interface_virtual_sched_ico_loop.py index 68e52200a..45dd9a141 100644 --- a/test_regress/t/t_interface_virtual_sched_ico_loop.py +++ b/test_regress/t/t_interface_virtual_sched_ico_loop.py @@ -4,7 +4,7 @@ # 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 Michael Rogenmoser +# SPDX-FileCopyrightText: 2026 Wilson Snyder # SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 import vltest_bootstrap diff --git a/test_regress/t/t_interface_virtual_sched_ico_loop.v b/test_regress/t/t_interface_virtual_sched_ico_loop.v index 0648d69e2..0994cb95d 100644 --- a/test_regress/t/t_interface_virtual_sched_ico_loop.v +++ b/test_regress/t/t_interface_virtual_sched_ico_loop.v @@ -18,7 +18,7 @@ // re-evaluation of all VIF-dependent logic in the ICO loop // // This file ONLY is placed under the Creative Commons Public Domain. -// SPDX-FileCopyrightText: 2026 Michael Rogenmoser +// SPDX-FileCopyrightText: 2026 Wilson Snyder // SPDX-License-Identifier: CC0-1.0 // Simple bus interface with request and response signals From 08579ac2c02652317f82e36241c87476af6a72dc Mon Sep 17 00:00:00 2001 From: github action Date: Mon, 23 Feb 2026 22:50:37 +0000 Subject: [PATCH 10/10] Apply 'make format' --- src/V3Sched.cpp | 31 +++++++++---------- src/V3SchedTrigger.cpp | 2 +- .../t/t_interface_virtual_sched_ico_loop.py | 0 3 files changed, 15 insertions(+), 18 deletions(-) mode change 100644 => 100755 test_regress/t/t_interface_virtual_sched_ico_loop.py diff --git a/src/V3Sched.cpp b/src/V3Sched.cpp index d2fd08832..098beece2 100644 --- a/src/V3Sched.cpp +++ b/src/V3Sched.cpp @@ -396,17 +396,18 @@ void createFinal(AstNetlist* netlistp, const LogicClasses& logicClasses) { //============================================================================ // Helper that creates virtual interface trigger resets -std::vector addVirtIfaceTriggerAssignments( - AstNetlist* netlistp, const VirtIfaceTriggers& virtIfaceTriggers, uint32_t vifTriggerIndex, - uint32_t vifMemberTriggerIndex, const TriggerKit& trigKit, const std::string& tag) { +std::vector +addVirtIfaceTriggerAssignments(AstNetlist* netlistp, const VirtIfaceTriggers& virtIfaceTriggers, + uint32_t vifTriggerIndex, uint32_t vifMemberTriggerIndex, + const TriggerKit& trigKit, const std::string& tag) { AstScope* const scopeTopp = netlistp->topScopep()->scopep(); std::vector gateVscps; for (const auto& p : virtIfaceTriggers.m_memberTriggers) { // Create an "already fired" gate variable for this VIF trigger. // This prevents the same trigger from re-firing on subsequent convergence // iterations when the VIF member is written with the same value. - const std::string gateName = "__Vvif_" + tag + "_fired_" - + p.first.m_ifacep->name() + "_" + p.first.m_memberp->name(); + const std::string gateName = "__Vvif_" + tag + "_fired_" + p.first.m_ifacep->name() + "_" + + p.first.m_memberp->name(); AstVarScope* const gatep = scopeTopp->createTemp(gateName, 1); gateVscps.push_back(gatep); trigKit.addGatedExtraTriggerAssignment(p.second, vifMemberTriggerIndex, gatep); @@ -531,9 +532,9 @@ IcoResult createInputCombLoop(AstNetlist* netlistp, AstCFunc* const initFuncp, if (dpiExportTriggerVscp) { trigKit.addExtraTriggerAssignment(dpiExportTriggerVscp, dpiExportTriggerIndex); } - const auto icoGateVscps = addVirtIfaceTriggerAssignments( - netlistp, virtIfaceTriggers, firstVifTriggerIndex, firstVifMemberTriggerIndex, trigKit, - "ico"); + const auto icoGateVscps + = addVirtIfaceTriggerAssignments(netlistp, virtIfaceTriggers, firstVifTriggerIndex, + firstVifMemberTriggerIndex, trigKit, "ico"); // Remap sensitivities remapSensitivities(logic, trigKit.mapVec()); @@ -814,12 +815,8 @@ void createEval(AstNetlist* netlistp, // if (v3Global.opt.profExec()) funcp->addStmtsp(AstCStmt::profExecSectionPush(flp, "eval")); // Clear the VIF trigger gate flags so triggers can fire in this eval - for (AstVarScope* const gatep : icoGateVscps) { - funcp->addStmtsp(util::setVar(gatep, 0)); - } - for (AstVarScope* const gatep : actGateVscps) { - funcp->addStmtsp(util::setVar(gatep, 0)); - } + for (AstVarScope* const gatep : icoGateVscps) { funcp->addStmtsp(util::setVar(gatep, 0)); } + for (AstVarScope* const gatep : actGateVscps) { funcp->addStmtsp(util::setVar(gatep, 0)); } // Start with the ico loop, if any if (icoLoop) funcp->addStmtsp(icoLoop); @@ -990,9 +987,9 @@ void schedule(AstNetlist* netlistp) { if (dpiExportTriggerVscp) { trigKit.addExtraTriggerAssignment(dpiExportTriggerVscp, dpiExportTriggerIndex); } - const auto actGateVscps = addVirtIfaceTriggerAssignments( - netlistp, virtIfaceTriggers, firstVifTriggerIndex, firstVifMemberTriggerIndex, trigKit, - "act"); + const auto actGateVscps + = addVirtIfaceTriggerAssignments(netlistp, virtIfaceTriggers, firstVifTriggerIndex, + firstVifMemberTriggerIndex, trigKit, "act"); if (v3Global.opt.stats()) V3Stats::statsStage("sched-create-triggers"); // Note: Experiments so far show that running the Act (or Ico) regions on diff --git a/src/V3SchedTrigger.cpp b/src/V3SchedTrigger.cpp index 13c84c5b8..2b4ad1219 100644 --- a/src/V3SchedTrigger.cpp +++ b/src/V3SchedTrigger.cpp @@ -427,7 +427,7 @@ void TriggerKit::addExtraTriggerAssignment(AstVarScope* vscp, uint32_t index, bo } void TriggerKit::addGatedExtraTriggerAssignment(AstVarScope* vscp, uint32_t index, - AstVarScope* gatep) const { + AstVarScope* gatep) const { index += m_nSenseWords * WORD_SIZE; const uint32_t wordIndex = index / WORD_SIZE; const uint32_t bitIndex = index % WORD_SIZE; diff --git a/test_regress/t/t_interface_virtual_sched_ico_loop.py b/test_regress/t/t_interface_virtual_sched_ico_loop.py old mode 100644 new mode 100755