From 38038143eb497533933955bfe0eb0b43c9a1db93 Mon Sep 17 00:00:00 2001 From: cpplearner Date: Fri, 23 Jun 2023 17:17:55 +0800 Subject: [PATCH 1/7] constexpr mutex --- stl/inc/mutex | 52 ++++--------- stl/inc/xthreads.h | 52 +++++++++---- stl/src/cond.cpp | 4 +- stl/src/mutex.cpp | 89 ++++++++++------------ stl/src/primitives.hpp | 44 ++++++----- tests/std/tests/VSO_0226079_mutex/env.lst | 3 + tests/std/tests/VSO_0226079_mutex/test.cpp | 9 +++ 7 files changed, 128 insertions(+), 125 deletions(-) diff --git a/stl/inc/mutex b/stl/inc/mutex index a17c022650..7c697875ec 100644 --- a/stl/inc/mutex +++ b/stl/inc/mutex @@ -31,37 +31,20 @@ _STD_BEGIN _EXPORT_STD class condition_variable; _EXPORT_STD class condition_variable_any; -struct _Mtx_internal_imp_mirror { -#ifdef _CRT_WINDOWS -#ifdef _WIN64 - static constexpr size_t _Critical_section_size = 16; -#else // _WIN64 - static constexpr size_t _Critical_section_size = 8; -#endif // _WIN64 -#else // _CRT_WINDOWS -#ifdef _WIN64 - static constexpr size_t _Critical_section_size = 64; -#else // _WIN64 - static constexpr size_t _Critical_section_size = 36; -#endif // _WIN64 -#endif // _CRT_WINDOWS - - static constexpr size_t _Critical_section_align = alignof(void*); - - int _Type; - _Aligned_storage_t<_Critical_section_size, _Critical_section_align> _Cs_storage; - long _Thread_id; - int _Count; -}; - -static_assert(sizeof(_Mtx_internal_imp_mirror) == _Mtx_internal_imp_size, "inconsistent size for mutex"); -static_assert(alignof(_Mtx_internal_imp_mirror) == _Mtx_internal_imp_alignment, "inconsistent alignment for mutex"); - class _Mutex_base { // base class for all mutex types public: +#ifdef _USE_CONSTEXPR_MUTEX_CONSTRUCTOR + constexpr _Mutex_base(int _Flags = 0) noexcept { + _Mtx_storage._Critical_section = {}; + _Mtx_storage._Thread_id = -1; + _Mtx_storage._Type = _Flags | _Mtx_try; + _Mtx_storage._Count = 0; + } +#else // ^^^ _USE_CONSTEXPR_MUTEX_CONSTRUCTOR / !_USE_CONSTEXPR_MUTEX_CONSTRUCTOR vvv _Mutex_base(int _Flags = 0) noexcept { _Mtx_init_in_situ(_Mymtx(), _Flags | _Mtx_try); } +#endif // !_USE_CONSTEXPR_MUTEX_CONSTRUCTOR ~_Mutex_base() noexcept { _Mtx_destroy_in_situ(_Mymtx()); @@ -97,9 +80,9 @@ public: protected: _NODISCARD_TRY_CHANGE_STATE bool _Verify_ownership_levels() noexcept { - if (_Mtx_storage_mirror._Count == INT_MAX) { + if (_Mtx_storage._Count == INT_MAX) { // only occurs for recursive mutexes (N4950 [thread.mutex.recursive]/3) - --_Mtx_storage_mirror._Count; + --_Mtx_storage._Count; return false; } @@ -110,23 +93,16 @@ private: friend condition_variable; friend condition_variable_any; - union { - _Aligned_storage_t<_Mtx_internal_imp_size, _Mtx_internal_imp_alignment> _Mtx_storage; - _Mtx_internal_imp_mirror _Mtx_storage_mirror; - }; + _Mtx_internal_imp_t _Mtx_storage; _Mtx_t _Mymtx() noexcept { // get pointer to _Mtx_internal_imp_t inside _Mtx_storage - return reinterpret_cast<_Mtx_t>(&_Mtx_storage); + return &_Mtx_storage; } }; -static_assert(sizeof(_Mutex_base) == _Mtx_internal_imp_size, "inconsistent size for mutex"); -static_assert(alignof(_Mutex_base) == _Mtx_internal_imp_alignment, "inconsistent alignment for mutex"); - _EXPORT_STD class mutex : public _Mutex_base { // class for mutual exclusion public: - /* constexpr */ mutex() noexcept // TRANSITION, ABI - : _Mutex_base() {} + mutex() noexcept = default; mutex(const mutex&) = delete; mutex& operator=(const mutex&) = delete; diff --git a/stl/inc/xthreads.h b/stl/inc/xthreads.h index 71503026c2..94fb11bbb7 100644 --- a/stl/inc/xthreads.h +++ b/stl/inc/xthreads.h @@ -9,6 +9,7 @@ #include #if _STL_COMPILER_PREPROCESSOR #include +#include #include #pragma pack(push, _CRT_PACKING) @@ -25,40 +26,64 @@ struct _Thrd_t { // thread identifier for Win32 _Thrd_id_t _Id; }; -// Size and alignment for _Mtx_internal_imp_t and _Cnd_internal_imp_t +using _Smtx_t = void*; + +struct _Stl_critical_section { + void* _Unused = nullptr; + _Smtx_t _M_srw_lock = nullptr; +}; + +struct _Mtx_internal_imp_t { +#ifdef _CRT_WINDOWS +#ifdef _WIN64 + static constexpr size_t _Critical_section_size = 16; +#else // _WIN64 + static constexpr size_t _Critical_section_size = 8; +#endif // _WIN64 +#else // _CRT_WINDOWS +#ifdef _WIN64 + static constexpr size_t _Critical_section_size = 64; +#else // _WIN64 + static constexpr size_t _Critical_section_size = 36; +#endif // _WIN64 +#endif // _CRT_WINDOWS + + static constexpr size_t _Critical_section_align = alignof(void*); + + int _Type{}; + union { + _Stl_critical_section _Critical_section{}; + _STD _Aligned_storage_t<_Critical_section_size, _Critical_section_align> _Cs_storage; + }; + long _Thread_id{}; + int _Count{}; +}; + +// Size and alignment for _Cnd_internal_imp_t #ifdef _CRT_WINDOWS #ifdef _WIN64 -_INLINE_VAR constexpr size_t _Mtx_internal_imp_size = 32; -_INLINE_VAR constexpr size_t _Mtx_internal_imp_alignment = 8; _INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 16; _INLINE_VAR constexpr size_t _Cnd_internal_imp_alignment = 8; #else // _WIN64 -_INLINE_VAR constexpr size_t _Mtx_internal_imp_size = 20; -_INLINE_VAR constexpr size_t _Mtx_internal_imp_alignment = 4; _INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 8; _INLINE_VAR constexpr size_t _Cnd_internal_imp_alignment = 4; #endif // _WIN64 #else // _CRT_WINDOWS #ifdef _WIN64 -_INLINE_VAR constexpr size_t _Mtx_internal_imp_size = 80; -_INLINE_VAR constexpr size_t _Mtx_internal_imp_alignment = 8; _INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 72; _INLINE_VAR constexpr size_t _Cnd_internal_imp_alignment = 8; #else // _WIN64 -_INLINE_VAR constexpr size_t _Mtx_internal_imp_size = 48; -_INLINE_VAR constexpr size_t _Mtx_internal_imp_alignment = 4; _INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 40; _INLINE_VAR constexpr size_t _Cnd_internal_imp_alignment = 4; #endif // _WIN64 #endif // _CRT_WINDOWS -#ifdef _M_CEE // avoid warning LNK4248: unresolved typeref token for '_Mtx_internal_imp_t'; image may not run -using _Mtx_t = void*; +using _Mtx_t = _Mtx_internal_imp_t*; + +#ifdef _M_CEE // avoid warning LNK4248: unresolved typeref token for '_Cnd_internal_imp_t'; image may not run using _Cnd_t = void*; #else // ^^^ defined(_M_CEE) / !defined(_M_CEE) vvv -struct _Mtx_internal_imp_t; struct _Cnd_internal_imp_t; -using _Mtx_t = _Mtx_internal_imp_t*; using _Cnd_t = _Cnd_internal_imp_t*; #endif // ^^^ !defined(_M_CEE) ^^^ @@ -96,7 +121,6 @@ _CRTIMP2_PURE void __cdecl _Mtx_reset_owner(_Mtx_t); // shared mutex // these declarations must be in sync with those in sharedmutex.cpp -using _Smtx_t = void*; void __cdecl _Smtx_lock_exclusive(_Smtx_t*); void __cdecl _Smtx_lock_shared(_Smtx_t*); int __cdecl _Smtx_try_lock_exclusive(_Smtx_t*); diff --git a/stl/src/cond.cpp b/stl/src/cond.cpp index 153b59974e..897ff88c2e 100644 --- a/stl/src/cond.cpp +++ b/stl/src/cond.cpp @@ -51,7 +51,7 @@ void _Cnd_destroy(const _Cnd_t cond) { // clean up } int _Cnd_wait(const _Cnd_t cond, const _Mtx_t mtx) { // wait until signaled - const auto cs = static_cast(_Mtx_getconcrtcs(mtx)); + const auto cs = &mtx->_Critical_section; _Mtx_clear_owner(mtx); cond->_get_cv()->wait(cs); _Mtx_reset_owner(mtx); @@ -61,7 +61,7 @@ int _Cnd_wait(const _Cnd_t cond, const _Mtx_t mtx) { // wait until signaled // wait until signaled or timeout int _Cnd_timedwait(const _Cnd_t cond, const _Mtx_t mtx, const _timespec64* const target) { int res = _Thrd_success; - const auto cs = static_cast(_Mtx_getconcrtcs(mtx)); + const auto cs = &mtx->_Critical_section; if (target == nullptr) { // no target time specified, wait on mutex _Mtx_clear_owner(mtx); cond->_get_cv()->wait(cs); diff --git a/stl/src/mutex.cpp b/stl/src/mutex.cpp index 6deb0e8ebd..123e8051a7 100644 --- a/stl/src/mutex.cpp +++ b/stl/src/mutex.cpp @@ -37,34 +37,20 @@ extern "C" _CRTIMP2_PURE void _Thrd_abort(const char* msg) { // abort on precond enum class __stl_sync_api_modes_enum { normal, win7, vista, concrt }; extern "C" _CRTIMP2 void __cdecl __set_stl_sync_api_mode(__stl_sync_api_modes_enum) {} -struct _Mtx_internal_imp_t { // ConcRT mutex - int type; - typename std::_Aligned_storage::type cs; - long thread_id; - int count; - [[nodiscard]] Concurrency::details::stl_critical_section_win7* _get_cs() { // get pointer to implementation - return reinterpret_cast(&cs); - } -}; - -static_assert(sizeof(_Mtx_internal_imp_t) == _Mtx_internal_imp_size, "incorrect _Mtx_internal_imp_size"); -static_assert(alignof(_Mtx_internal_imp_t) == _Mtx_internal_imp_alignment, "incorrect _Mtx_internal_imp_alignment"); - -static_assert( - std::_Mtx_internal_imp_mirror::_Critical_section_size == Concurrency::details::stl_critical_section_max_size); -static_assert( - std::_Mtx_internal_imp_mirror::_Critical_section_align == Concurrency::details::stl_critical_section_max_alignment); +[[nodiscard]] static PSRWLOCK get_srw_lock(_Mtx_t mtx) { + return reinterpret_cast(&mtx->_Critical_section._M_srw_lock); +} +// TRANSITION, only used when constexpr mutex constructor is not enabled void _Mtx_init_in_situ(_Mtx_t mtx, int type) { // initialize mutex in situ - Concurrency::details::create_stl_critical_section(mtx->_get_cs()); - mtx->thread_id = -1; - mtx->type = type; - mtx->count = 0; + Concurrency::details::create_stl_critical_section(&mtx->_Critical_section); + mtx->_Thread_id = -1; + mtx->_Type = type; + mtx->_Count = 0; } void _Mtx_destroy_in_situ(_Mtx_t mtx) { // destroy mutex in situ - _THREAD_ASSERT(mtx->count == 0, "mutex destroyed while busy"); + _THREAD_ASSERT(mtx->_Count == 0, "mutex destroyed while busy"); (void) mtx; } @@ -91,27 +77,27 @@ void _Mtx_destroy(_Mtx_t mtx) { // destroy mutex } static int mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock mutex - if ((mtx->type & ~_Mtx_recursive) == _Mtx_plain) { // set the lock - if (mtx->thread_id != static_cast(GetCurrentThreadId())) { // not current thread, do lock - mtx->_get_cs()->lock(); - mtx->thread_id = static_cast(GetCurrentThreadId()); + if ((mtx->_Type & ~_Mtx_recursive) == _Mtx_plain) { // set the lock + if (mtx->_Thread_id != static_cast(GetCurrentThreadId())) { // not current thread, do lock + AcquireSRWLockExclusive(get_srw_lock(mtx)); + mtx->_Thread_id = static_cast(GetCurrentThreadId()); } - ++mtx->count; + ++mtx->_Count; return _Thrd_success; } else { // handle timed or recursive mutex int res = WAIT_TIMEOUT; if (target == nullptr) { // no target --> plain wait (i.e. infinite timeout) - if (mtx->thread_id != static_cast(GetCurrentThreadId())) { - mtx->_get_cs()->lock(); + if (mtx->_Thread_id != static_cast(GetCurrentThreadId())) { + AcquireSRWLockExclusive(get_srw_lock(mtx)); } res = WAIT_OBJECT_0; } else if (target->tv_sec < 0 || target->tv_sec == 0 && target->tv_nsec <= 0) { // target time <= 0 --> plain trylock or timed wait for time that has passed; try to lock with 0 timeout - if (mtx->thread_id != static_cast(GetCurrentThreadId())) { // not this thread, lock it - if (mtx->_get_cs()->try_lock()) { + if (mtx->_Thread_id != static_cast(GetCurrentThreadId())) { // not this thread, lock it + if (TryAcquireSRWLockExclusive(get_srw_lock(mtx)) != 0) { res = WAIT_OBJECT_0; } else { res = WAIT_TIMEOUT; @@ -125,8 +111,8 @@ static int mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock mutex _Timespec64_get_sys(&now); while (now.tv_sec < target->tv_sec || now.tv_sec == target->tv_sec && now.tv_nsec < target->tv_nsec) { // time has not expired - if (mtx->thread_id == static_cast(GetCurrentThreadId()) - || mtx->_get_cs()->try_lock()) { // stop waiting + if (mtx->_Thread_id == static_cast(GetCurrentThreadId()) + || TryAcquireSRWLockExclusive(get_srw_lock(mtx)) != 0) { // stop waiting res = WAIT_OBJECT_0; break; } else { @@ -138,13 +124,13 @@ static int mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock mutex } if (res == WAIT_OBJECT_0 || res == WAIT_ABANDONED) { - if (1 < ++mtx->count) { // check count - if ((mtx->type & _Mtx_recursive) != _Mtx_recursive) { // not recursive, fixup count - --mtx->count; + if (1 < ++mtx->_Count) { // check count + if ((mtx->_Type & _Mtx_recursive) != _Mtx_recursive) { // not recursive, fixup count + --mtx->_Count; res = WAIT_TIMEOUT; } } else { - mtx->thread_id = static_cast(GetCurrentThreadId()); + mtx->_Thread_id = static_cast(GetCurrentThreadId()); } } @@ -168,11 +154,14 @@ static int mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock mutex int _Mtx_unlock(_Mtx_t mtx) { // unlock mutex _THREAD_ASSERT( - 1 <= mtx->count && mtx->thread_id == static_cast(GetCurrentThreadId()), "unlock of unowned mutex"); + 1 <= mtx->_Count && mtx->_Thread_id == static_cast(GetCurrentThreadId()), "unlock of unowned mutex"); + + if (--mtx->_Count == 0) { // leave critical section + mtx->_Thread_id = -1; - if (--mtx->count == 0) { // leave critical section - mtx->thread_id = -1; - mtx->_get_cs()->unlock(); + auto srw_lock = get_srw_lock(mtx); + _Analysis_assume_lock_held_(*srw_lock); + ReleaseSRWLockExclusive(srw_lock); } return _Thrd_success; // TRANSITION, ABI: always returns _Thrd_success } @@ -183,7 +172,7 @@ int _Mtx_lock(_Mtx_t mtx) { // lock mutex int _Mtx_trylock(_Mtx_t mtx) { // attempt to lock try_mutex _timespec64 xt; - _THREAD_ASSERT((mtx->type & (_Mtx_try | _Mtx_timed)) != 0, "trylock not supported by mutex"); + _THREAD_ASSERT((mtx->_Type & (_Mtx_try | _Mtx_timed)) != 0, "trylock not supported by mutex"); xt.tv_sec = 0; xt.tv_nsec = 0; return mtx_do_lock(mtx, &xt); @@ -192,27 +181,27 @@ int _Mtx_trylock(_Mtx_t mtx) { // attempt to lock try_mutex int _Mtx_timedlock(_Mtx_t mtx, const _timespec64* xt) { // attempt to lock timed mutex int res; - _THREAD_ASSERT((mtx->type & _Mtx_timed) != 0, "timedlock not supported by mutex"); + _THREAD_ASSERT((mtx->_Type & _Mtx_timed) != 0, "timedlock not supported by mutex"); res = mtx_do_lock(mtx, xt); return res == _Thrd_busy ? _Thrd_timedout : res; } int _Mtx_current_owns(_Mtx_t mtx) { // test if current thread owns mutex - return mtx->count != 0 && mtx->thread_id == static_cast(GetCurrentThreadId()); + return mtx->_Count != 0 && mtx->_Thread_id == static_cast(GetCurrentThreadId()); } void* _Mtx_getconcrtcs(_Mtx_t mtx) { // get internal cs impl - return mtx->_get_cs(); + return &mtx->_Critical_section; } void _Mtx_clear_owner(_Mtx_t mtx) { // set owner to nobody - mtx->thread_id = -1; - --mtx->count; + mtx->_Thread_id = -1; + --mtx->_Count; } void _Mtx_reset_owner(_Mtx_t mtx) { // set owner to current thread - mtx->thread_id = static_cast(GetCurrentThreadId()); - ++mtx->count; + mtx->_Thread_id = static_cast(GetCurrentThreadId()); + ++mtx->_Count; } /* diff --git a/stl/src/primitives.hpp b/stl/src/primitives.hpp index b3d2db3236..a0bad4a3fc 100644 --- a/stl/src/primitives.hpp +++ b/stl/src/primitives.hpp @@ -10,7 +10,8 @@ namespace Concurrency { namespace details { - class stl_critical_section_win7 { + // TRANSITION, only used when constexpr mutex constructor is not enabled + class stl_critical_section_win7 final { public: stl_critical_section_win7() = default; @@ -18,29 +19,30 @@ namespace Concurrency { stl_critical_section_win7(const stl_critical_section_win7&) = delete; stl_critical_section_win7& operator=(const stl_critical_section_win7&) = delete; - void lock() { + virtual void lock() { AcquireSRWLockExclusive(&m_srw_lock); } - bool try_lock() { + virtual bool try_lock() { return TryAcquireSRWLockExclusive(&m_srw_lock) != 0; } - void unlock() { + virtual bool try_lock_for(unsigned int) { + return try_lock(); + } + + virtual void unlock() { _Analysis_assume_lock_held_(m_srw_lock); ReleaseSRWLockExclusive(&m_srw_lock); } - PSRWLOCK native_handle() { - return &m_srw_lock; - } + virtual void destroy() {} private: - void* unused = nullptr; // TRANSITION, ABI: was the vptr SRWLOCK m_srw_lock = SRWLOCK_INIT; }; - class stl_condition_variable_win7 { + class stl_condition_variable_win7 final { public: stl_condition_variable_win7() = default; @@ -48,49 +50,49 @@ namespace Concurrency { stl_condition_variable_win7(const stl_condition_variable_win7&) = delete; stl_condition_variable_win7& operator=(const stl_condition_variable_win7&) = delete; - void wait(stl_critical_section_win7* lock) { + virtual void wait(_Stl_critical_section* lock) { if (!wait_for(lock, INFINITE)) { std::terminate(); } } - bool wait_for(stl_critical_section_win7* lock, unsigned int timeout) { - return SleepConditionVariableSRW(&m_condition_variable, lock->native_handle(), timeout, 0) != 0; + virtual bool wait_for(_Stl_critical_section* lock, unsigned int timeout) { + return SleepConditionVariableSRW( + &m_condition_variable, reinterpret_cast(lock->_M_srw_lock), timeout, 0) + != 0; } - void notify_one() { + virtual void notify_one() { WakeConditionVariable(&m_condition_variable); } - void notify_all() { + virtual void notify_all() { WakeAllConditionVariable(&m_condition_variable); } + virtual void destroy() {} + private: - void* unused = nullptr; // TRANSITION, ABI: was the vptr CONDITION_VARIABLE m_condition_variable = CONDITION_VARIABLE_INIT; }; - inline void create_stl_critical_section(stl_critical_section_win7* p) { + // TRANSITION, only used when constexpr mutex constructor is not enabled + inline void create_stl_critical_section(void* p) { new (p) stl_critical_section_win7; } - inline void create_stl_condition_variable(stl_condition_variable_win7* p) { + inline void create_stl_condition_variable(void* p) { new (p) stl_condition_variable_win7; } #if defined(_CRT_WINDOWS) // for Windows-internal code - const size_t stl_critical_section_max_size = 2 * sizeof(void*); const size_t stl_condition_variable_max_size = 2 * sizeof(void*); #elif defined(_WIN64) // ordinary 64-bit code - const size_t stl_critical_section_max_size = 64; const size_t stl_condition_variable_max_size = 72; #else // vvv ordinary 32-bit code vvv - const size_t stl_critical_section_max_size = 36; const size_t stl_condition_variable_max_size = 40; #endif // ^^^ ordinary 32-bit code ^^^ - const size_t stl_critical_section_max_alignment = alignof(void*); const size_t stl_condition_variable_max_alignment = alignof(void*); } // namespace details } // namespace Concurrency diff --git a/tests/std/tests/VSO_0226079_mutex/env.lst b/tests/std/tests/VSO_0226079_mutex/env.lst index f141421b29..65dc71d458 100644 --- a/tests/std/tests/VSO_0226079_mutex/env.lst +++ b/tests/std/tests/VSO_0226079_mutex/env.lst @@ -2,3 +2,6 @@ # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception RUNALL_INCLUDE ..\impure_matrix.lst +RUNALL_CROSSLIST +PM_CL="/D_USE_CONSTEXPR_MUTEX_CONSTRUCTOR" +PM_CL="" diff --git a/tests/std/tests/VSO_0226079_mutex/test.cpp b/tests/std/tests/VSO_0226079_mutex/test.cpp index 8224f83f3a..22a519b52e 100644 --- a/tests/std/tests/VSO_0226079_mutex/test.cpp +++ b/tests/std/tests/VSO_0226079_mutex/test.cpp @@ -468,6 +468,15 @@ void test_vso_1253916() { do_shared_locked_things(shared_lock{mtx}); } +void test_constexpr_mutex_constructor() { + struct test_type { + constexpr test_type() {} + mutex mtx; + }; + + test_type obj; +} + int main() { { mutex_test_fixture fixture; From b497d9e7d455e2ec665a0f3c8d3247634304ca9e Mon Sep 17 00:00:00 2001 From: cpplearner Date: Fri, 23 Jun 2023 19:01:13 +0800 Subject: [PATCH 2/7] Fix --- stl/src/primitives.hpp | 2 +- tests/std/tests/VSO_0226079_mutex/test.cpp | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/stl/src/primitives.hpp b/stl/src/primitives.hpp index a0bad4a3fc..e4a73f67e8 100644 --- a/stl/src/primitives.hpp +++ b/stl/src/primitives.hpp @@ -58,7 +58,7 @@ namespace Concurrency { virtual bool wait_for(_Stl_critical_section* lock, unsigned int timeout) { return SleepConditionVariableSRW( - &m_condition_variable, reinterpret_cast(lock->_M_srw_lock), timeout, 0) + &m_condition_variable, reinterpret_cast(&lock->_M_srw_lock), timeout, 0) != 0; } diff --git a/tests/std/tests/VSO_0226079_mutex/test.cpp b/tests/std/tests/VSO_0226079_mutex/test.cpp index 22a519b52e..179a42d907 100644 --- a/tests/std/tests/VSO_0226079_mutex/test.cpp +++ b/tests/std/tests/VSO_0226079_mutex/test.cpp @@ -469,12 +469,14 @@ void test_vso_1253916() { } void test_constexpr_mutex_constructor() { +#ifdef _USE_CONSTEXPR_MUTEX_CONSTRUCTOR struct test_type { constexpr test_type() {} mutex mtx; }; test_type obj; +#endif // _USE_CONSTEXPR_MUTEX_CONSTRUCTOR } int main() { From 6b8e3532bb57dd5e01f3a1ede6c928ae4a996e33 Mon Sep 17 00:00:00 2001 From: cpplearner Date: Fri, 23 Jun 2023 19:43:02 +0800 Subject: [PATCH 3/7] Fix --- stl/inc/mutex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/mutex b/stl/inc/mutex index 7c697875ec..fcbf425db6 100644 --- a/stl/inc/mutex +++ b/stl/inc/mutex @@ -93,7 +93,7 @@ private: friend condition_variable; friend condition_variable_any; - _Mtx_internal_imp_t _Mtx_storage; + _Mtx_internal_imp_t _Mtx_storage{}; _Mtx_t _Mymtx() noexcept { // get pointer to _Mtx_internal_imp_t inside _Mtx_storage return &_Mtx_storage; From 83dbc72373009ff48a749c58b8925aaa0b909435 Mon Sep 17 00:00:00 2001 From: cpplearner Date: Sat, 24 Jun 2023 15:04:21 +0800 Subject: [PATCH 4/7] Address review comments --- stl/inc/mutex | 12 +++--- stl/src/primitives.hpp | 47 ++++------------------ tests/std/tests/VSO_0226079_mutex/env.lst | 2 +- tests/std/tests/VSO_0226079_mutex/test.cpp | 4 +- 4 files changed, 16 insertions(+), 49 deletions(-) diff --git a/stl/inc/mutex b/stl/inc/mutex index fcbf425db6..4925a30892 100644 --- a/stl/inc/mutex +++ b/stl/inc/mutex @@ -33,18 +33,18 @@ _EXPORT_STD class condition_variable_any; class _Mutex_base { // base class for all mutex types public: -#ifdef _USE_CONSTEXPR_MUTEX_CONSTRUCTOR +#ifdef _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR + _Mutex_base(int _Flags = 0) noexcept { + _Mtx_init_in_situ(_Mymtx(), _Flags | _Mtx_try); + } +#else // ^^^ _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR / !_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR vvv constexpr _Mutex_base(int _Flags = 0) noexcept { _Mtx_storage._Critical_section = {}; _Mtx_storage._Thread_id = -1; _Mtx_storage._Type = _Flags | _Mtx_try; _Mtx_storage._Count = 0; } -#else // ^^^ _USE_CONSTEXPR_MUTEX_CONSTRUCTOR / !_USE_CONSTEXPR_MUTEX_CONSTRUCTOR vvv - _Mutex_base(int _Flags = 0) noexcept { - _Mtx_init_in_situ(_Mymtx(), _Flags | _Mtx_try); - } -#endif // !_USE_CONSTEXPR_MUTEX_CONSTRUCTOR +#endif // !_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR ~_Mutex_base() noexcept { _Mtx_destroy_in_situ(_Mymtx()); diff --git a/stl/src/primitives.hpp b/stl/src/primitives.hpp index e4a73f67e8..81948fb6a8 100644 --- a/stl/src/primitives.hpp +++ b/stl/src/primitives.hpp @@ -10,39 +10,7 @@ namespace Concurrency { namespace details { - // TRANSITION, only used when constexpr mutex constructor is not enabled - class stl_critical_section_win7 final { - public: - stl_critical_section_win7() = default; - - ~stl_critical_section_win7() = delete; - stl_critical_section_win7(const stl_critical_section_win7&) = delete; - stl_critical_section_win7& operator=(const stl_critical_section_win7&) = delete; - - virtual void lock() { - AcquireSRWLockExclusive(&m_srw_lock); - } - - virtual bool try_lock() { - return TryAcquireSRWLockExclusive(&m_srw_lock) != 0; - } - - virtual bool try_lock_for(unsigned int) { - return try_lock(); - } - - virtual void unlock() { - _Analysis_assume_lock_held_(m_srw_lock); - ReleaseSRWLockExclusive(&m_srw_lock); - } - - virtual void destroy() {} - - private: - SRWLOCK m_srw_lock = SRWLOCK_INIT; - }; - - class stl_condition_variable_win7 final { + class stl_condition_variable_win7 { public: stl_condition_variable_win7() = default; @@ -50,35 +18,34 @@ namespace Concurrency { stl_condition_variable_win7(const stl_condition_variable_win7&) = delete; stl_condition_variable_win7& operator=(const stl_condition_variable_win7&) = delete; - virtual void wait(_Stl_critical_section* lock) { + void wait(_Stl_critical_section* lock) { if (!wait_for(lock, INFINITE)) { std::terminate(); } } - virtual bool wait_for(_Stl_critical_section* lock, unsigned int timeout) { + bool wait_for(_Stl_critical_section* lock, unsigned int timeout) { return SleepConditionVariableSRW( &m_condition_variable, reinterpret_cast(&lock->_M_srw_lock), timeout, 0) != 0; } - virtual void notify_one() { + void notify_one() { WakeConditionVariable(&m_condition_variable); } - virtual void notify_all() { + void notify_all() { WakeAllConditionVariable(&m_condition_variable); } - virtual void destroy() {} - private: + void* unused = nullptr; // TRANSITION, ABI: was the vptr CONDITION_VARIABLE m_condition_variable = CONDITION_VARIABLE_INIT; }; // TRANSITION, only used when constexpr mutex constructor is not enabled inline void create_stl_critical_section(void* p) { - new (p) stl_critical_section_win7; + new (p) _Stl_critical_section; } inline void create_stl_condition_variable(void* p) { diff --git a/tests/std/tests/VSO_0226079_mutex/env.lst b/tests/std/tests/VSO_0226079_mutex/env.lst index 65dc71d458..371ea99eca 100644 --- a/tests/std/tests/VSO_0226079_mutex/env.lst +++ b/tests/std/tests/VSO_0226079_mutex/env.lst @@ -3,5 +3,5 @@ RUNALL_INCLUDE ..\impure_matrix.lst RUNALL_CROSSLIST -PM_CL="/D_USE_CONSTEXPR_MUTEX_CONSTRUCTOR" +PM_CL="/D_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR" PM_CL="" diff --git a/tests/std/tests/VSO_0226079_mutex/test.cpp b/tests/std/tests/VSO_0226079_mutex/test.cpp index 179a42d907..a6671bd426 100644 --- a/tests/std/tests/VSO_0226079_mutex/test.cpp +++ b/tests/std/tests/VSO_0226079_mutex/test.cpp @@ -469,14 +469,14 @@ void test_vso_1253916() { } void test_constexpr_mutex_constructor() { -#ifdef _USE_CONSTEXPR_MUTEX_CONSTRUCTOR +#ifndef _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR struct test_type { constexpr test_type() {} mutex mtx; }; test_type obj; -#endif // _USE_CONSTEXPR_MUTEX_CONSTRUCTOR +#endif // _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR } int main() { From b85f6268e9eb321050ee86738efdb178adeb3013 Mon Sep 17 00:00:00 2001 From: cpplearner Date: Sat, 24 Jun 2023 17:13:11 +0800 Subject: [PATCH 5/7] Test `constinit` --- tests/std/tests/VSO_0226079_mutex/test.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/std/tests/VSO_0226079_mutex/test.cpp b/tests/std/tests/VSO_0226079_mutex/test.cpp index a6671bd426..1b38895bf4 100644 --- a/tests/std/tests/VSO_0226079_mutex/test.cpp +++ b/tests/std/tests/VSO_0226079_mutex/test.cpp @@ -468,16 +468,17 @@ void test_vso_1253916() { do_shared_locked_things(shared_lock{mtx}); } -void test_constexpr_mutex_constructor() { #ifndef _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR - struct test_type { - constexpr test_type() {} - mutex mtx; - }; +struct test_constexpr_ctor { + constexpr test_constexpr_ctor() {} + mutex mtx; +}; - test_type obj; -#endif // _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR -} +test_constexpr_ctor obj; +#if _HAS_CXX20 +constinit test_constexpr_ctor obj2; +#endif // _HAS_CXX20 +#endif // !_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR int main() { { From bcdfcb0dfca4f34c1c816addbd75657f5aba5c93 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 6 Jul 2023 21:36:39 -0700 Subject: [PATCH 6/7] Code review feedback. --- stl/inc/mutex | 2 +- stl/inc/xthreads.h | 2 +- stl/src/primitives.hpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/stl/inc/mutex b/stl/inc/mutex index 4925a30892..1ebcb3a6b4 100644 --- a/stl/inc/mutex +++ b/stl/inc/mutex @@ -95,7 +95,7 @@ private: _Mtx_internal_imp_t _Mtx_storage{}; - _Mtx_t _Mymtx() noexcept { // get pointer to _Mtx_internal_imp_t inside _Mtx_storage + _Mtx_t _Mymtx() noexcept { return &_Mtx_storage; } }; diff --git a/stl/inc/xthreads.h b/stl/inc/xthreads.h index 94fb11bbb7..36a5d1bfde 100644 --- a/stl/inc/xthreads.h +++ b/stl/inc/xthreads.h @@ -29,7 +29,7 @@ struct _Thrd_t { // thread identifier for Win32 using _Smtx_t = void*; struct _Stl_critical_section { - void* _Unused = nullptr; + void* _Unused = nullptr; // TRANSITION, ABI: was the vptr _Smtx_t _M_srw_lock = nullptr; }; diff --git a/stl/src/primitives.hpp b/stl/src/primitives.hpp index 81948fb6a8..4a1cb66662 100644 --- a/stl/src/primitives.hpp +++ b/stl/src/primitives.hpp @@ -44,11 +44,11 @@ namespace Concurrency { }; // TRANSITION, only used when constexpr mutex constructor is not enabled - inline void create_stl_critical_section(void* p) { + inline void create_stl_critical_section(_Stl_critical_section* p) { new (p) _Stl_critical_section; } - inline void create_stl_condition_variable(void* p) { + inline void create_stl_condition_variable(stl_condition_variable_win7* p) { new (p) stl_condition_variable_win7; } From bf043f3884adcff8ac69762f7bf01ce182acb065 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 19 Jul 2023 21:06:25 -0700 Subject: [PATCH 7/7] Avoid `/clr` compiler error. --- tests/std/tests/VSO_0226079_mutex/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/VSO_0226079_mutex/test.cpp b/tests/std/tests/VSO_0226079_mutex/test.cpp index 1b38895bf4..dd41244401 100644 --- a/tests/std/tests/VSO_0226079_mutex/test.cpp +++ b/tests/std/tests/VSO_0226079_mutex/test.cpp @@ -475,9 +475,9 @@ struct test_constexpr_ctor { }; test_constexpr_ctor obj; -#if _HAS_CXX20 +#if _HAS_CXX20 && !defined(_M_CEE) constinit test_constexpr_ctor obj2; -#endif // _HAS_CXX20 +#endif // _HAS_CXX20 && !defined(_M_CEE) #endif // !_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR int main() {