diff --git a/src/V3SenExprBuilder.h b/src/V3SenExprBuilder.h index 4dd6634ff..cd088ac1a 100644 --- a/src/V3SenExprBuilder.h +++ b/src/V3SenExprBuilder.h @@ -34,6 +34,9 @@ public: std::vector m_inits; // Initialization statements for previous values std::vector m_preUpdates; // Pre update assignments std::vector m_postUpdates; // Post update assignments + // Destructive post updates (for example event clearFired) that may need + // to run once before entering a fresh dynamic wait loop. + std::vector m_destructivePostUpdates; std::vector m_vars; // Created temporary variables }; @@ -278,8 +281,10 @@ private: AstCMethodHard* const clearp = new AstCMethodHard{flp, currp(), VCMethod::EVENT_CLEAR_FIRED}; clearp->dtypeSetVoid(); - m_results.m_postUpdates.push_back( - wrapStmtWithNullCheck(flp, clearp->makeStmt(), baseClassRefp)); + AstNodeStmt* const clearStmtp + = wrapStmtWithNullCheck(flp, clearp->makeStmt(), baseClassRefp); + m_results.m_postUpdates.push_back(clearStmtp); + m_results.m_destructivePostUpdates.push_back(clearStmtp->cloneTree(false)); // Get 'fired' state AstCMethodHard* const callp diff --git a/src/V3Timing.cpp b/src/V3Timing.cpp index 060ac2348..6959085fa 100644 --- a/src/V3Timing.cpp +++ b/src/V3Timing.cpp @@ -625,15 +625,6 @@ class TimingControlVisitor final : public VNVisitor { return !nodep->isPure(); }); } - // Returns true if the given trigger expression needs a destructive post update after trigger - // evaluation. Currently this only applies to named events. - bool destructivePostUpdate(AstNode* const exprp) const { - return exprp->exists([](const AstNode* const nodep) { - const AstNodeDType* const dtypep = nodep->dtypep(); - const AstBasicDType* const basicp = dtypep ? dtypep->skipRefp()->basicp() : nullptr; - return basicp && basicp->isEvent(); - }); - } // Creates a trigger scheduler variable AstVarScope* getCreateTriggerSchedulerp(AstSenTree* const sentreep) { if (!sentreep->user1p()) { @@ -1065,7 +1056,6 @@ class TimingControlVisitor final : public VNVisitor { = createTemp(flp, m_dynTrigNames.get(nodep), nodep->findBitDType(), nodep); auto* const initp = new AstAssign{flp, new AstVarRef{flp, trigvscp, VAccess::WRITE}, new AstConst{flp, AstConst::BitFalse{}}}; - nodep->addHereThisAsNext(initp); // Await the eval step with the dynamic trigger scheduler. First, create the method // call auto* const evalMethodp = new AstCMethodHard{ @@ -1085,10 +1075,10 @@ class TimingControlVisitor final : public VNVisitor { // Get the SenExprBuilder results const SenExprBuilder::Results senResults = m_senExprBuilderp->getAndClearResults(); localizeVars(m_procp, senResults.m_vars); - // Put all and inits before the trigger eval loop - for (AstNodeStmt* const stmtp : senResults.m_inits) { - nodep->addHereThisAsNext(stmtp); - } + // If post updates are destructive (e.g. clearFired on events), perform a + // conservative pre-clear once before entering the wait loop so stale state from a + // previous wait does not cause an immediate false-positive trigger. + const bool hasDestructivePostUpdates = !senResults.m_destructivePostUpdates.empty(); // Create the trigger eval loop, which will await the evaluation step and check the // trigger AstNodeExpr* const condp @@ -1109,7 +1099,7 @@ class TimingControlVisitor final : public VNVisitor { loopp->addStmtsp(anyTriggeredMethodp->makeStmt()); // If the post update is destructive (e.g. event vars are cleared), create an await for // the post update step - if (destructivePostUpdate(sentreep)) { + if (hasDestructivePostUpdates) { AstCAwait* const awaitPostUpdatep = awaitEvalp->cloneTree(false); VN_AS(awaitPostUpdatep->exprp(), CMethodHard)->method(VCMethod::SCHED_POST_UPDATE); loopp->addStmtsp(awaitPostUpdatep); @@ -1120,8 +1110,20 @@ class TimingControlVisitor final : public VNVisitor { AstCAwait* const awaitResumep = awaitEvalp->cloneTree(false); VN_AS(awaitResumep->exprp(), CMethodHard)->method(VCMethod::SCHED_RESUMPTION); AstNode::addNext(loopp, awaitResumep); - // Replace the event control with the loop - nodep->replaceWith(loopp); + // Replace the event control with one explicit stmt chain: + // init -> [inits] -> [optional destructive pre-clears] -> loop -> awaitResumption + AstNodeStmt* chainp = nullptr; + chainp = AstNode::addNextNull(chainp, initp); + for (AstNodeStmt* const stmtp : senResults.m_inits) { + chainp = AstNode::addNextNull(chainp, stmtp); + } + if (hasDestructivePostUpdates) { + for (AstNodeStmt* const stmtp : senResults.m_destructivePostUpdates) { + chainp = AstNode::addNextNull(chainp, stmtp); + } + } + chainp = AstNode::addNextNull(chainp, loopp); + nodep->replaceWith(chainp); } else { auto* const sentreep = m_finder.getSenTree(nodep->sentreep()); nodep->sentreep()->unlinkFrBack()->deleteTree(); diff --git a/test_regress/t/t_clocking_event_readback.py b/test_regress/t/t_clocking_event_readback.py new file mode 100755 index 000000000..fa1e26284 --- /dev/null +++ b/test_regress/t/t_clocking_event_readback.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", "--timing", "--vpi", "--bbox-sys"]) + +test.execute(fails=True) + +test.passes() diff --git a/test_regress/t/t_clocking_event_readback.v b/test_regress/t/t_clocking_event_readback.v new file mode 100644 index 000000000..0d3d9426d --- /dev/null +++ b/test_regress/t/t_clocking_event_readback.v @@ -0,0 +1,180 @@ +// DESCRIPTION: Verilator: APB clocking readback regression +// +// 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 + +`timescale 1ns/1ns +// verilog_format: off +`define stop $stop +`define checkd(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0d exp=%0d (%s !== %s)\n", `__FILE__,`__LINE__, (gotv), (expv), `"gotv`", `"expv`"); `stop; end while(0); +// verilog_format: on + +interface apb_if(input bit pclk); + wire [31:0] paddr; + wire psel; + wire penable; + wire pwrite; + wire [31:0] prdata; + wire [31:0] pwdata; + + clocking mck @(posedge pclk); + output paddr, psel, penable, pwrite, pwdata; + input prdata; + endclocking + + clocking pck @(posedge pclk); + input paddr, psel, penable, pwrite, prdata, pwdata; + endclocking +endinterface + +module blk(apb_if apb, input bit rst); + logic [31:0] mode; + logic [31:0] rate; + logic [31:0] pr_data; + + assign apb.prdata = (apb.psel && apb.penable) ? pr_data : 'z; + + always_comb begin + pr_data = 32'h0; + case (apb.paddr[7:0]) + 8'h00: pr_data = mode; + 8'h04: pr_data = rate; + default: ; + endcase + end + + always_ff @(posedge apb.pclk) begin + if (rst) begin + mode <= 0; + rate <= 0; + end else if (apb.psel && apb.penable && apb.pwrite) begin + case (apb.paddr[7:0]) + 8'h00: mode <= apb.pwdata; + 8'h04: rate <= apb.pwdata; + default: ; + endcase + end + end +endmodule + +module soc(apb_if apb, input bit rst); + apb_if apb0(apb.pclk); + apb_if apb1(apb.pclk); + apb_if apb2(apb.pclk); + + blk blk0(apb0, rst); + blk blk1(apb1, rst); + blk blk2(apb2, rst); + + assign apb0.paddr = apb.paddr; + assign apb0.psel = ((apb.paddr[31:8] == 24'h000000) || (apb.paddr[31:8] == 24'h030000)); + assign apb0.penable = apb.penable; + assign apb0.pwrite = apb.pwrite; + assign apb0.pwdata = apb.pwdata; + assign apb.prdata = apb0.prdata; + + assign apb1.paddr = apb.paddr; + assign apb1.psel = ((apb.paddr[31:8] == 24'h010000) || (apb.paddr[31:8] == 24'h030000)); + assign apb1.penable = apb.penable; + assign apb1.pwrite = apb.pwrite; + assign apb1.pwdata = apb.pwdata; + assign apb.prdata = apb1.prdata; + + assign apb2.paddr = apb.paddr; + assign apb2.psel = ((apb.paddr[31:8] == 24'h020000) || (apb.paddr[31:8] == 24'h030000)); + assign apb2.penable = apb.penable; + assign apb2.pwrite = apb.pwrite; + assign apb2.pwdata = apb.pwdata; + assign apb.prdata = apb2.prdata; +endmodule + +class Master; + virtual apb_if vif; + + function new(virtual apb_if vif); + this.vif = vif; + endfunction + + task read(input logic [31:0] addr, output logic [31:0] data_mck); + vif.mck.paddr <= addr; + vif.mck.pwrite <= 1'b0; + vif.mck.psel <= 1'b1; + @(vif.mck); + vif.mck.penable <= 1'b1; + @(vif.mck); + data_mck = vif.mck.prdata; + $display("READ_SAMPLE t=%0t addr=%08x mck=%08x", $time, addr, data_mck); + vif.mck.psel <= 1'b0; + vif.mck.penable <= 1'b0; + endtask + + task write(input logic [31:0] addr, input logic [31:0] data); + vif.mck.paddr <= addr; + vif.mck.pwdata <= data; + vif.mck.pwrite <= 1'b1; + vif.mck.psel <= 1'b1; + @(vif.mck); + vif.mck.penable <= 1'b1; + @(vif.mck); + $display("WRITE_SAMPLE t=%0t addr=%08x data=%08x", $time, addr, data); + vif.mck.psel <= 1'b0; + vif.mck.penable <= 1'b0; + endtask +endclass + +module t; + bit clk = 0; + bit rst = 1; + apb_if apb(clk); + soc dut(apb, rst); + + Master m; + + always #10 clk = ~clk; + + initial begin + logic [31:0] d0, d1, d2; + logic [31:0] m0, m1, m2; + m = new(apb); + + repeat (2) @(posedge clk); + rst = 0; + + // First phase: direct mode initialization then three reads. + // This catches first-read timing regressions. + dut.blk0.mode = 32'h5; + dut.blk1.mode = 32'h5; + dut.blk2.mode = 32'h5; + @(posedge clk); + m.read(32'h0000_0000, d0); + m.read(32'h0100_0000, d1); + m.read(32'h0200_0000, d2); + $display("FIRST_RESULT d0=%08x d1=%08x d2=%08x", d0, d1, d2); + `checkd(d0, 32'h0000_0005); + `checkd(d1, 32'h0000_0005); + `checkd(d2, 32'h0000_0005); + + // Second phase: broadcast write through the clocking path and readback. + // This catches mck.prdata sampling regressions. + m.write(32'h0300_0000, 32'h5); + m.read(32'h0000_0000, m0); + m.read(32'h0100_0000, m1); + m.read(32'h0200_0000, m2); + $display("SECOND_RESULT m0=%08x m1=%08x m2=%08x", m0, m1, m2); + `checkd(m0, 32'h0000_0005); + `checkd(m1, 32'h0000_0005); + `checkd(m2, 32'h0000_0005); + + $write("*-* All Finished *-*\n"); + $finish; + end + + always @(apb.pck) begin + if (apb.pck.psel && apb.pck.penable && !apb.pck.pwrite) begin + $display("MON_READ t=%0t addr=%08x data=%08x", $time, apb.pck.paddr, apb.pck.prdata); + end + end +endmodule