From 48e575b8858b47774760e0e9e6bab4f1e19f1f61 Mon Sep 17 00:00:00 2001 From: Marco Bartoli Date: Tue, 28 Jul 2026 23:31:32 +0200 Subject: [PATCH] Fix self-disable of named blocks with forks (Part 3 of #7857) (#7996) --- src/V3LinkJump.cpp | 30 +++++----- test_regress/t/t_disable_begin_self_fork.py | 18 ++++++ test_regress/t/t_disable_begin_self_fork.v | 58 +++++++++++++++++++ test_regress/t/t_disable_begin_shadow_bad.out | 5 ++ test_regress/t/t_disable_begin_shadow_bad.py | 19 ++++++ test_regress/t/t_disable_begin_shadow_bad.v | 22 +++++++ 6 files changed, 138 insertions(+), 14 deletions(-) create mode 100755 test_regress/t/t_disable_begin_self_fork.py create mode 100644 test_regress/t/t_disable_begin_self_fork.v create mode 100644 test_regress/t/t_disable_begin_shadow_bad.out create mode 100755 test_regress/t/t_disable_begin_shadow_bad.py create mode 100644 test_regress/t/t_disable_begin_shadow_bad.v diff --git a/src/V3LinkJump.cpp b/src/V3LinkJump.cpp index 848d05929..ff0e0f2d7 100644 --- a/src/V3LinkJump.cpp +++ b/src/V3LinkJump.cpp @@ -167,6 +167,18 @@ class LinkJumpVisitor final : public VNVisitor { } return false; } + AstBegin* innerForkBranchp(const AstNodeBlock* const targetp) const { + AstBegin* innerForkBranchp = nullptr; + AstNodeBlock* prevBlockp = nullptr; + for (AstNodeBlock* const blockp : vlstd::reverse_view(m_blockStack)) { + if (!innerForkBranchp && VN_IS(blockp, Fork)) { + innerForkBranchp = VN_CAST(prevBlockp, Begin); + } + if (blockp == targetp) return innerForkBranchp; + prevBlockp = blockp; + } + return nullptr; + } static AstStmtExpr* getQueuePushProcessSelfp(AstVarRef* const queueRefp) { // Constructs queue.push_back(std::process::self()) statement FileLine* const flp = queueRefp->fileline(); @@ -206,10 +218,6 @@ class LinkJumpVisitor final : public VNVisitor { } nodep->addStmtsp(stmtp); } - static bool directlyUnderFork(const AstNode* const nodep) { - if (nodep->backp()->nextp() == nodep) return directlyUnderFork(nodep->backp()); - return VN_IS(nodep->backp(), Fork); - } AstBegin* getOrCreateTaskDisableBeginp(AstTask* const taskp, FileLine* const fl) { const auto it = m_taskDisableBegins.find(taskp); if (it != m_taskDisableBegins.end()) return it->second; @@ -565,16 +573,10 @@ class LinkJumpVisitor final : public VNVisitor { // process::kill does not terminate the currently running process immediately. // If disable executes inside a fork branch of this named block, jump to the // end of that branch to prevent statements after disable from executing. - AstBegin* currentBeginp = nullptr; - for (AstNodeBlock* const blockp : vlstd::reverse_view(m_blockStack)) { - if (VN_IS(blockp, Begin)) { - currentBeginp = VN_AS(blockp, Begin); - break; - } - } - if (currentBeginp && directlyUnderFork(currentBeginp)) { - addJumpAfterKill(killStmtp, currentBeginp); - } + AstBegin* const branchp = innerForkBranchp(beginp); + AstBegin* const jumpTargetp + = branchp ? branchp : m_beginDisableBegins.at(beginp); + addJumpAfterKill(killStmtp, jumpTargetp); } } else { AstVar* const processQueuep diff --git a/test_regress/t/t_disable_begin_self_fork.py b/test_regress/t/t_disable_begin_self_fork.py new file mode 100755 index 000000000..6fe7d000c --- /dev/null +++ b/test_regress/t/t_disable_begin_self_fork.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_begin_self_fork.v b/test_regress/t/t_disable_begin_self_fork.v new file mode 100644 index 000000000..da19f244b --- /dev/null +++ b/test_regress/t/t_disable_begin_self_fork.v @@ -0,0 +1,58 @@ +// 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 + +// A named begin block that contains a fork still has to behave like a normal +// named block when it disables itself: execution resumes after the block, so +// statements after the disable inside the block must not run. All activity +// enabled within the block, including forked child processes, must terminate. + +// verilog_format: off +`define stop $stop +`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 after_disable = 1'b0; + bit after_disable_fork = 1'b0; + bit disable_fork_survived = 1'b0; + bit fork_survived = 1'b0; + + initial begin : blk + fork + begin + #5; + fork_survived = 1'b1; + end + join_none + #1; + disable blk; + after_disable = 1'b1; + end + + // A named fork disabled after join_none has no active enclosing fork branch. + initial begin + fork : fork_blk + begin + #5; + disable_fork_survived = 1'b1; + end + join_none + #1; + disable fork_blk; + after_disable_fork = 1'b1; + end + + initial begin + #10; + `checkd(after_disable, 1'b0); + `checkd(after_disable_fork, 1'b1); + `checkd(disable_fork_survived, 1'b0); + `checkd(fork_survived, 1'b0); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_disable_begin_shadow_bad.out b/test_regress/t/t_disable_begin_shadow_bad.out new file mode 100644 index 000000000..1ce133514 --- /dev/null +++ b/test_regress/t/t_disable_begin_shadow_bad.out @@ -0,0 +1,5 @@ +%Error: t/t_disable_begin_shadow_bad.v:19:7: break isn't underneath a loop + 19 | break; + | ^~~~~ + ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. +%Error: Exiting due to diff --git a/test_regress/t/t_disable_begin_shadow_bad.py b/test_regress/t/t_disable_begin_shadow_bad.py new file mode 100755 index 000000000..c58485648 --- /dev/null +++ b/test_regress/t/t_disable_begin_shadow_bad.py @@ -0,0 +1,19 @@ +#!/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('vlt') + +# Exit on the deliberate LinkJump error before later passes inspect the tree. +test.lint(verilator_flags2=["--no-debug-check"], + fails=True, + expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_disable_begin_shadow_bad.v b/test_regress/t/t_disable_begin_shadow_bad.v new file mode 100644 index 000000000..5e9642e82 --- /dev/null +++ b/test_regress/t/t_disable_begin_shadow_bad.v @@ -0,0 +1,22 @@ +// 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 + +// The invalid break exits after LinkJump handles the shadowed disable target. + +module t; + + initial begin : blk + fork + join + end + + initial fork : caller + begin : blk + disable t.blk; + break; + end + join +endmodule