V3Timing: pre-clear destructive event state before dynamic waits (#7340)

When lowering dynamic event controls, destructive pre-clear updates were inserted with addHereThisAsNext() on the original node and then that node was replaced. This could leave leaked/orphaned nodes under leak-checking runs.

Build an explicit replacement stmt chain instead:
  [pre-clear stmts] -> trigger loop -> awaitResumption,
and replace the original control with the chain head. Keep the loop-only path unchanged when no destructive pre-clear is needed.
This commit is contained in:
Nick Brereton 2026-05-12 14:01:37 -04:00 committed by GitHub
parent 67b21e4c62
commit 9588e67ca9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 89 additions and 19 deletions

View File

@ -34,6 +34,9 @@ public:
std::vector<AstNodeStmt*> m_inits; // Initialization statements for previous values
std::vector<AstNodeStmt*> m_preUpdates; // Pre update assignments
std::vector<AstNodeStmt*> m_postUpdates; // Post update assignments
// Aliases of destructive post updates (for example event clearFired) that may need
// to run once before entering a fresh dynamic wait loop.
std::vector<AstNodeStmt*> m_destructivePostUpdates;
std::vector<AstVar*> 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);
// Get 'fired' state
AstCMethodHard* const callp

View File

@ -639,15 +639,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()) {
@ -1085,7 +1076,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{
@ -1105,10 +1095,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
@ -1129,7 +1119,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);
@ -1140,8 +1130,20 @@ class TimingControlVisitor final : public VNVisitor {
AstCAwait* const awaitResumep = awaitEvalp->cloneTree(false);
VN_AS(awaitResumep->exprp(), CMethodHard)->method(VCMethod::SCHED_RESUMPTION);
AstNode::addNext<AstNodeStmt, AstNodeStmt>(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->cloneTree(false));
}
}
chainp = AstNode::addNextNull(chainp, loopp);
nodep->replaceWith(chainp);
} else {
auto* const sentreep = m_finder.getSenTree(nodep->sentreep());
nodep->sentreep()->unlinkFrBack()->deleteTree();

View File

@ -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'])
test.execute()
test.passes()

View File

@ -0,0 +1,45 @@
// DESCRIPTION: Verilator: dynamic wait on stale class event
//
// This file ONLY is placed under the Creative Commons Public Domain.
// SPDX-FileCopyrightText: 2026 Wilson Snyder
// SPDX-License-Identifier: CC0-1.0
class EventHolder;
event ev;
time t_wait = '0;
task wait_once;
@ev;
t_wait = $time;
endtask
endclass
module t;
EventHolder h;
initial begin
h = new;
// Leave the event in the fired state before a class-method event control
// starts. Dynamic waits must pre-clear this stale state before evaluating.
->h.ev;
#10;
fork
begin
#10 ->h.ev;
end
begin
h.wait_once;
end
join
if (h.t_wait != 20) begin
$display("%%Error: wait time=%0d expected=20", h.t_wait);
$stop;
end
$write("*-* All Finished *-*\n");
$finish;
end
endmodule