From 56ac054fb2e9842cf88e0e08f9cfa1ea59c9ae38 Mon Sep 17 00:00:00 2001 From: Krzysztof Bieganski Date: Mon, 3 Oct 2022 17:40:30 +0200 Subject: [PATCH] Internals: Refactor verilated_timing.* (#3653). * Put suspended coroutine source location in a separate struct, * Have `dump()` always print, wrap calls in `VL_DEBUG_IF`, * Improve const correctness. --- include/verilated_timing.cpp | 54 ++++++------- include/verilated_timing.h | 150 +++++++++++++++++------------------ 2 files changed, 99 insertions(+), 105 deletions(-) diff --git a/include/verilated_timing.cpp b/include/verilated_timing.cpp index 3282b6b3f..baef60230 100644 --- a/include/verilated_timing.cpp +++ b/include/verilated_timing.cpp @@ -36,8 +36,8 @@ void VlCoroutineHandle::resume() { } #ifdef VL_DEBUG -void VlCoroutineHandle::dump() { - VL_DEBUG_IF(VL_PRINTF("Process waiting at %s:%d\n", m_filename, m_linenum);); +void VlCoroutineHandle::dump() const { + VL_PRINTF("Process waiting at %s:%d\n", m_fileline.filename(), m_fileline.lineno()); } #endif @@ -45,16 +45,15 @@ void VlCoroutineHandle::dump() { // VlDelayScheduler:: Methods #ifdef VL_DEBUG -void VlDelayScheduler::VlDelayedCoroutine::dump() { - VL_DEBUG_IF(VL_DBG_MSGF(" Awaiting time %lu: ", m_timestep);); +void VlDelayScheduler::VlDelayedCoroutine::dump() const { + VL_DBG_MSGF(" Awaiting time %lu: ", m_timestep); m_handle.dump(); } #endif void VlDelayScheduler::resume() { #ifdef VL_DEBUG - dump(); - VL_DEBUG_IF(VL_DBG_MSGF(" Resuming delayed processes\n");); + VL_DEBUG_IF(dump(); VL_DBG_MSGF(" Resuming delayed processes\n");); #endif while (awaitingCurrentTime()) { if (m_queue.front().m_timestep != m_context.time()) { @@ -70,7 +69,7 @@ void VlDelayScheduler::resume() { } } -uint64_t VlDelayScheduler::nextTimeSlot() { +uint64_t VlDelayScheduler::nextTimeSlot() const { if (empty()) { VL_FATAL_MT(__FILE__, __LINE__, "", "%Error: There is no next time slot scheduled"); } @@ -78,12 +77,12 @@ uint64_t VlDelayScheduler::nextTimeSlot() { } #ifdef VL_DEBUG -void VlDelayScheduler::dump() { +void VlDelayScheduler::dump() const { if (m_queue.empty()) { - VL_DEBUG_IF(VL_DBG_MSGF(" No delayed processes:\n");); + VL_DBG_MSGF(" No delayed processes:\n"); } else { - VL_DEBUG_IF(VL_DBG_MSGF(" Delayed processes:\n");); - for (auto& susp : m_queue) susp.dump(); + VL_DBG_MSGF(" Delayed processes:\n"); + for (const auto& susp : m_queue) susp.dump(); } } #endif @@ -93,8 +92,8 @@ void VlDelayScheduler::dump() { void VlTriggerScheduler::resume(const char* eventDescription) { #ifdef VL_DEBUG - dump(eventDescription); - VL_DEBUG_IF(VL_DBG_MSGF(" Resuming processes waiting for %s\n", eventDescription);); + VL_DEBUG_IF(dump(eventDescription); + VL_DBG_MSGF(" Resuming processes waiting for %s\n", eventDescription);); #endif for (auto& susp : m_ready) susp.resume(); m_ready.clear(); @@ -106,7 +105,7 @@ void VlTriggerScheduler::commit(const char* eventDescription) { if (!m_uncommitted.empty()) { VL_DEBUG_IF( VL_DBG_MSGF(" Committing processes waiting for %s:\n", eventDescription); - for (auto& susp + for (const auto& susp : m_uncommitted) { VL_DBG_MSGF(" - "); susp.dump(); @@ -120,26 +119,22 @@ void VlTriggerScheduler::commit(const char* eventDescription) { } #ifdef VL_DEBUG -void VlTriggerScheduler::dump(const char* eventDescription) { +void VlTriggerScheduler::dump(const char* eventDescription) const { if (m_ready.empty()) { - VL_DEBUG_IF( - VL_DBG_MSGF(" No ready processes waiting for %s\n", eventDescription);); + VL_DBG_MSGF(" No ready processes waiting for %s\n", eventDescription); } else { - VL_DEBUG_IF(for (auto& susp - : m_ready) { + for (const auto& susp : m_ready) { VL_DBG_MSGF(" Ready processes waiting for %s:\n", eventDescription); VL_DBG_MSGF(" - "); susp.dump(); - }); + } } if (!m_uncommitted.empty()) { - VL_DEBUG_IF( - VL_DBG_MSGF(" Uncommitted processes waiting for %s:\n", eventDescription); - for (auto& susp - : m_uncommitted) { - VL_DBG_MSGF(" - "); - susp.dump(); - }); + VL_DBG_MSGF(" Uncommitted processes waiting for %s:\n", eventDescription); + for (const auto& susp : m_uncommitted) { + VL_DBG_MSGF(" - "); + susp.dump(); + } } } #endif @@ -147,9 +142,8 @@ void VlTriggerScheduler::dump(const char* eventDescription) { //====================================================================== // VlForkSync:: Methods -void VlForkSync::done(const char* filename, int linenum) { - VL_DEBUG_IF( - VL_DBG_MSGF(" Process forked at %s:%d finished\n", filename, linenum);); +void VlForkSync::done(const char* filename, int lineno) { + VL_DEBUG_IF(VL_DBG_MSGF(" Process forked at %s:%d finished\n", filename, lineno);); if (m_join->m_counter > 0) m_join->m_counter--; if (m_join->m_counter == 0) m_join->m_susp.resume(); } diff --git a/include/verilated_timing.h b/include/verilated_timing.h index d4f4f7cb3..c07daa7af 100644 --- a/include/verilated_timing.h +++ b/include/verilated_timing.h @@ -56,6 +56,36 @@ #endif // clang-format on +//============================================================================= +// VlFileLineDebug stores a SystemVerilog source code location. Used in VlCoroutineHandle for +// debugging purposes. + +class VlFileLineDebug final { + // MEMBERS +#ifdef VL_DEBUG + const char* m_filename = nullptr; + int m_lineno = 0; +#endif + +public: + // CONSTRUCTORS + // Construct + VlFileLineDebug() = default; + VlFileLineDebug(const char* filename, int lineno) +#ifdef VL_DEBUG + : m_filename{filename} + , m_lineno{lineno} +#endif + { + } + + // METHODS +#ifdef VL_DEBUG + const char* filename() const { return m_filename; } + int lineno() const { return m_lineno; } +#endif +}; + //============================================================================= // VlCoroutineHandle is a non-copyable (but movable) coroutine handle. On resume, the handle is // cleared, as we assume that either the coroutine has finished and deleted itself, or, if it got @@ -66,32 +96,20 @@ class VlCoroutineHandle final { // MEMBERS std::coroutine_handle<> m_coro; // The wrapped coroutine handle -#ifdef VL_DEBUG - const char* m_filename; - int m_linenum; -#endif + VlFileLineDebug m_fileline; public: // CONSTRUCTORS // Construct - VlCoroutineHandle(std::coroutine_handle<> coro = nullptr, const char* filename = nullptr, - int linenum = 0) + VlCoroutineHandle() + : m_coro{nullptr} {} + VlCoroutineHandle(std::coroutine_handle<> coro, VlFileLineDebug fileline) : m_coro{coro} -#ifdef VL_DEBUG - , m_filename{filename} - , m_linenum{linenum} -#endif - { - } + , m_fileline{fileline} {} // Move the handle, leaving a nullptr VlCoroutineHandle(VlCoroutineHandle&& moved) : m_coro{std::exchange(moved.m_coro, nullptr)} -#ifdef VL_DEBUG - , m_filename{moved.m_filename} - , m_linenum{moved.m_linenum} -#endif - { - } + , m_fileline{moved.m_fileline} {} // Destroy if the handle isn't null ~VlCoroutineHandle() { // Usually these coroutines should get resumed; we only need to clean up if we destroy a @@ -107,7 +125,7 @@ public: // Resume the coroutine if the handle isn't null void resume(); #ifdef VL_DEBUG - void dump(); + void dump() const; #endif }; @@ -126,7 +144,7 @@ class VlDelayScheduler final { return m_timestep > other.m_timestep; } #ifdef VL_DEBUG - void dump(); + void dump() const; #endif }; using VlDelayedCoroutineQueue = std::vector; @@ -144,42 +162,32 @@ public: void resume(); // Returns the simulation time of the next time slot (aborts if there are no delayed // coroutines) - uint64_t nextTimeSlot(); + uint64_t nextTimeSlot() const; // Are there no delayed coroutines awaiting? - bool empty() { return m_queue.empty(); } + bool empty() const { return m_queue.empty(); } // Are there coroutines to resume at the current simulation time? - bool awaitingCurrentTime() { + bool awaitingCurrentTime() const { return !empty() && m_queue.front().m_timestep <= m_context.time(); } #ifdef VL_DEBUG - void dump(); + void dump() const; #endif // Used by coroutines for co_awaiting a certain simulation time - auto delay(uint64_t delay, const char* filename, int linenum) { + auto delay(uint64_t delay, const char* filename, int lineno) { struct Awaitable { VlDelayedCoroutineQueue& queue; uint64_t delay; -#ifdef VL_DEBUG - const char* filename; - int linenum; -#endif - bool await_ready() { return false; } // Always suspend + VlFileLineDebug fileline; + + bool await_ready() const { return false; } // Always suspend void await_suspend(std::coroutine_handle<> coro) { -#ifdef VL_DEBUG - queue.push_back({delay, VlCoroutineHandle{coro, filename, linenum}}); -#else - queue.push_back({delay, coro}); -#endif + queue.push_back({delay, VlCoroutineHandle{coro, fileline}}); // Move last element to the proper place in the max-heap std::push_heap(queue.begin(), queue.end()); } - void await_resume() {} + void await_resume() const {} }; -#ifdef VL_DEBUG - return Awaitable{m_queue, m_context.time() + delay, filename, linenum}; -#else - return Awaitable{m_queue, m_context.time() + delay}; -#endif + return Awaitable{m_queue, m_context.time() + delay, VlFileLineDebug{filename, lineno}}; } }; @@ -207,35 +215,25 @@ public: // Moves all coroutines from m_uncommitted to m_ready void commit(const char* eventDescription); // Are there no coroutines awaiting? - bool empty() { return m_ready.empty() && m_uncommitted.empty(); } + bool empty() const { return m_ready.empty() && m_uncommitted.empty(); } #ifdef VL_DEBUG - void dump(const char* eventDescription); + void dump(const char* eventDescription) const; #endif // Used by coroutines for co_awaiting a certain trigger - auto trigger(const char* eventDescription, const char* filename, int linenum) { + auto trigger(const char* eventDescription, const char* filename, int lineno) { VL_DEBUG_IF(VL_DBG_MSGF(" Suspending process waiting for %s at %s:%d\n", - eventDescription, filename, linenum);); + eventDescription, filename, lineno);); struct Awaitable { VlCoroutineVec& suspended; // Coros waiting on trigger -#ifdef VL_DEBUG - const char* filename; - int linenum; -#endif - bool await_ready() { return false; } // Always suspend + VlFileLineDebug fileline; + + bool await_ready() const { return false; } // Always suspend void await_suspend(std::coroutine_handle<> coro) { -#ifdef VL_DEBUG - suspended.emplace_back(coro, filename, linenum); -#else - suspended.emplace_back(coro); -#endif + suspended.emplace_back(coro, fileline); } - void await_resume() {} + void await_resume() const {} }; -#ifdef VL_DEBUG - return Awaitable{m_uncommitted, filename, linenum}; -#else - return Awaitable{m_uncommitted}; -#endif + return Awaitable{m_uncommitted, VlFileLineDebug{filename, lineno}}; } }; @@ -244,9 +242,9 @@ public: // Allows forcing the move of coroutine locals to the heap. struct VlNow { - bool await_ready() { return false; } // Always suspend - bool await_suspend(std::coroutine_handle<>) { return false; } // Resume immediately - void await_resume() {} + bool await_ready() const { return false; } // Always suspend + bool await_suspend(std::coroutine_handle<>) const { return false; } // Resume immediately + void await_resume() const {} }; //============================================================================= @@ -254,9 +252,9 @@ struct VlNow { // wait statements. struct VlForever { - bool await_ready() { return false; } // Always suspend - void await_suspend(std::coroutine_handle<> coro) { coro.destroy(); } - void await_resume() {} + bool await_ready() const { return false; } // Always suspend + void await_suspend(std::coroutine_handle<> coro) const { coro.destroy(); } + void await_resume() const {} }; //============================================================================= @@ -278,19 +276,21 @@ public: void init(size_t count) { m_join.reset(new VlJoin{count, {}}); } // Called whenever any of the forked processes finishes. If the join counter reaches 0, the // main process gets resumed - void done(const char* filename, int linenum); + void done(const char* filename, int lineno); // Used by coroutines for co_awaiting a join - auto join(const char* filename, int linenum) { + auto join(const char* filename, int lineno) { assert(m_join); VL_DEBUG_IF( - VL_DBG_MSGF(" Awaiting join of fork at: %s:%d\n", filename, linenum);); + VL_DBG_MSGF(" Awaiting join of fork at: %s:%d\n", filename, lineno);); struct Awaitable { const std::shared_ptr join; // Join to await on + VlFileLineDebug fileline; + bool await_ready() { return join->m_counter == 0; } // Suspend if join still exists - void await_suspend(std::coroutine_handle<> coro) { join->m_susp = coro; } - void await_resume() {} + void await_suspend(std::coroutine_handle<> coro) { join->m_susp = {coro, fileline}; } + void await_resume() const {} }; - return Awaitable{m_join}; + return Awaitable{m_join, VlFileLineDebug{filename, lineno}}; } }; @@ -310,13 +310,13 @@ private: VlCoroutine get_return_object() { return {this}; } // Never suspend at the start of the coroutine - std::suspend_never initial_suspend() { return {}; } + std::suspend_never initial_suspend() const { return {}; } // Never suspend at the end of the coroutine (thanks to this, the coroutine will clean up // after itself) std::suspend_never final_suspend() noexcept; - void unhandled_exception() { std::abort(); } + void unhandled_exception() const { std::abort(); } void return_void() const {} };