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

@@ -36,8 +36,8 @@ void VlCoroutineHandle::resume() {
 }
 
 #ifdef VL_DEBUG
-void VlCoroutineHandle::dump() {
+void VlCoroutineHandle::dump() const {
-    VL_DEBUG_IF(VL_PRINTF("Process waiting at %s:%d\n", m_filename, m_linenum););
+    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() {
+void VlDelayScheduler::VlDelayedCoroutine::dump() const {
-    VL_DEBUG_IF(VL_DBG_MSGF("             Awaiting time %lu: ", m_timestep););
+    VL_DBG_MSGF("             Awaiting time %lu: ", m_timestep);
     m_handle.dump();
 }
 #endif
 
 void VlDelayScheduler::resume() {
 #ifdef VL_DEBUG
-    dump();
+    VL_DEBUG_IF(dump(); VL_DBG_MSGF("         Resuming delayed processes\n"););
-    VL_DEBUG_IF(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"););
+        VL_DBG_MSGF("         Delayed processes:\n");
-        for (auto& susp : m_queue) susp.dump();
+        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(dump(eventDescription);
-    VL_DEBUG_IF(VL_DBG_MSGF("         Resuming processes waiting for %s\n", 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
+        for (const auto& susp : m_ready) {
-                         : 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);
-            VL_DBG_MSGF("         Uncommitted processes waiting for %s:\n", eventDescription);
+        for (const auto& susp : m_uncommitted) {
-            for (auto& susp
+            VL_DBG_MSGF("           - ");
-                 : m_uncommitted) {
+            susp.dump();
-                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) {
+void VlForkSync::done(const char* filename, int lineno) {
-    VL_DEBUG_IF(
+    VL_DEBUG_IF(VL_DBG_MSGF("             Process forked at %s:%d finished\n", filename, lineno););
-        VL_DBG_MSGF("             Process forked at %s:%d finished\n", filename, linenum););
     if (m_join->m_counter > 0) m_join->m_counter--;
     if (m_join->m_counter == 0) m_join->m_susp.resume();
 }

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
+    VlFileLineDebug m_fileline;
-    const char* m_filename;
-    int m_linenum;
-#endif
 
 public:
     // CONSTRUCTORS
     // Construct
-    VlCoroutineHandle(std::coroutine_handle<> coro = nullptr, const char* filename = nullptr,
+    VlCoroutineHandle()
-                      int linenum = 0)
+        : m_coro{nullptr} {}
+    VlCoroutineHandle(std::coroutine_handle<> coro, VlFileLineDebug fileline)
         : m_coro{coro}
-#ifdef VL_DEBUG
+        , m_fileline{fileline} {}
-        , m_filename{filename}
-        , m_linenum{linenum}
-#endif
-    {
-    }
     // Move the handle, leaving a nullptr
     VlCoroutineHandle(VlCoroutineHandle&& moved)
         : m_coro{std::exchange(moved.m_coro, nullptr)}
-#ifdef VL_DEBUG
+        , m_fileline{moved.m_fileline} {}
-        , m_filename{moved.m_filename}
-        , m_linenum{moved.m_linenum}
-#endif
-    {
-    }
     // 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<VlDelayedCoroutine>;
@@ -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
+            VlFileLineDebug fileline;
-            const char* filename;
+
-            int linenum;
+            bool await_ready() const { return false; }  // Always suspend
-#endif
-            bool await_ready() { return false; }  // Always suspend
             void await_suspend(std::coroutine_handle<> coro) {
-#ifdef VL_DEBUG
+                queue.push_back({delay, VlCoroutineHandle{coro, fileline}});
-                queue.push_back({delay, VlCoroutineHandle{coro, filename, linenum}});
-#else
-                queue.push_back({delay, coro});
-#endif
                 // 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, VlFileLineDebug{filename, lineno}};
-        return Awaitable{m_queue, m_context.time() + delay, filename, linenum};
-#else
-        return Awaitable{m_queue, m_context.time() + delay};
-#endif
     }
 };
 
@@ -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
+            VlFileLineDebug fileline;
-            const char* filename;
+
-            int linenum;
+            bool await_ready() const { return false; }  // Always suspend
-#endif
-            bool await_ready() { return false; }  // Always suspend
             void await_suspend(std::coroutine_handle<> coro) {
-#ifdef VL_DEBUG
+                suspended.emplace_back(coro, fileline);
-                suspended.emplace_back(coro, filename, linenum);
-#else
-                suspended.emplace_back(coro);
-#endif
             }
-            void await_resume() {}
+            void await_resume() const {}
         };
-#ifdef VL_DEBUG
+        return Awaitable{m_uncommitted, VlFileLineDebug{filename, lineno}};
-        return Awaitable{m_uncommitted, filename, linenum};
-#else
-        return Awaitable{m_uncommitted};
-#endif
     }
 };
 
@@ -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_ready() const { return false; }  // Always suspend
-    bool await_suspend(std::coroutine_handle<>) { return false; }  // Resume immediately
+    bool await_suspend(std::coroutine_handle<>) const { return false; }  // Resume immediately
-    void await_resume() {}
+    void await_resume() const {}
 };
 
 //=============================================================================
@@ -254,9 +252,9 @@ struct VlNow {
 // wait statements.
 
 struct VlForever {
-    bool await_ready() { return false; }  // Always suspend
+    bool await_ready() const { return false; }  // Always suspend
-    void await_suspend(std::coroutine_handle<> coro) { coro.destroy(); }
+    void await_suspend(std::coroutine_handle<> coro) const { coro.destroy(); }
-    void await_resume() {}
+    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<VlJoin> 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_suspend(std::coroutine_handle<> coro) { join->m_susp = {coro, fileline}; }
-            void await_resume() {}
+            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 {}
     };