From b7df832483ec84d2f44286a4c785f9c4ba495f89 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 16 Jun 2024 11:02:50 -0700 Subject: [PATCH 01/12] Win8 WaitOnAddress is statically available. No ABI impact - all removed machinery was in an unnamed namespace, or within a function. --- stl/src/atomic_wait.cpp | 145 ---------------------------------------- 1 file changed, 145 deletions(-) diff --git a/stl/src/atomic_wait.cpp b/stl/src/atomic_wait.cpp index b45ed197403..be0e6689612 100644 --- a/stl/src/atomic_wait.cpp +++ b/stl/src/atomic_wait.cpp @@ -89,120 +89,12 @@ namespace { #endif // defined(_DEBUG) } -#ifndef _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE -#if _STL_WIN32_WINNT >= _STL_WIN32_WINNT_WIN8 -#define _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE 1 -#else // ^^^ _STL_WIN32_WINNT >= _STL_WIN32_WINNT_WIN8 / _STL_WIN32_WINNT < _STL_WIN32_WINNT_WIN8 vvv -#define _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE 0 -#endif // ^^^ _STL_WIN32_WINNT < _STL_WIN32_WINNT_WIN8 ^^^ -#endif // !defined(_ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE) - -#if _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE - #pragma comment(lib, "synchronization") #define __crtWaitOnAddress WaitOnAddress #define __crtWakeByAddressSingle WakeByAddressSingle #define __crtWakeByAddressAll WakeByAddressAll -#else // ^^^ _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE / !_ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE vvv - - struct _Wait_functions_table { - _STD atomic _Pfn_WaitOnAddress{nullptr}; - _STD atomic _Pfn_WakeByAddressSingle{nullptr}; - _STD atomic _Pfn_WakeByAddressAll{nullptr}; - _STD atomic<__std_atomic_api_level> _Api_level{__std_atomic_api_level::__not_set}; - }; - - _Wait_functions_table _Wait_functions; - - void _Force_wait_functions_srwlock_only() noexcept { - auto _Local = _Wait_functions._Api_level.load(_STD memory_order_acquire); - if (_Local <= __std_atomic_api_level::__detecting) { - while (!_Wait_functions._Api_level.compare_exchange_weak( - _Local, __std_atomic_api_level::__has_srwlock, _STD memory_order_acq_rel)) { - if (_Local > __std_atomic_api_level::__detecting) { - return; - } - } - } - } - - [[nodiscard]] __std_atomic_api_level _Init_wait_functions(__std_atomic_api_level _Level) { - while (!_Wait_functions._Api_level.compare_exchange_weak( - _Level, __std_atomic_api_level::__detecting, _STD memory_order_acq_rel)) { - if (_Level > __std_atomic_api_level::__detecting) { - return _Level; - } - } - - _Level = __std_atomic_api_level::__has_srwlock; - - const HMODULE _Sync_module = GetModuleHandleW(L"api-ms-win-core-synch-l1-2-0.dll"); - if (_Sync_module != nullptr) { - const auto _Wait_on_address = - reinterpret_cast(GetProcAddress(_Sync_module, "WaitOnAddress")); - const auto _Wake_by_address_single = - reinterpret_cast(GetProcAddress(_Sync_module, "WakeByAddressSingle")); - const auto _Wake_by_address_all = - reinterpret_cast(GetProcAddress(_Sync_module, "WakeByAddressAll")); - - if (_Wait_on_address != nullptr && _Wake_by_address_single != nullptr && _Wake_by_address_all != nullptr) { - _Wait_functions._Pfn_WaitOnAddress.store(_Wait_on_address, _STD memory_order_relaxed); - _Wait_functions._Pfn_WakeByAddressSingle.store(_Wake_by_address_single, _STD memory_order_relaxed); - _Wait_functions._Pfn_WakeByAddressAll.store(_Wake_by_address_all, _STD memory_order_relaxed); - _Level = __std_atomic_api_level::__has_wait_on_address; - } - } - - // for __has_srwlock, relaxed would have been enough, not distinguishing for consistency - _Wait_functions._Api_level.store(_Level, _STD memory_order_release); - return _Level; - } - - [[nodiscard]] __std_atomic_api_level _Acquire_wait_functions() noexcept { - auto _Level = _Wait_functions._Api_level.load(_STD memory_order_acquire); - if (_Level <= __std_atomic_api_level::__detecting) { - _Level = _Init_wait_functions(_Level); - } - - return _Level; - } - - [[nodiscard]] BOOL __crtWaitOnAddress( - volatile VOID* Address, PVOID CompareAddress, SIZE_T AddressSize, DWORD dwMilliseconds) { - const auto _Wait_on_address = _Wait_functions._Pfn_WaitOnAddress.load(_STD memory_order_relaxed); - return _Wait_on_address(Address, CompareAddress, AddressSize, dwMilliseconds); - } - - VOID __crtWakeByAddressSingle(PVOID Address) { - const auto _Wake_by_address_single = _Wait_functions._Pfn_WakeByAddressSingle.load(_STD memory_order_relaxed); - _Wake_by_address_single(Address); - } - - VOID __crtWakeByAddressAll(PVOID Address) { - const auto _Wake_by_address_all = _Wait_functions._Pfn_WakeByAddressAll.load(_STD memory_order_relaxed); - _Wake_by_address_all(Address); - } - - bool __stdcall _Atomic_wait_are_equal_direct_fallback( - const void* _Storage, void* _Comparand, size_t _Size, void*) noexcept { - switch (_Size) { - case 1: - return __iso_volatile_load8(static_cast(_Storage)) == *static_cast(_Comparand); - case 2: - return __iso_volatile_load16(static_cast(_Storage)) == *static_cast(_Comparand); - case 4: - return __iso_volatile_load32(static_cast(_Storage)) == *static_cast(_Comparand); - case 8: - return __iso_volatile_load64(static_cast(_Storage)) - == *static_cast(_Comparand); - default: - _CSTD abort(); - } - } -#endif // _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE - [[nodiscard]] unsigned char __std_atomic_compare_exchange_128_fallback( _Inout_bytecount_(16) long long* _Destination, _In_ long long _ExchangeHigh, _In_ long long _ExchangeLow, _Inout_bytecount_(16) long long* _ComparandResult) noexcept { @@ -225,13 +117,6 @@ namespace { extern "C" { int __stdcall __std_atomic_wait_direct(const void* const _Storage, void* const _Comparand, const size_t _Size, const unsigned long _Remaining_timeout) noexcept { -#if _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE == 0 - if (_Acquire_wait_functions() < __std_atomic_api_level::__has_wait_on_address) { - return __std_atomic_wait_indirect( - _Storage, _Comparand, _Size, nullptr, &_Atomic_wait_are_equal_direct_fallback, _Remaining_timeout); - } -#endif // _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE == 0 - const auto _Result = __crtWaitOnAddress( const_cast(_Storage), const_cast(_Comparand), _Size, _Remaining_timeout); @@ -242,24 +127,10 @@ int __stdcall __std_atomic_wait_direct(const void* const _Storage, void* const _ } void __stdcall __std_atomic_notify_one_direct(const void* const _Storage) noexcept { -#if _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE == 0 - if (_Acquire_wait_functions() < __std_atomic_api_level::__has_wait_on_address) { - __std_atomic_notify_one_indirect(_Storage); - return; - } -#endif // _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE = 0 - __crtWakeByAddressSingle(const_cast(_Storage)); } void __stdcall __std_atomic_notify_all_direct(const void* const _Storage) noexcept { -#if _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE == 0 - if (_Acquire_wait_functions() < __std_atomic_api_level::__has_wait_on_address) { - __std_atomic_notify_all_indirect(_Storage); - return; - } -#endif // _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE == 0 - __crtWakeByAddressAll(const_cast(_Storage)); } @@ -357,24 +228,8 @@ unsigned long __stdcall __std_atomic_wait_get_remaining_timeout(unsigned long lo } __std_atomic_api_level __stdcall __std_atomic_set_api_level(__std_atomic_api_level _Requested_api_level) noexcept { -#if _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE (void) _Requested_api_level; return __std_atomic_api_level::__has_wait_on_address; -#else // ^^^ _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE / !_ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE vvv - switch (_Requested_api_level) { - case __std_atomic_api_level::__not_set: - case __std_atomic_api_level::__detecting: - _CSTD abort(); - case __std_atomic_api_level::__has_srwlock: - _Force_wait_functions_srwlock_only(); - break; - case __std_atomic_api_level::__has_wait_on_address: - default: // future compat: new header using an old DLL will get the highest requested level supported - break; - } - - return _Acquire_wait_functions(); -#endif // !_ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE } #pragma warning(push) From aaf0b70634eade8c25801ed577282d6c223f0f7b Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 16 Jun 2024 11:09:24 -0700 Subject: [PATCH 02/12] Demacroize `__crtWaitOnAddress` => `WaitOnAddress` etc. --- stl/src/atomic_wait.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/stl/src/atomic_wait.cpp b/stl/src/atomic_wait.cpp index be0e6689612..1397639e73d 100644 --- a/stl/src/atomic_wait.cpp +++ b/stl/src/atomic_wait.cpp @@ -91,10 +91,6 @@ namespace { #pragma comment(lib, "synchronization") -#define __crtWaitOnAddress WaitOnAddress -#define __crtWakeByAddressSingle WakeByAddressSingle -#define __crtWakeByAddressAll WakeByAddressAll - [[nodiscard]] unsigned char __std_atomic_compare_exchange_128_fallback( _Inout_bytecount_(16) long long* _Destination, _In_ long long _ExchangeHigh, _In_ long long _ExchangeLow, _Inout_bytecount_(16) long long* _ComparandResult) noexcept { @@ -117,8 +113,8 @@ namespace { extern "C" { int __stdcall __std_atomic_wait_direct(const void* const _Storage, void* const _Comparand, const size_t _Size, const unsigned long _Remaining_timeout) noexcept { - const auto _Result = __crtWaitOnAddress( - const_cast(_Storage), const_cast(_Comparand), _Size, _Remaining_timeout); + const auto _Result = + WaitOnAddress(const_cast(_Storage), const_cast(_Comparand), _Size, _Remaining_timeout); if (!_Result) { _Assume_timeout(); @@ -127,11 +123,11 @@ int __stdcall __std_atomic_wait_direct(const void* const _Storage, void* const _ } void __stdcall __std_atomic_notify_one_direct(const void* const _Storage) noexcept { - __crtWakeByAddressSingle(const_cast(_Storage)); + WakeByAddressSingle(const_cast(_Storage)); } void __stdcall __std_atomic_notify_all_direct(const void* const _Storage) noexcept { - __crtWakeByAddressAll(const_cast(_Storage)); + WakeByAddressAll(const_cast(_Storage)); } void __stdcall __std_atomic_notify_one_indirect(const void* const _Storage) noexcept { From a729f46cd42bcd5070ff2611cce6936dd4b60688 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 16 Jun 2024 11:20:47 -0700 Subject: [PATCH 03/12] Preserve `__std_atomic_set_api_level` for bincompat, delete `P1135R6_atomic_wait_vista`. Move `enum class __std_atomic_api_level` to `atomic_wait.cpp`, marked as preserved. It's still `extern "C"`. Drop the declaration of `__std_atomic_set_api_level` and mark the definition as preserved. It's still dllexported by `stl/src/msvcp_atomic_wait.src`. Drop the unused parameter name `_Requested_api_level`. `P1135R6_atomic_wait` doesn't need to verify the API level anymore. Delete `P1135R6_atomic_wait_vista`, which existed only to test the older API level. --- stl/inc/xatomic_wait.h | 13 ------------- stl/src/atomic_wait.cpp | 12 ++++++++++-- tests/std/test.lst | 1 - tests/std/tests/P1135R6_atomic_wait/test.cpp | 2 -- tests/std/tests/P1135R6_atomic_wait_vista/env.lst | 4 ---- tests/std/tests/P1135R6_atomic_wait_vista/test.cpp | 11 ----------- 6 files changed, 10 insertions(+), 33 deletions(-) delete mode 100644 tests/std/tests/P1135R6_atomic_wait_vista/env.lst delete mode 100644 tests/std/tests/P1135R6_atomic_wait_vista/test.cpp diff --git a/stl/inc/xatomic_wait.h b/stl/inc/xatomic_wait.h index 178f91fa9e8..9c9fe1fd024 100644 --- a/stl/inc/xatomic_wait.h +++ b/stl/inc/xatomic_wait.h @@ -21,19 +21,6 @@ _STL_DISABLE_CLANG_WARNINGS extern "C" { inline constexpr unsigned long __std_atomic_wait_no_timeout = 0xFFFF'FFFF; // Pass as partial timeout -enum class __std_atomic_api_level : unsigned long { - __not_set, - __detecting, - __has_srwlock, - __has_wait_on_address, -}; - -// This function allows testing the atomic wait support while always using the APIs for a platform with fewer -// capabilities; it attempts to lock the APIs used to the level `_Requested_api_level`, and returns the actual API level -// in use. Once the API level has been set by calling this function (or detected by a call to one of the atomic wait -// functions), it can no longer be changed. -__std_atomic_api_level __stdcall __std_atomic_set_api_level(__std_atomic_api_level _Requested_api_level) noexcept; - // Support for atomic waits. // The "direct" functions are used when the underlying infrastructure can use WaitOnAddress directly; that is, _Size is // 1, 2, 4, or 8. The contract is the same as the WaitOnAddress function from the Windows SDK. If WaitOnAddress is not diff --git a/stl/src/atomic_wait.cpp b/stl/src/atomic_wait.cpp index 1397639e73d..5180390f366 100644 --- a/stl/src/atomic_wait.cpp +++ b/stl/src/atomic_wait.cpp @@ -223,8 +223,16 @@ unsigned long __stdcall __std_atomic_wait_get_remaining_timeout(unsigned long lo return static_cast(_Remaining); } -__std_atomic_api_level __stdcall __std_atomic_set_api_level(__std_atomic_api_level _Requested_api_level) noexcept { - (void) _Requested_api_level; +// TRANSITION, ABI: preserved for binary compatibility +enum class __std_atomic_api_level : unsigned long { + __not_set, + __detecting, + __has_srwlock, + __has_wait_on_address, +}; + +// TRANSITION, ABI: preserved for binary compatibility +__std_atomic_api_level __stdcall __std_atomic_set_api_level(__std_atomic_api_level) noexcept { return __std_atomic_api_level::__has_wait_on_address; } diff --git a/tests/std/test.lst b/tests/std/test.lst index 3b22ab147a9..9c6c25788b2 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -522,7 +522,6 @@ tests\P1032R1_miscellaneous_constexpr tests\P1132R7_out_ptr tests\P1135R6_atomic_flag_test tests\P1135R6_atomic_wait -tests\P1135R6_atomic_wait_vista tests\P1135R6_barrier tests\P1135R6_latch tests\P1135R6_semaphore diff --git a/tests/std/tests/P1135R6_atomic_wait/test.cpp b/tests/std/tests/P1135R6_atomic_wait/test.cpp index 2c62d15e1b2..ae5f71b857d 100644 --- a/tests/std/tests/P1135R6_atomic_wait/test.cpp +++ b/tests/std/tests/P1135R6_atomic_wait/test.cpp @@ -57,7 +57,5 @@ void test_incomplete_associated_class_all() { // COMPILE-ONLY #endif // ^^^ no workaround ^^^ int main() { - assert(__std_atomic_set_api_level(__std_atomic_api_level::__has_wait_on_address) - == __std_atomic_api_level::__has_wait_on_address); test_atomic_wait(); } diff --git a/tests/std/tests/P1135R6_atomic_wait_vista/env.lst b/tests/std/tests/P1135R6_atomic_wait_vista/env.lst deleted file mode 100644 index 351a8293d9d..00000000000 --- a/tests/std/tests/P1135R6_atomic_wait_vista/env.lst +++ /dev/null @@ -1,4 +0,0 @@ -# Copyright (c) Microsoft Corporation. -# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - -RUNALL_INCLUDE ..\usual_20_matrix.lst diff --git a/tests/std/tests/P1135R6_atomic_wait_vista/test.cpp b/tests/std/tests/P1135R6_atomic_wait_vista/test.cpp deleted file mode 100644 index dc1dd27e1b0..00000000000 --- a/tests/std/tests/P1135R6_atomic_wait_vista/test.cpp +++ /dev/null @@ -1,11 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - -#include "test_atomic_wait.hpp" - -int main() { -#if defined(_M_IX86) || defined(_M_X64) && !defined(_M_ARM64EC) - assert(__std_atomic_set_api_level(__std_atomic_api_level::__has_srwlock) == __std_atomic_api_level::__has_srwlock); - test_atomic_wait(); -#endif // defined(_M_IX86) || defined(_M_X64) && !defined(_M_ARM64EC) -} From 36c8462ba57061878627546e9f3bcc2ee09f0b58 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 16 Jun 2024 12:17:37 -0700 Subject: [PATCH 04/12] Spend fewer lines on `__std_atomic_api_level` by dropping the trailing comma. --- stl/src/atomic_wait.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/stl/src/atomic_wait.cpp b/stl/src/atomic_wait.cpp index 5180390f366..7b47aedf052 100644 --- a/stl/src/atomic_wait.cpp +++ b/stl/src/atomic_wait.cpp @@ -224,14 +224,7 @@ unsigned long __stdcall __std_atomic_wait_get_remaining_timeout(unsigned long lo } // TRANSITION, ABI: preserved for binary compatibility -enum class __std_atomic_api_level : unsigned long { - __not_set, - __detecting, - __has_srwlock, - __has_wait_on_address, -}; - -// TRANSITION, ABI: preserved for binary compatibility +enum class __std_atomic_api_level : unsigned long { __not_set, __detecting, __has_srwlock, __has_wait_on_address }; __std_atomic_api_level __stdcall __std_atomic_set_api_level(__std_atomic_api_level) noexcept { return __std_atomic_api_level::__has_wait_on_address; } From 58787b572f7b68b6a81acbce92fa80fae5b99d4f Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sun, 16 Jun 2024 11:29:39 -0700 Subject: [PATCH 05/12] Fuse `test_atomic_wait.hpp` into `P1135R6_atomic_wait`. No changes other than dropping the duplicate banner and `#pragma once`. --- tests/std/include/test_atomic_wait.hpp | 366 ------------------- tests/std/tests/P1135R6_atomic_wait/test.cpp | 362 +++++++++++++++++- 2 files changed, 361 insertions(+), 367 deletions(-) delete mode 100644 tests/std/include/test_atomic_wait.hpp diff --git a/tests/std/include/test_atomic_wait.hpp b/tests/std/include/test_atomic_wait.hpp deleted file mode 100644 index 28f4be97fba..00000000000 --- a/tests/std/include/test_atomic_wait.hpp +++ /dev/null @@ -1,366 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - -#pragma once - -#include -#include -#include -#include -#include -#include - -template