From b1ce702eaf2162ad69b30171ba4b47061ffc0f98 Mon Sep 17 00:00:00 2001 From: username Date: Fri, 10 Jul 2026 22:21:47 +0200 Subject: [PATCH] Fix immediate disable of fork join branches (#7856) --- src/V3Fork.cpp | 74 ++++++++--- test_regress/t/t_disable_fork_join.py | 18 +++ test_regress/t/t_disable_fork_join.v | 171 ++++++++++++++++++++++++++ 3 files changed, 244 insertions(+), 19 deletions(-) create mode 100755 test_regress/t/t_disable_fork_join.py create mode 100644 test_regress/t/t_disable_fork_join.v diff --git a/src/V3Fork.cpp b/src/V3Fork.cpp index 025c13bdb..0b58c859c 100644 --- a/src/V3Fork.cpp +++ b/src/V3Fork.cpp @@ -593,22 +593,50 @@ class ForkVisitor final : public VNVisitor { const AstConst* const constp = VN_CAST(delayp->lhsp(), Const); return constp && (constp->toUQuad() == std::numeric_limits::max()); } - static void moveForkSentinelAfterDisableQueuePushes(AstBegin* const beginp) { - AstNode* const firstStmtp = beginp->stmtsp(); - if (!isForkJoinNoneSentinelDelay(firstStmtp)) return; - - AstNode* insertBeforep = firstStmtp->nextp(); + static bool isDisableQueuePushSelfPrefix(AstNode* nodep) { + while (AstJumpBlock* const jumpBlockp = VN_CAST(nodep, JumpBlock)) { + nodep = jumpBlockp->stmtsp(); + } + return nodep && nodep->isDisableQueuePushSelfStmt(); + } + template + static bool insertForkSentinelAfterDisableQueuePushes(T_Owner* const ownerp, + AstNode* const firstStmtp, + AstNode* const delayp) { + AstNode* insertBeforep = firstStmtp; while (insertBeforep && insertBeforep->isDisableQueuePushSelfStmt()) { insertBeforep = insertBeforep->nextp(); } - if (insertBeforep == firstStmtp->nextp()) return; - - AstNode* const delayp = firstStmtp->unlinkFrBack(); + if (AstJumpBlock* const jumpBlockp = VN_CAST(insertBeforep, JumpBlock)) { + if (insertForkSentinelAfterDisableQueuePushes(jumpBlockp, jumpBlockp->stmtsp(), + delayp)) { + return true; + } + } + if (insertBeforep == firstStmtp) return false; if (insertBeforep) { insertBeforep->addHereThisAsNext(delayp); } else { - beginp->addStmtsp(delayp); + ownerp->addStmtsp(delayp); } + return true; + } + static void moveForkSentinelAfterDisableQueuePushes(AstBegin* const beginp) { + AstNode* const firstStmtp = beginp->stmtsp(); + if (!isForkJoinNoneSentinelDelay(firstStmtp)) return; + AstNode* const afterSentinelp = firstStmtp->nextp(); + if (!isDisableQueuePushSelfPrefix(afterSentinelp)) return; + + AstNode* const delayp = firstStmtp->unlinkFrBack(); + const bool moved + = insertForkSentinelAfterDisableQueuePushes(beginp, afterSentinelp, delayp); + UASSERT_OBJ(moved, beginp, "Failed to move fork sentinel after disable queue pushes"); + } + static bool forkIsDisableable(AstFork* const nodep) { + for (AstBegin* itemp = nodep->forksp(); itemp; itemp = VN_AS(itemp->nextp(), Begin)) { + if (isDisableQueuePushSelfPrefix(itemp->stmtsp())) return true; + } + return false; } // VISITORS @@ -625,17 +653,14 @@ class ForkVisitor final : public VNVisitor { } void visit(AstFork* nodep) override { - if (nodep->joinType().join()) { - iterateChildren(nodep); - return; - } - // IEEE 1800-2023 9.3.2: In all cases, processes spawned by a fork-join block shall not - // start executing until the parent process is blocked or terminates. - // Because join and join_any block the parent process, it is only needed when join_none - // is used. - if (nodep->joinType().joinNone()) { - UINFO(9, "Visiting fork..join_none " << nodep); + // start executing until the parent process is blocked or terminates. Because join and + // join_any block the parent process, deferring branch start with a synthetic #0 delay is + // normally only needed for join_none. A fork that can be disabled by name needs the same + // deferral for every join type so all branches register their processes before any branch + // body can disable the block. + if (nodep->joinType().joinNone() || forkIsDisableable(nodep)) { + UINFO(9, "Adding fork branch start sentinels " << nodep); FileLine* fl = nodep->fileline(); // We use a sentinel value of UINT64_MAX to mark this delay so that it goes to the // ACTIVE region with a delay value of 0. @@ -653,6 +678,17 @@ class ForkVisitor final : public VNVisitor { iterateAndNextNull(nodep->declsp()); iterateAndNextNull(nodep->stmtsp()); + + // A plain 'join' blocks the parent until every branch finishes, so a branch cannot + // outlive the variables it references and need not be extracted into a task; just + // recurse to process any forks nested inside the branches. join_any and join_none + // branches may outlive the fork, so each branch body is wrapped in a task that + // captures the variables it references. + if (nodep->joinType().join()) { + iterateAndNextNull(nodep->forksp()); + return; + } + std::vector wrappedp; { VL_RESTORER(m_inFork); diff --git a/test_regress/t/t_disable_fork_join.py b/test_regress/t/t_disable_fork_join.py new file mode 100755 index 000000000..6fe7d000c --- /dev/null +++ b/test_regress/t/t_disable_fork_join.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_disable_fork_join.v b/test_regress/t/t_disable_fork_join.v new file mode 100644 index 000000000..8cf504faa --- /dev/null +++ b/test_regress/t/t_disable_fork_join.v @@ -0,0 +1,171 @@ +// 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..join / fork..join_any block from one of its own +// branches must terminate the sibling branches, including any that are +// suspended on a delay. This is the join/join_any analogue of the +// fork..join_none cases in t_disable_inside (#6591): the bug was that a sibling +// spawned later in source order registered its process for kill only after the +// disabling branch had already issued the kill, so the sibling survived and its +// $stop fired one time unit later. + +// 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); +// verilog_format: on + +module t; + + // fork..join: the disabling branch comes first, the delayed sibling second. + initial begin + fork : fork_blk2 + begin + `checkt($time, 0); + disable fork_blk2; + $stop; + end + begin + `checkt($time, 0); + #1 $stop; + end + join + end + + // fork..join: the delayed sibling comes first, the disabling branch second. + initial begin + fork : fork_blk3 + begin + `checkt($time, 0); + #1 $stop; + end + begin + `checkt($time, 0); + disable fork_blk3; + $stop; + end + join + end + + // fork..join: the delayed sibling also contains an unreachable disable, which + // makes LinkJump wrap its registration in a JumpBlock. It must still register + // before the synthetic fork-start delay so the earlier disable kills it. + initial begin + fork : fork_blk_wrap + begin + `checkt($time, 0); + disable fork_blk_wrap; + $stop; + end + begin + `checkt($time, 0); + #1; + if ($c("false")) disable fork_blk_wrap; + $stop; + end + join + end + + // fork..join_any: the disabling branch comes first, the delayed sibling second. + initial begin + fork : fork_blk2a + begin + `checkt($time, 0); + disable fork_blk2a; + $stop; + end + begin + `checkt($time, 0); + #1 $stop; + end + join_any + end + + // fork..join_any: the delayed sibling comes first, the disabling branch second. + initial begin + fork : fork_blk3a + begin + `checkt($time, 0); + #1 $stop; + end + begin + `checkt($time, 0); + disable fork_blk3a; + $stop; + end + join_any + end + + // fork..join_any: the delayed sibling's own unreachable disable again forces a + // JumpBlock wrapper around its registration. + initial begin + fork : fork_blk_wrap_any + begin + `checkt($time, 0); + disable fork_blk_wrap_any; + $stop; + end + begin + `checkt($time, 0); + #1; + if ($c("false")) disable fork_blk_wrap_any; + $stop; + end + join_any + end + + // fork..join that can be disabled, but takes the path where disable is not + // reached: both branches must run to completion with their timing intact. + initial begin + fork : fork_blk4 + begin + `checkt($time, 0); + if ($c("false")) begin + disable fork_blk4; + $stop; + end + #1; + `checkt($time, 1); + end + begin + `checkt($time, 0); + #1; + `checkt($time, 1); + end + join + end + + // Deeply nested fork..join: the named fork is several levels deep inside other + // forks and one of its own branches disables it. The delayed sibling at that + // deep level must still be killed, so the fix has to work when the disable-able + // fork is itself nested inside other (suspending) forks. + initial begin + fork + begin + fork + begin + fork : fork_deep + begin + `checkt($time, 0); + disable fork_deep; + $stop; + end + begin + `checkt($time, 0); + #1 $stop; + end + join + end + join + end + join + end + + initial begin + #10; + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule