From e52f20c13245a2287232d1da9dd542bf2a0857e4 Mon Sep 17 00:00:00 2001 From: Mark Burton Date: Tue, 19 Nov 2024 20:02:29 +0100 Subject: [PATCH 1/3] Fix mutexes in sc_prim_channel Signed-off-by: Mark Burton --- src/sysc/communication/sc_prim_channel.cpp | 5 ++--- tests/systemc/kernel/sc_suspend/golden/sc_suspend.log | 2 -- tests/systemc/kernel/sc_suspend/sc_suspend.cpp | 1 - 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/sysc/communication/sc_prim_channel.cpp b/src/sysc/communication/sc_prim_channel.cpp index de73f0f5f..88afac4b6 100644 --- a/src/sysc/communication/sc_prim_channel.cpp +++ b/src/sysc/communication/sc_prim_channel.cpp @@ -150,8 +150,9 @@ class sc_prim_channel_registry::async_update_list { public: - bool pending() const + bool pending() { + sc_scoped_lock lock( m_mutex ); return m_push_queue.size() != 0; } @@ -195,7 +196,6 @@ class sc_prim_channel_registry::async_update_list void attach_suspending( sc_prim_channel& p ) { - sc_scoped_lock lock( m_mutex ); std::vector::iterator it = std::find(m_suspending_channels.begin(), m_suspending_channels.end(), &p); if ( it == m_suspending_channels.end() ) { @@ -207,7 +207,6 @@ class sc_prim_channel_registry::async_update_list void detach_suspending( sc_prim_channel& p ) { - sc_scoped_lock lock( m_mutex ); std::vector::iterator it = std::find(m_suspending_channels.begin(), m_suspending_channels.end(), &p); if ( it != m_suspending_channels.end() ) { diff --git a/tests/systemc/kernel/sc_suspend/golden/sc_suspend.log b/tests/systemc/kernel/sc_suspend/golden/sc_suspend.log index 7e5d7f410..208fc7371 100644 --- a/tests/systemc/kernel/sc_suspend/golden/sc_suspend.log +++ b/tests/systemc/kernel/sc_suspend/golden/sc_suspend.log @@ -10,8 +10,6 @@ Info: tester: Suspend at SystemC time: 350 us Info: async_event: Called stage_callback at stage: SC_PRE_SUSPEND -Info: child thread: Release child thread - Info: tester: Restart SystemC time: 350 us Info: tester: Start to advance SystemC time: 350 us diff --git a/tests/systemc/kernel/sc_suspend/sc_suspend.cpp b/tests/systemc/kernel/sc_suspend/sc_suspend.cpp index c1a0dab2e..2a8b39137 100644 --- a/tests/systemc/kernel/sc_suspend/sc_suspend.cpp +++ b/tests/systemc/kernel/sc_suspend/sc_suspend.cpp @@ -126,7 +126,6 @@ SC_MODULE (tester) { t([&] { std::unique_lock lock(mutex); condition.wait(lock); - SC_REPORT_INFO("child thread", "Release child thread"); start.notify(); }), start(false) From 946dccf85040a4450df3fa6f9f2b68fe508db9ab Mon Sep 17 00:00:00 2001 From: Mark Burton Date: Wed, 20 Nov 2024 14:28:02 +0100 Subject: [PATCH 2/3] fix race condition in sc_suspend test Align async_event example code with the scp_async_event as it's cleaner Prevent race condition on startup in sc_suspend test Signed-off-by: Mark Burton --- tests/CMakeLists.txt | 5 -- .../systemc/kernel/sc_suspend/sc_suspend.cpp | 65 +++++++++++++------ 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 220d52f09..2966a4bef 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -128,8 +128,3 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") ) endif() -# Skip some test execution under some conditions -if(APPLE OR HAS__aarch64__DEFINED) - # TODO: Test hangs frequently on macOS and/or ARM 64-bit setups - skip_test(systemc/kernel/sc_suspend) -endif() diff --git a/tests/systemc/kernel/sc_suspend/sc_suspend.cpp b/tests/systemc/kernel/sc_suspend/sc_suspend.cpp index 2a8b39137..1d8dab5c5 100644 --- a/tests/systemc/kernel/sc_suspend/sc_suspend.cpp +++ b/tests/systemc/kernel/sc_suspend/sc_suspend.cpp @@ -41,62 +41,85 @@ #include #include -class async_event : public sc_core::sc_prim_channel, public sc_core::sc_event, public sc_core::sc_stage_callback_if +class async_event : public sc_core::sc_prim_channel, + public sc_core::sc_event, + public sc_core::sc_stage_callback_if { private: - int outstanding; sc_core::sc_time m_delay; std::thread::id tid; std::mutex mutex; // Belt and braces + bool outstanding=false; public: - async_event(bool start_attached = true): outstanding(0) - { + async_event(bool start_attached = true) { sc_core::sc_register_stage_callback(*this, sc_core::SC_PRE_PAUSE | sc_core::SC_PRE_SUSPEND | sc_core::SC_POST_SUSPEND); tid = std::this_thread::get_id(); - outstanding = 0; enable_attach_suspending(start_attached); } void async_notify() { notify(); } - void notify(sc_core::sc_time delay = sc_core::sc_time(sc_core::SC_ZERO_TIME)) - { + void notify() { + if (tid == std::this_thread::get_id()) { + sc_core::sc_event::notify(); + } else { + notify(sc_core::SC_ZERO_TIME); + } + } + void notify(double d, sc_core::sc_time_unit u) { + notify(sc_core::sc_time(d, u)); + } + void notify(sc_core::sc_time delay) { if (tid == std::this_thread::get_id()) { sc_core::sc_event::notify(delay); } else { mutex.lock(); m_delay = delay; - outstanding++; + outstanding = true; mutex.unlock(); async_request_update(); } } - void async_attach_suspending() { this->sc_core::sc_prim_channel::async_attach_suspending(); } + bool triggered() const { + if (tid == std::this_thread::get_id()) { + return sc_core::sc_event::triggered(); + } else { + SC_REPORT_ERROR("async_event", + "It is an error to call triggered() from outside " + "the SystemC thread"); + } + return false; + } + void async_attach_suspending() { + this->sc_core::sc_prim_channel::async_attach_suspending(); + } - void async_detach_suspending() { this->sc_core::sc_prim_channel::async_detach_suspending(); } + void async_detach_suspending() { + this->sc_core::sc_prim_channel::async_detach_suspending(); + } - void enable_attach_suspending(bool e) - { - mutex.lock(); + void enable_attach_suspending(bool e) { e ? this->async_attach_suspending() : this->async_detach_suspending(); - mutex.unlock(); } private: - void update(void) - { + void update(void) { mutex.lock(); // we should be in SystemC thread if (outstanding) { sc_event::notify(m_delay); - outstanding--; - if (outstanding) request_update(); + outstanding = false; } mutex.unlock(); } - + void start_of_simulation() { + // we should be in SystemC thread + if (outstanding) { + request_update(); + } + } void stage_callback(const sc_core::sc_stage& stage) { std::ostringstream stage_str; @@ -116,6 +139,7 @@ class async_event : public sc_core::sc_prim_channel, public sc_core::sc_event, p }; SC_MODULE (tester) { + bool released = false; std::mutex mutex; std::condition_variable condition; std::thread t; @@ -125,7 +149,7 @@ SC_MODULE (tester) { SC_CTOR (tester) : t([&] { std::unique_lock lock(mutex); - condition.wait(lock); + condition.wait(lock, [&](){return released;}); start.notify(); }), start(false) @@ -156,6 +180,7 @@ SC_MODULE (tester) { // triggure the other thread to release us ! std::lock_guard lock(mutex); + released = true; condition.notify_one(); } else { SC_REPORT_INFO("tester", ("Unsuspend at SystemC time: " + sc_core::sc_time_stamp().to_string()).c_str()); From 29717f292d56cf9b3208f673bd983f9712a073ad Mon Sep 17 00:00:00 2001 From: Mark Burton Date: Fri, 22 Nov 2024 15:41:16 +0100 Subject: [PATCH 3/3] Remove ambiguity and TSAN errors in async_suspend example Signed-off-by: Mark Burton --- examples/sysc/async_suspend/collector.h | 1 + examples/sysc/async_suspend/node.h | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/examples/sysc/async_suspend/collector.h b/examples/sysc/async_suspend/collector.h index 221aa8348..7e71c6fb3 100644 --- a/examples/sysc/async_suspend/collector.h +++ b/examples/sysc/async_suspend/collector.h @@ -54,6 +54,7 @@ class collector void csvreport() { + std::lock_guard guard(lock); cout << "event"; for (auto kv : names) { diff --git a/examples/sysc/async_suspend/node.h b/examples/sysc/async_suspend/node.h index d2979289d..419c882eb 100644 --- a/examples/sysc/async_suspend/node.h +++ b/examples/sysc/async_suspend/node.h @@ -36,6 +36,7 @@ #include #include #include +#include #include "tlm.h" #include "tlm_utils/simple_initiator_socket.h" @@ -110,8 +111,8 @@ SC_MODULE (asynctestnode) collector &col; // just used to print out the results - bool running; - bool finished; + std::atomic running; + std::atomic finished; asynctestnode(sc_core::sc_module_name name, collector &c) : init_socket("output"),