Fix nested named fork disable propagation (Part 4 of #7857) (#8001)

This commit is contained in:
Marco Bartoli 2026-07-29 15:49:34 +02:00 committed by GitHub
parent cac2c3df13
commit c3be3c8050
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 282 additions and 39 deletions

View File

@ -68,6 +68,7 @@ class LinkJumpVisitor final : public VNVisitor {
"__VprocessQueue"}; // Names for queues needed for 'disable' handling
std::unordered_map<const AstTask*, AstVar*> m_taskDisableQueues; // Per-task process queues
std::unordered_map<const AstBegin*, AstVar*> m_beginDisableQueues; // Per-begin process queues
std::unordered_map<const AstFork*, AstVar*> m_forkDisableQueues; // Per-fork process queues
std::unordered_map<const AstTask*, AstBegin*>
m_taskDisableBegins; // Per-task process wrappers
std::unordered_map<const AstBegin*, AstBegin*>
@ -310,6 +311,21 @@ class LinkJumpVisitor final : public VNVisitor {
m_beginDisableQueues.emplace(beginp, processQueuep);
return processQueuep;
}
AstVar* getOrCreateForkDisableQueuep(AstFork* const forkp, FileLine* const fl) {
const auto it = m_forkDisableQueues.find(forkp);
if (it != m_forkDisableQueues.end()) return it->second;
AstVar* const processQueuep = getProcessQueuep(forkp, fl);
prependForkBranchQueuePushes(forkp, processQueuep, fl, true);
// Disabling a fork must also terminate detached descendants created by nested forks
// under each branch, so track nested fork branch processes in the same queue.
for (AstBegin* branchp = forkp->forksp(); branchp;
branchp = VN_AS(branchp->nextp(), Begin)) {
prependNestedForkBranchQueuePushes(branchp, processQueuep, fl, true);
}
m_forkDisableQueues.emplace(forkp, processQueuep);
return processQueuep;
}
AstStmtExpr* insertKillStmtp(AstDisable* const nodep, AstVar* const processQueuep) {
AstStmtExpr* const killStmtp = getQueueKillStmtp(nodep->fileline(), processQueuep);
nodep->addNextHere(killStmtp);
@ -319,13 +335,13 @@ class LinkJumpVisitor final : public VNVisitor {
AstJumpBlock* const jmpBlockp = getJumpBlock(targetp, false);
killStmtp->addNextHere(new AstJumpGo{killStmtp->fileline(), jmpBlockp});
}
void handleDisableOnFork(AstDisable* const nodep, const std::vector<AstBegin*>& forks) {
// The support utilizes the process::kill()` method. For each `disable` a queue of
// processes is declared. At the beginning of each fork that can be disabled, its process
// handle is pushed to the queue. `disable` statement is replaced with calling `kill()`
// method on each element of the queue.
void handleDisableOnFork(AstDisable* const nodep) {
// The support utilizes the process::kill()` method. For each disabled fork a queue of
// processes is declared. At the beginning of each fork branch (and each nested fork
// branch) its process handle is pushed to the queue. The `disable` statement is replaced
// with calling `kill()` on each element of the queue.
FileLine* const fl = nodep->fileline();
AstNode* const targetp = nodep->targetp();
AstFork* const targetp = VN_AS(nodep->targetp(), Fork);
if (m_ftaskp) {
if (!m_ftaskp->exists(
[targetp](const AstNodeBlock* blockp) -> bool { return blockp == targetp; })) {
@ -334,33 +350,18 @@ class LinkJumpVisitor final : public VNVisitor {
}
}
AstPackage* const topPkgp = v3Global.rootp()->dollarUnitPkgAddp();
AstVar* const processQueuep = getProcessQueuep(targetp, fl);
AstVarRef* const queueWriteRefp
= new AstVarRef{fl, topPkgp, processQueuep, VAccess::WRITE};
AstStmtExpr* pushCurrentProcessp = getQueuePushProcessSelfp(queueWriteRefp);
for (AstBegin* const beginp : forks) {
if (pushCurrentProcessp->backp()) {
pushCurrentProcessp = pushCurrentProcessp->cloneTree(false);
}
prependStmtsp(beginp, pushCurrentProcessp);
}
AstVar* const processQueuep = getOrCreateForkDisableQueuep(targetp, fl);
AstStmtExpr* const killStmtp = insertKillStmtp(nodep, processQueuep);
// 'process::kill' does not immediately kill the current process
// executing the disable statement (because it's in the running state).
// If the disable statement is indeed executed by a process under the
// target AstFork, then jump to the end of that fork branch.
if (VN_IS(targetp, Fork)) {
AstNodeBlock* forkBranchp = nullptr;
for (AstNodeBlock* const blockp : vlstd::reverse_view(m_blockStack)) {
if (blockp == targetp) {
addJumpAfterKill(killStmtp, VN_AS(forkBranchp, Begin));
break;
}
forkBranchp = blockp;
}
// 'process::kill' does not immediately kill the current process executing the disable
// statement (because it's in the running state). If the disable runs under the target
// fork, jump to the end of the innermost enclosing fork branch (the branch that holds the
// running process) so statements after the disable do not execute. Targeting the innermost
// branch keeps the jump inside a single emitted coroutine even when the disable sits in a
// nested sub-fork; targeting the outer target fork's branch would cross a coroutine
// boundary once forks are split into separate functions.
if (AstBegin* const branchp = innerForkBranchp(targetp)) {
addJumpAfterKill(killStmtp, branchp);
}
}
// VISITORS
@ -553,12 +554,8 @@ class LinkJumpVisitor final : public VNVisitor {
if (it != m_taskDisableBegins.end()) jumpTargetp = it->second;
addJumpAfterKill(killStmtp, jumpTargetp);
}
} else if (AstFork* const forkp = VN_CAST(targetp, Fork)) {
std::vector<AstBegin*> forks;
for (AstBegin* itemp = forkp->forksp(); itemp; itemp = VN_AS(itemp->nextp(), Begin)) {
forks.push_back(itemp);
}
handleDisableOnFork(nodep, forks);
} else if (VN_IS(targetp, Fork)) {
handleDisableOnFork(nodep);
} else if (AstBegin* const beginp = VN_CAST(targetp, Begin)) {
if (existsBlockAbove(beginp->name())) {
if (!beginp->user3()) {

View File

@ -749,10 +749,21 @@ class TimingControlVisitor final : public VNVisitor {
addDebugInfo(donep);
beginp->addStmtsp(donep->makeStmt());
}
static bool hasDisableQueuePushSelfPrefix(const AstBegin* const beginp) {
static bool hasDisableQueuePushSelfPrefix(AstBegin* const beginp) {
// LinkJump prepends disable-by-name registration as:
// __VprocessQueue_*.push_back(std::process::self())
return beginp->stmtsp() && beginp->stmtsp()->isDisableQueuePushSelfStmt();
// V3LiftExpr lifts std::process::self() into an assignment to a temporary, then V3Task
// lowers that assignment's function call into a leading comment and AstStmtExpr. Thus the
// push_back is no longer necessarily the first statement. Scan across this generated
// prefix for it, stopping at the fork-start sentinel or the branch body. Registrations for
// nested forks live in sub-blocks and so are not in this statement list.
for (AstNode* stmtp = beginp->stmtsp(); stmtp; stmtp = stmtp->nextp()) {
if (VN_IS(stmtp, Comment)) continue;
AstStmtExpr* const stmtExprp = VN_CAST(stmtp, StmtExpr);
if (!stmtExprp) break;
if (stmtExprp->isDisableQueuePushSelfStmt()) return true;
}
return false;
}
// Register a callback so killing a process-backed fork branch decrements the join counter
void addForkOnKill(AstBegin* const beginp, AstVarScope* const forkVscp) const {

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

View File

@ -0,0 +1,217 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain.
// SPDX-FileCopyrightText: 2026 Wilson Snyder
// SPDX-License-Identifier: CC0-1.0
// Disabling a named fork from a deeply nested sub-fork branch must terminate
// every process spawned within that fork, at all nesting levels, including
// branches suspended on a delay. Previously this crashed Verilator: the bail-out
// jump emitted for the disable targeted the outer fork branch and crossed a
// coroutine boundary once the nested forks were split into separate functions.
// Even disregarding the crash, processes of nested sub-forks were not registered
// for the kill, so a nested sibling survived and its $stop fired.
//
// The disable must also let the process that owns the disabled fork's 'join'
// resume after the join (IEEE 1800-2023 9.6.2): once the block is terminated,
// execution continues at the point after it. The *_resumed bits below latch that
// the parent ran past its join; a hang there leaves the bit clear and is caught
// at the end. A fork branch that itself holds a nested fork is killed while
// suspended at the inner join, so it must register a kill hook on its parent
// fork or the parent join counter is never decremented and the parent hangs.
// verilog_format: off
`define stop $stop
`define checkt(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0t exp=%0t (%t !== %t)\n", `__FILE__,`__LINE__, (gotv), (expv), `"gotv`", `"expv`"); `stop; end while(0);
`define checkd(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got=%0d exp=%0d\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0);
// verilog_format: on
module t;
bit d2_resumed = 1'b0;
bit d3_resumed = 1'b0;
bit d2a_resumed = 1'b0;
bit d2r_resumed = 1'b0;
bit ja1_resumed = 1'b0;
int prefix_queue[$];
// An ordinary queue push at the start of a fork branch must not be mistaken
// for compiler-generated disable queue registration.
initial begin
fork
begin
prefix_queue.push_back(1);
end
join
end
// fork..join nested two levels: disabling the outer named fork from inside the
// inner fork must kill the inner delayed sibling and the outer delayed sibling.
initial begin
fork : fork_d2
begin
fork
begin
`checkt($time, 0);
disable fork_d2;
$stop;
end
begin
`checkt($time, 0);
#1 $stop;
end
join
end
begin
`checkt($time, 0);
#1 $stop;
end
join
`checkt($time, 0);
d2_resumed = 1'b1;
end
// fork..join nested three levels: disabling the outermost named fork from the
// innermost branch must kill the delayed siblings at every level.
initial begin
fork : fork_d3
begin
fork
begin
fork
begin
`checkt($time, 0);
disable fork_d3;
$stop;
end
begin
`checkt($time, 0);
#1 $stop;
end
join
end
begin
`checkt($time, 0);
#1 $stop;
end
join
end
begin
`checkt($time, 0);
#1 $stop;
end
join
`checkt($time, 0);
d3_resumed = 1'b1;
end
// fork..join_any nested two levels.
initial begin
fork : fork_d2a
begin
fork
begin
`checkt($time, 0);
disable fork_d2a;
$stop;
end
begin
`checkt($time, 0);
#1 $stop;
end
join_any
end
begin
`checkt($time, 0);
#1 $stop;
end
join_any
`checkt($time, 0);
d2a_resumed = 1'b1;
end
// fork..join_none nested two levels.
initial begin
fork : fork_d2n
begin
fork
begin
`checkt($time, 0);
disable fork_d2n;
$stop;
end
begin
`checkt($time, 0);
#1 $stop;
end
join_none
end
begin
`checkt($time, 0);
#1 $stop;
end
join_none
end
// fork..join nested two levels, but the disabling branch comes after the
// delayed sibling (in source order) at both levels.
initial begin
fork : fork_d2r
begin
`checkt($time, 0);
#1 $stop;
end
begin
fork
begin
`checkt($time, 0);
#1 $stop;
end
begin
`checkt($time, 0);
disable fork_d2r;
$stop;
end
join
end
join
`checkt($time, 0);
d2r_resumed = 1'b1;
end
// fork..join_any whose only branch holds the nested fork that gets disabled.
// join_any needs one branch to complete, but the only branch is killed while
// suspended at its inner join, so the parent resumes only if that branch
// decrements the join counter on kill.
initial begin
fork : fork_ja1
begin
fork
begin
`checkt($time, 0);
disable fork_ja1;
$stop;
end
begin
`checkt($time, 0);
#1 $stop;
end
join
end
join_any
`checkt($time, 0);
ja1_resumed = 1'b1;
end
initial begin
#10;
`checkd(d2_resumed, 1'b1);
`checkd(d3_resumed, 1'b1);
`checkd(d2a_resumed, 1'b1);
`checkd(d2r_resumed, 1'b1);
`checkd(ja1_resumed, 1'b1);
`checkd(prefix_queue.size(), 1);
$write("*-* All Finished *-*\n");
$finish;
end
endmodule