diff --git a/include/verilated_timing.cpp b/include/verilated_timing.cpp index 41b5e0430..d44c7a948 100644 --- a/include/verilated_timing.cpp +++ b/include/verilated_timing.cpp @@ -57,24 +57,33 @@ void VlDelayScheduler::resume() { #ifdef VL_DEBUG VL_DEBUG_IF(dump(); VL_DBG_MSGF(" Resuming delayed processes\n");); #endif - while (awaitingCurrentTime()) { - if (m_queue.begin()->first != m_context.time()) { - VL_FATAL_MT(__FILE__, __LINE__, "", - "%Error: Encountered process that should've been resumed at an " - "earlier simulation time. Missed a time slot?"); - } - // Remove earliest handle + bool resumed = false; + + while (!m_queue.empty() && (m_queue.begin()->first == m_context.time())) { VlCoroutineHandle handle = std::move(m_queue.begin()->second); m_queue.erase(m_queue.begin()); handle.resume(); + resumed = true; + } + + if (!m_zeroDelayed.empty()) { + for (auto&& handle : m_zeroDelayed) handle.resume(); + m_zeroDelayed.clear(); + resumed = true; + } + + if (!resumed) { + VL_FATAL_MT(__FILE__, __LINE__, "", + "%Error: Encountered process that should've been resumed at an " + "earlier simulation time. Missed a time slot?\n"); } } uint64_t VlDelayScheduler::nextTimeSlot() const { - if (empty()) { + if (!m_queue.empty()) return m_queue.begin()->first; + if (m_zeroDelayed.empty()) VL_FATAL_MT(__FILE__, __LINE__, "", "%Error: There is no next time slot scheduled"); - } - return m_queue.begin()->first; + return m_context.time(); } #ifdef VL_DEBUG @@ -83,6 +92,12 @@ void VlDelayScheduler::dump() const { VL_DBG_MSGF(" No delayed processes:\n"); } else { VL_DBG_MSGF(" Delayed processes:\n"); + for (auto& susp : m_zeroDelayed) { + VL_DBG_MSGF(" Awaiting #0-delayed resumption, " + "time () %" PRIu64 ": ", + m_context.time()); + susp.dump(); + } for (const auto& susp : m_queue) { VL_DBG_MSGF(" Awaiting time %" PRIu64 ": ", susp.first); susp.second.dump(); diff --git a/include/verilated_timing.h b/include/verilated_timing.h index d2d67ecc5..5d1403eb8 100644 --- a/include/verilated_timing.h +++ b/include/verilated_timing.h @@ -27,6 +27,8 @@ #include "verilated.h" +#include + // clang-format off // Some preprocessor magic to support both Clang and GCC coroutines with both libc++ and libstdc++ #if defined _LIBCPP_VERSION // libc++ @@ -148,6 +150,8 @@ public: #endif }; +enum class VlDelayPhase : bool { ACTIVE, INACTIVE }; + //============================================================================= // VlDelayScheduler stores coroutines to be resumed at a certain simulation time. If the current // time is equal to a coroutine's resume time, the coroutine gets resumed. @@ -155,11 +159,12 @@ public: class VlDelayScheduler final { // TYPES // Time-sorted queue of timestamps and handles - using VlDelayedCoroutineQueue = std::multimap; + using VlDelayedCoroutineQueue = std::multimap; // MEMBERS VerilatedContext& m_context; VlDelayedCoroutineQueue m_queue; // Coroutines to be restored at a certain simulation time + std::vector m_zeroDelayed; // Coroutines waiting for #0 public: // CONSTRUCTORS @@ -172,10 +177,11 @@ public: // coroutines) uint64_t nextTimeSlot() const; // Are there no delayed coroutines awaiting? - bool empty() const { return m_queue.empty(); } + bool empty() const { return m_queue.empty() && m_zeroDelayed.empty(); } // Are there coroutines to resume at the current simulation time? bool awaitingCurrentTime() const { - return !empty() && m_queue.begin()->first <= m_context.time(); + return (!m_queue.empty() && (m_queue.begin()->first <= m_context.time())) + || !m_zeroDelayed.empty(); } #ifdef VL_DEBUG void dump() const; @@ -186,17 +192,34 @@ public: struct Awaitable { VlProcessRef process; // Data of the suspended process, null if not needed VlDelayedCoroutineQueue& queue; + std::vector& queueZeroDelay; uint64_t delay; + VlDelayPhase phase; VlFileLineDebug fileline; bool await_ready() const { return false; } // Always suspend void await_suspend(std::coroutine_handle<> coro) { - queue.emplace(delay, VlCoroutineHandle{coro, process, fileline}); + if (phase == VlDelayPhase::ACTIVE) { + queue.emplace(delay, VlCoroutineHandle{coro, process, fileline}); + } else { + queueZeroDelay.emplace_back(VlCoroutineHandle{coro, process, fileline}); + } } void await_resume() const {} }; - return Awaitable{process, m_queue, m_context.time() + delay, - VlFileLineDebug{filename, lineno}}; + + VlDelayPhase phase = (delay == 0) ? VlDelayPhase::INACTIVE : VlDelayPhase::ACTIVE; +#ifdef VL_DEBUG + if (phase == VlDelayPhase::INACTIVE) { + VL_WARN_MT(filename, lineno, VL_UNKNOWN, + "Encountered #0 delay. #0 scheduling support is incomplete and the " + "process will be resumed before combinational logic evaluation."); + } +#endif + + return Awaitable{process, m_queue, + m_zeroDelayed, m_context.time() + delay, + phase, VlFileLineDebug{filename, lineno}}; } }; diff --git a/src/V3SchedTiming.cpp b/src/V3SchedTiming.cpp index 780a699e2..9197475b2 100644 --- a/src/V3SchedTiming.cpp +++ b/src/V3SchedTiming.cpp @@ -67,11 +67,22 @@ AstCCall* TimingKit::createResume(AstNetlist* const netlistp) { m_resumeFuncp->isConst(false); m_resumeFuncp->declPrivate(true); scopeTopp->addBlocksp(m_resumeFuncp); + + // Put all the timing actives in the resume function + AstActive* dlyShedActivep = nullptr; for (auto& p : m_lbs) { - // Put all the timing actives in the resume function AstActive* const activep = p.second; + // Hack to ensure that #0 delays will be executed after any other `act` events. + // Just handle delayed coroutines last. + AstVarRef* const schedrefp = VN_AS( + VN_AS(VN_AS(activep->stmtsp(), StmtExpr)->exprp(), CMethodHard)->fromp(), VarRef); + if (schedrefp->varScopep()->dtypep()->basicp()->isDelayScheduler()) { + dlyShedActivep = activep; + continue; + } m_resumeFuncp->addStmtsp(activep); } + if (dlyShedActivep) m_resumeFuncp->addStmtsp(dlyShedActivep); } AstCCall* const callp = new AstCCall{m_resumeFuncp->fileline(), m_resumeFuncp}; callp->dtypeSetVoid(); diff --git a/src/V3Timing.cpp b/src/V3Timing.cpp index 02dc9b210..628bb9911 100644 --- a/src/V3Timing.cpp +++ b/src/V3Timing.cpp @@ -896,10 +896,7 @@ class TimingControlVisitor final : public VNVisitor { FileLine* const flp = nodep->fileline(); AstNodeExpr* valuep = V3Const::constifyEdit(nodep->lhsp()->unlinkFrBack()); auto* const constp = VN_CAST(valuep, Const); - if (constp && constp->isZero()) { - nodep->v3warn(ZERODLY, "Unsupported: #0 delays do not schedule process resumption in " - "the Inactive region"); - } else { + if (!constp || !constp->isZero()) { // Scale the delay if (valuep->dtypep()->isDouble()) { valuep = new AstRToIRoundS{ diff --git a/test_regress/t/t_delay_var.out b/test_regress/t/t_delay_var.out deleted file mode 100644 index 1ee2ef517..000000000 --- a/test_regress/t/t_delay_var.out +++ /dev/null @@ -1,6 +0,0 @@ -%Warning-ZERODLY: t/t_delay_var.v:20:7: Unsupported: #0 delays do not schedule process resumption in the Inactive region - 20 | #0 in = 1'b1; - | ^ - ... For warning description see https://verilator.org/warn/ZERODLY?v=latest - ... Use "/* verilator lint_off ZERODLY */" and lint_on around source to disable this message. -%Error: Exiting due to diff --git a/test_regress/t/t_delay_var.pl b/test_regress/t/t_delay_var.pl index 68a8b13cd..0b10f155d 100755 --- a/test_regress/t/t_delay_var.pl +++ b/test_regress/t/t_delay_var.pl @@ -13,8 +13,6 @@ scenarios(vlt => 1); compile( verilator_flags2 => ["--exe --main --timing"], make_main => 0, - fails => $Self->{vlt}, - expect_filename => $Self->{golden_filename}, ); execute( diff --git a/test_regress/t/t_delay_var.v b/test_regress/t/t_delay_var.v index b303a45f0..6c0137758 100644 --- a/test_regress/t/t_delay_var.v +++ b/test_regress/t/t_delay_var.v @@ -17,7 +17,6 @@ module t; wire #PDLY d_param = in; initial begin - #0 in = 1'b1; #2 in = 1'b0; #100; $write("*-* All Finished *-*\n"); diff --git a/test_regress/t/t_timing_zerodly.pl b/test_regress/t/t_timing_zerodly.pl new file mode 100755 index 000000000..9e4a1a1ff --- /dev/null +++ b/test_regress/t/t_timing_zerodly.pl @@ -0,0 +1,23 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2022 by Antmicro Ltd. 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-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(simulator => 1); + +compile( + verilator_flags2 => ["--exe --main --timing"], + make_main => 0, + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_timing_zerodly.v b/test_regress/t/t_timing_zerodly.v new file mode 100644 index 000000000..941c6df01 --- /dev/null +++ b/test_regress/t/t_timing_zerodly.v @@ -0,0 +1,42 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2023 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +class Foo; + static task fork_w_zerodly(longint unsigned current_time); + bit my_bit; + bit zero_dly_first = 0; + // Th code below relies on Verilator's deterministic scheduler and is not + // compatible across different simulators. + // + // The `zero_dly` block is going to be executed first and then suspended at the #0 delay. + // Then the `finish_before` block is going to be executed. Once that happens, the + // execution of `zero_dly` block should be resumed, all within a single time-slot. + // + // IF THIS TEST FAILS AFTER CHANGES TO VERILATOR'S SCHEDULER, IT DOESN'T NECESSARILY + // MEAN THE CHANGES ARE WRONG. + fork + begin : zero_dly + zero_dly_first = 1; + #0; + if (current_time != $time) $stop; + if (my_bit == 0) $stop; + end : zero_dly + begin : finish_before + if (!zero_dly_first) $stop; + my_bit = 1; + end : finish_before + join_none + #1 $display("After fork."); // Check if there's no skipped coroutine + endtask +endclass + +module test(); + initial begin + Foo::fork_w_zerodly($time); + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_timing_zerodly_unsup.out b/test_regress/t/t_timing_zerodly_unsup.out index bc85c6dcb..8e56ef2cf 100644 --- a/test_regress/t/t_timing_zerodly_unsup.out +++ b/test_regress/t/t_timing_zerodly_unsup.out @@ -1,6 +1,3 @@ -%Warning-ZERODLY: t/t_timing_zerodly_unsup.v:12:13: Unsupported: #0 delays do not schedule process resumption in the Inactive region - 12 | #0 if (v) $finish; - | ^ - ... For warning description see https://verilator.org/warn/ZERODLY?v=latest - ... Use "/* verilator lint_off ZERODLY */" and lint_on around source to disable this message. -%Error: Exiting due to +%Warning: t/t_timing_zerodly_unsup.v:22: Encountered #0 delay. #0 scheduling support is incomplete and the process will be resumed before combinational logic evaluation. +%Error: t/t_timing_zerodly_unsup.v:23: Verilog $stop +Aborting... diff --git a/test_regress/t/t_timing_zerodly_unsup.pl b/test_regress/t/t_timing_zerodly_unsup.pl index 8ab3c0995..2d852adb1 100755 --- a/test_regress/t/t_timing_zerodly_unsup.pl +++ b/test_regress/t/t_timing_zerodly_unsup.pl @@ -8,10 +8,14 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di # Version 2.0. # SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 -scenarios(linter => 1); +scenarios(simulator => 1); -lint( - verilator_flags2 => ["--timing"], +compile( + verilator_flags2 => ["--exe --main --timing"], + make_main => 0, + ); + +execute( fails => 1, expect_filename => $Self->{golden_filename}, ); diff --git a/test_regress/t/t_timing_zerodly_unsup.v b/test_regress/t/t_timing_zerodly_unsup.v index 7fba70252..5c10fa12e 100644 --- a/test_regress/t/t_timing_zerodly_unsup.v +++ b/test_regress/t/t_timing_zerodly_unsup.v @@ -5,14 +5,23 @@ // SPDX-License-Identifier: CC0-1.0 module t; - event e; - logic v = 0; - initial #1 begin - fork - #0 if (v) $finish; - else $stop; - join_none - ->e; - end - initial @e v = 1; + logic v; + int num; + + initial begin + num = 1; + #1; + if (v) $stop; + num = 21; + // Zero delay should postpone the execution and resume it after + // evaluating combinational logic which would update `v`. However, + // currently we can't postpone the resumption in the current timeframe + // past the combinatorial logic evaluation as that is intertwined with + // NBA evaluation and partitioned for multithreading. This causes `v` + // to not have its value updated despite being checked after #0 delay. + #0 if (v) $finish; + $stop; + end + + always_comb v = (num == 21); endmodule