From 925a6640d139b6ab9f4ad6dfe9d4d1442e386eb7 Mon Sep 17 00:00:00 2001 From: Yilou Wang Date: Tue, 4 Aug 2026 07:46:54 +0200 Subject: [PATCH] Fix queued $finish/$stop request thread ordering (#7946 prep) (#7952) --- include/verilated.cpp | 18 ++ include/verilated.h | 21 ++- test_regress/t/t_verilated_finish_pending.cpp | 158 ++++++++++++++++++ test_regress/t/t_verilated_finish_pending.py | 21 +++ test_regress/t/t_verilated_finish_pending.v | 8 + test_regress/t/t_verilated_user_hooks.cpp | 75 +++++++++ test_regress/t/t_verilated_user_hooks.py | 23 +++ test_regress/t/t_verilated_user_hooks.v | 24 +++ 8 files changed, 346 insertions(+), 2 deletions(-) create mode 100644 test_regress/t/t_verilated_finish_pending.cpp create mode 100755 test_regress/t/t_verilated_finish_pending.py create mode 100644 test_regress/t/t_verilated_finish_pending.v create mode 100644 test_regress/t/t_verilated_user_hooks.cpp create mode 100755 test_regress/t/t_verilated_user_hooks.py create mode 100644 test_regress/t/t_verilated_user_hooks.v diff --git a/include/verilated.cpp b/include/verilated.cpp index 981ac1f57..cb4b052da 100644 --- a/include/verilated.cpp +++ b/include/verilated.cpp @@ -252,14 +252,23 @@ void VL_FINISH_MT(const char* filename, int linenum, const char* hier) VL_MT_SAF } void VL_STOP_MT(const char* filename, int linenum, const char* hier, bool maybe) VL_MT_SAFE { + // Classify now, so a queued request is pending from the moment it is posted + VerilatedContext* const contextp = Verilated::threadContextp(); + const bool stop = contextp->stopRequestReserve(maybe); + if (stop) contextp->finishPendingInc(); VerilatedThreadMsgQueue::post(VerilatedMsg{[=]() { // vl_stop_maybe(filename, linenum, hier, maybe); + contextp->stopRequestRelease(); + if (stop) contextp->finishPendingDec(); }}); } void VL_FATAL_MT(const char* filename, int linenum, const char* hier, const char* msg) VL_MT_SAFE { + VerilatedContext* const contextp = Verilated::threadContextp(); + contextp->finishPendingInc(); VerilatedThreadMsgQueue::post(VerilatedMsg{[=]() { // vl_fatal(filename, linenum, hier, msg); + contextp->finishPendingDec(); }}); } @@ -3282,6 +3291,15 @@ void VerilatedContext::gotFinish(bool flag) VL_MT_SAFE { const VerilatedLockGuard lock{m_mutex}; m_s.m_gotFinish = flag; } +bool VerilatedContext::stopRequestReserve(bool maybe) VL_MT_SAFE { + const VerilatedLockGuard lock{m_mutex}; + const int reserved = ++m_ns.m_stopReserved; + return !maybe || m_s.m_errorCount + reserved >= m_s.m_errorLimit; +} +void VerilatedContext::stopRequestRelease() VL_MT_SAFE { + const VerilatedLockGuard lock{m_mutex}; + --m_ns.m_stopReserved; +} bool VerilatedContext::executingFinal() const VL_MT_SAFE { const VerilatedLockGuard lock{m_mutex}; return m_ns.m_executingFinal; diff --git a/include/verilated.h b/include/verilated.h index b8fc95e76..93e2b06bb 100644 --- a/include/verilated.h +++ b/include/verilated.h @@ -387,6 +387,8 @@ private: static uint32_t assertOnMask(VerilatedAssertType_t types, VerilatedAssertDirectiveType_t directives) VL_PURE; static constexpr size_t ASSERT_CONTROL_SLOT_COUNT = ASSERT_ON_WIDTH - 1; + // No termination request has stamped m_finishPendingTime yet + static constexpr uint64_t TIME_UNSET = ~0ULL; protected: // TYPES @@ -447,6 +449,8 @@ protected: // Fast path // A worker queues $finish before the main thread callback can set m_gotFinish. std::atomic m_finishPending{0}; // Number of queued $finish callbacks + std::atomic m_finishPendingTime{TIME_UNSET}; // Time of the first callback + int m_stopReserved = 0; // Posted $stop requests not yet executed bool m_executingFinal = false; // Running generated final() code uint64_t m_profExecStart = 1; // +prof+exec+start time uint32_t m_profExecWindow = 2; // +prof+exec+window size @@ -686,13 +690,26 @@ public: // METHODS - public but for internal use only - // Internal: Track $finish callbacks queued by worker threads + // Internal: Track $finish/$stop callbacks queued by worker threads bool finishPending() const VL_MT_SAFE { return m_ns.m_finishPending.load() != 0; } - void finishPendingInc() VL_MT_SAFE { ++m_ns.m_finishPending; } + void finishPendingInc() VL_MT_SAFE { + ++m_ns.m_finishPending; + uint64_t unset = TIME_UNSET; + m_ns.m_finishPendingTime.compare_exchange_strong(unset, time()); + } void finishPendingDec() VL_MT_SAFE { const uint32_t previous = m_ns.m_finishPending.fetch_sub(1); assert(previous > 0); + if (previous == 1 && !gotFinish()) m_ns.m_finishPendingTime = TIME_UNSET; } + // Internal: Time of the first termination request, else the current time + uint64_t finishPendingTime() const VL_MT_SAFE { + const uint64_t stamped = m_ns.m_finishPendingTime.load(); + return stamped == TIME_UNSET ? time() : stamped; + } + // Internal: Reserve a posted $stop, returning true if it reaches the termination limit + bool stopRequestReserve(bool maybe) VL_MT_SAFE; + void stopRequestRelease() VL_MT_SAFE; // Internal: access to implementation class VerilatedContextImp* impp() VL_MT_SAFE { return reinterpret_cast(this); } diff --git a/test_regress/t/t_verilated_finish_pending.cpp b/test_regress/t/t_verilated_finish_pending.cpp new file mode 100644 index 000000000..74ddc59f6 --- /dev/null +++ b/test_regress/t/t_verilated_finish_pending.cpp @@ -0,0 +1,158 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +// DESCRIPTION: Verilator: VerilatedContext pending termination state test +//************************************************************************* +// +// 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 PlanV GmbH +// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 +// +//************************************************************************* + +#include VM_PREFIX_INCLUDE + +// Workaround to include verilated_imp.h, needed to drive the eval message queue +#define VERILATOR_VERILATED_CPP_ +#include "verilated_imp.h" + +#include + +// These require the above. Comment prevents clang-format moving them +#include "TestCheck.h" + +int errors = 0; + +int stopCalls = 0; +int fatalCalls = 0; + +// Non-exiting overrides so a request can be observed after it ran +void vl_stop(const char* filename, int linenum, const char* hier) { ++stopCalls; } + +void vl_fatal(const char* filename, int linenum, const char* hier, const char* msg) { + ++fatalCalls; + TEST_CHECK_EQ(Verilated::threadContextp()->finishPending(), true); +} + +int main(int argc, char** argv) { + VerilatedContext context; + Verilated::threadContextp(&context); + context.commandArgs(argc, argv); + std::unique_ptr topp{new VM_PREFIX{&context}}; + + // A maybe-stop under the error limit is ignored and marks nothing pending + context.errorLimit(3); + context.errorCount(0); + VL_STOP_MT(__FILE__, __LINE__, "TOP.t"); + TEST_CHECK_EQ(context.errorCount(), 1); + TEST_CHECK_EQ(stopCalls, 0); + TEST_CHECK_EQ(context.finishPending(), false); + VL_STOP_MT(__FILE__, __LINE__, "TOP.t"); + TEST_CHECK_EQ(context.errorCount(), 2); + TEST_CHECK_EQ(stopCalls, 0); + TEST_CHECK_EQ(context.finishPending(), false); + + // Reaching the limit stops, and a definite stop always stops + VL_STOP_MT(__FILE__, __LINE__, "TOP.t"); + TEST_CHECK_EQ(context.errorCount(), 3); + TEST_CHECK_EQ(stopCalls, 1); + context.errorCount(0); + VL_STOP_MT(__FILE__, __LINE__, "TOP.t", false); + TEST_CHECK_EQ(context.errorCount(), 1); + TEST_CHECK_EQ(stopCalls, 2); + + // A worker-queued stop is pending from the moment it is posted, and the + // error is counted only when the queued message runs + context.errorCount(0); + context.time(10); + { + VerilatedEvalMsgQueue evalMsgQ; + Verilated::mtaskId(1); + VL_STOP_MT(__FILE__, __LINE__, "TOP.t", false); + TEST_CHECK_EQ(context.errorCount(), 0); + TEST_CHECK_EQ(stopCalls, 2); + TEST_CHECK_EQ(context.finishPending(), true); + TEST_CHECK_EQ(context.finishPendingTime(), 10); + context.time(20); + Verilated::endOfThreadMTask(&evalMsgQ); + TEST_CHECK_EQ(stopCalls, 2); + TEST_CHECK_EQ(context.finishPending(), true); + TEST_CHECK_EQ(context.finishPendingTime(), 10); + Verilated::endOfEval(&evalMsgQ); + TEST_CHECK_EQ(context.errorCount(), 1); + TEST_CHECK_EQ(stopCalls, 3); + TEST_CHECK_EQ(context.finishPending(), false); + TEST_CHECK_EQ(context.finishPendingTime(), 20); + } + + // An ignored worker-queued maybe-stop must not gate same-slot work + context.errorLimit(3); + context.errorCount(0); + { + VerilatedEvalMsgQueue evalMsgQ; + Verilated::mtaskId(1); + VL_STOP_MT(__FILE__, __LINE__, "TOP.t"); + TEST_CHECK_EQ(context.finishPending(), false); + Verilated::endOfThreadMTask(&evalMsgQ); + Verilated::endOfEval(&evalMsgQ); + TEST_CHECK_EQ(context.errorCount(), 1); + TEST_CHECK_EQ(stopCalls, 3); + TEST_CHECK_EQ(context.finishPending(), false); + } + + // Several queued maybe-stops still stop exactly once, on the one that crosses + context.errorLimit(3); + context.errorCount(0); + { + VerilatedEvalMsgQueue evalMsgQ; + Verilated::mtaskId(1); + VL_STOP_MT(__FILE__, __LINE__, "TOP.t"); + VL_STOP_MT(__FILE__, __LINE__, "TOP.t"); + VL_STOP_MT(__FILE__, __LINE__, "TOP.t"); + TEST_CHECK_EQ(context.finishPending(), true); + Verilated::endOfThreadMTask(&evalMsgQ); + Verilated::endOfEval(&evalMsgQ); + TEST_CHECK_EQ(context.errorCount(), 3); + TEST_CHECK_EQ(stopCalls, 4); + TEST_CHECK_EQ(context.finishPending(), false); + } + + // A worker-queued fatal stays pending until end-of-eval runs its handler + context.time(30); + { + VerilatedEvalMsgQueue evalMsgQ; + Verilated::mtaskId(1); + VL_FATAL_MT(__FILE__, __LINE__, "TOP.t", "queued fatal"); + TEST_CHECK_EQ(fatalCalls, 0); + TEST_CHECK_EQ(context.finishPending(), true); + TEST_CHECK_EQ(context.finishPendingTime(), 30); + Verilated::endOfThreadMTask(&evalMsgQ); + Verilated::endOfEval(&evalMsgQ); + TEST_CHECK_EQ(fatalCalls, 1); + TEST_CHECK_EQ(context.finishPending(), false); + } + + // The first pending request owns the timestamp until all requests drain + context.time(40); + context.finishPendingInc(); + context.time(50); + context.finishPendingInc(); + TEST_CHECK_EQ(context.finishPendingTime(), 40); + context.finishPendingDec(); + TEST_CHECK_EQ(context.finishPending(), true); + TEST_CHECK_EQ(context.finishPendingTime(), 40); + context.finishPendingDec(); + TEST_CHECK_EQ(context.finishPending(), false); + TEST_CHECK_EQ(context.finishPendingTime(), 50); + + // A latched termination keeps the timestamp after the request drains + context.time(60); + context.finishPendingInc(); + context.gotFinish(true); + context.finishPendingDec(); + context.time(70); + TEST_CHECK_EQ(context.finishPendingTime(), 60); + + topp->final(); + return errors ? 10 : 0; +} diff --git a/test_regress/t/t_verilated_finish_pending.py b/test_regress/t/t_verilated_finish_pending.py new file mode 100755 index 000000000..8f7ee09bc --- /dev/null +++ b/test_regress/t/t_verilated_finish_pending.py @@ -0,0 +1,21 @@ +#!/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_all') + +test.compile( + make_top_shell=False, + make_main=False, + verilator_flags2=['--exe', test.pli_filename, "-CFLAGS '-DVL_USER_STOP -DVL_USER_FATAL'"]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_verilated_finish_pending.v b/test_regress/t/t_verilated_finish_pending.v new file mode 100644 index 000000000..42e88c617 --- /dev/null +++ b/test_regress/t/t_verilated_finish_pending.v @@ -0,0 +1,8 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 PlanV GmbH +// SPDX-License-Identifier: CC0-1.0 + +module t; +endmodule diff --git a/test_regress/t/t_verilated_user_hooks.cpp b/test_regress/t/t_verilated_user_hooks.cpp new file mode 100644 index 000000000..dc3794192 --- /dev/null +++ b/test_regress/t/t_verilated_user_hooks.cpp @@ -0,0 +1,75 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +// DESCRIPTION: Verilator: User replaced vl_finish/vl_stop/vl_fatal/vl_warn test +//************************************************************************* +// +// 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 PlanV GmbH +// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 +// +//************************************************************************* + +#include VM_PREFIX_INCLUDE + +#include + +// This requires the above. Comment prevents clang-format moving it +#include "TestCheck.h" + +int errors = 0; + +int finishCalls = 0; +int stopCalls = 0; +int fatalCalls = 0; +int warnCalls = 0; + +// An embedder replaces these four and keeps control of the process +void vl_finish(const char* filename, int linenum, const char* hier) { ++finishCalls; } + +void vl_stop(const char* filename, int linenum, const char* hier) { + ++stopCalls; + if (Verilated::threadContextp()->fatalOnError()) + vl_fatal(filename, linenum, hier, "Verilog $stop"); +} + +void vl_fatal(const char* filename, int linenum, const char* hier, const char* msg) { + ++fatalCalls; +} + +void vl_warn(const char* filename, int linenum, const char* hier, const char* msg) { ++warnCalls; } + +static void tick(VM_PREFIX* topp, int step) { + topp->step = step; + topp->clk = 0; + topp->eval(); + topp->clk = 1; + topp->eval(); +} + +int main(int argc, char** argv) { + VerilatedContext context; + Verilated::threadContextp(&context); + context.commandArgs(argc, argv); + std::unique_ptr topp{new VM_PREFIX{&context}}; + + tick(topp.get(), 1); + TEST_CHECK_EQ(warnCalls, 1); + + tick(topp.get(), 2); + TEST_CHECK_EQ(stopCalls, 1); + TEST_CHECK_EQ(fatalCalls, 1); + TEST_CHECK_EQ(context.errorCount(), 1); + + tick(topp.get(), 3); + TEST_CHECK_EQ(stopCalls, 2); + TEST_CHECK_EQ(fatalCalls, 2); + TEST_CHECK_EQ(context.errorCount(), 2); + + tick(topp.get(), 4); + TEST_CHECK_EQ(finishCalls, 1); + + TEST_CHECK_EQ(warnCalls, 1); + topp->final(); + return errors ? 10 : 0; +} diff --git a/test_regress/t/t_verilated_user_hooks.py b/test_regress/t/t_verilated_user_hooks.py new file mode 100755 index 000000000..158818409 --- /dev/null +++ b/test_regress/t/t_verilated_user_hooks.py @@ -0,0 +1,23 @@ +#!/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_all') + +test.compile(make_top_shell=False, + make_main=False, + verilator_flags2=[ + '--exe', test.pli_filename, + "-CFLAGS '-DVL_USER_FINISH -DVL_USER_STOP -DVL_USER_FATAL -DVL_USER_WARN'" + ]) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_verilated_user_hooks.v b/test_regress/t/t_verilated_user_hooks.v new file mode 100644 index 000000000..0f2e94d69 --- /dev/null +++ b/test_regress/t/t_verilated_user_hooks.v @@ -0,0 +1,24 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 PlanV GmbH +// SPDX-License-Identifier: CC0-1.0 + +module t ( + input logic clk, + input int step +); + + logic [7:0] mem[0:1]; + + always @(posedge clk) begin + case (step) + 1: $readmemh("t_verilated_user_hooks_no_such_file.mem", mem); + 2: $stop; + 3: $fatal(0, "user hook fatal"); + 4: $finish; + default: ; + endcase + end + +endmodule