diff --git a/docs/CONTRIBUTORS b/docs/CONTRIBUTORS index de64a9a88..3ed73c5b1 100644 --- a/docs/CONTRIBUTORS +++ b/docs/CONTRIBUTORS @@ -178,6 +178,7 @@ Max Wipfli Michael Bedford Taylor Michael Bikovitsky Michael Killough +Michael Rogenmoser Michal Czyz Michaël Lefebvre Mike Popoloski diff --git a/src/V3Sched.cpp b/src/V3Sched.cpp index 7795c51b5..8b80eb2f8 100644 --- a/src/V3Sched.cpp +++ b/src/V3Sched.cpp @@ -396,13 +396,24 @@ 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 +482,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 +532,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 +591,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 +617,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 +814,10 @@ 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); @@ -935,8 +958,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 @@ -972,8 +996,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 @@ -1090,7 +1115,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..2b4ad1219 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/test_regress/t/t_interface_virtual_sched_ico_loop.py b/test_regress/t/t_interface_virtual_sched_ico_loop.py new file mode 100755 index 000000000..45dd9a141 --- /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 Wilson Snyder +# 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_interface_virtual_sched_ico_loop.v b/test_regress/t/t_interface_virtual_sched_ico_loop.v new file mode 100644 index 000000000..0994cb95d --- /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 Wilson Snyder +// 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