From 60884ab0862f48a3e5ec94d7d1dfe71e0befa8ce Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 10 Jul 2020 22:38:30 +0300 Subject: [PATCH 01/30] Atomic CAS with pad (#23, P0528R3) --- stl/inc/atomic | 145 ++++++++++++++++--- tests/std/test.lst | 1 + tests/std/tests/P0528R3_cmpxchg_pad/env.lst | 4 + tests/std/tests/P0528R3_cmpxchg_pad/test.cpp | 138 ++++++++++++++++++ 4 files changed, 268 insertions(+), 20 deletions(-) create mode 100644 tests/std/tests/P0528R3_cmpxchg_pad/env.lst create mode 100644 tests/std/tests/P0528R3_cmpxchg_pad/test.cpp diff --git a/stl/inc/atomic b/stl/inc/atomic index 8881fae37a8..40ff803d430 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -354,8 +354,18 @@ struct _Atomic_storage { _Check_memory_order(_Order); const auto _Storage_ptr = _STD addressof(_Storage); const auto _Expected_ptr = _STD addressof(_Expected); +#if _HAS_CXX20 + if constexpr (!_STD has_unique_object_representations_v<_Ty>) { + __builtin_zero_non_value_bits(_Expected_ptr); + } +#endif bool _Result; _Lock(); +#if _HAS_CXX20 + if constexpr (!_STD has_unique_object_representations_v<_Ty>) { + __builtin_zero_non_value_bits(_Storage_ptr); + } +#endif if (_CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0) { _CSTD memcpy(_Storage_ptr, _STD addressof(_Desired), sizeof(_Ty)); _Result = true; @@ -480,13 +490,37 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics bool compare_exchange_strong(_Ty& _Expected, const _Ty _Desired, const memory_order _Order = memory_order_seq_cst) noexcept { // CAS with given memory order - const char _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation + char _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation char _Prev_bytes; - _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, _Atomic_address_as(_Storage), - _Atomic_reinterpret_as(_Desired), _Expected_bytes); - if (_Prev_bytes == _Expected_bytes) { - return true; + +#if _HAS_CXX20 + if constexpr (_STD has_unique_object_representations_v<_Ty>) { +#endif + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } +#if _HAS_CXX20 + } else { + _Ty _Mask; + _CSTD memset(&_Mask, 0xff, sizeof(_Ty)); + __builtin_zero_non_value_bits(_STD addressof(_Mask)); + const short _Mask_val = _Atomic_reinterpret_as(_Mask); + + for (;;) { + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { + break; + } + _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); + } } +#endif reinterpret_cast(_Expected) = _Prev_bytes; return false; @@ -562,13 +596,36 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics bool compare_exchange_strong(_Ty& _Expected, const _Ty _Desired, const memory_order _Order = memory_order_seq_cst) noexcept { // CAS with given memory order - const short _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation + short _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation short _Prev_bytes; - _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, - _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); - if (_Prev_bytes == _Expected_bytes) { - return true; +#if _HAS_CXX20 + if constexpr (_STD has_unique_object_representations_v<_Ty>) { +#endif + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } +#if _HAS_CXX20 + } else { + _Ty _Mask; + _CSTD memset(&_Mask, 0xff, sizeof(_Ty)); + __builtin_zero_non_value_bits(_STD addressof(_Mask)); + const short _Mask_val = _Atomic_reinterpret_as(_Mask); + + for (;;) { + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { + break; + } + _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); + } } +#endif _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -642,13 +699,36 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics bool compare_exchange_strong(_Ty& _Expected, const _Ty _Desired, const memory_order _Order = memory_order_seq_cst) noexcept { // CAS with given memory order - const long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation + long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation long _Prev_bytes; - _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, _Atomic_address_as(_Storage), - _Atomic_reinterpret_as(_Desired), _Expected_bytes); - if (_Prev_bytes == _Expected_bytes) { - return true; +#if _HAS_CXX20 + if constexpr (_STD has_unique_object_representations_v<_Ty>) { +#endif + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } +#if _HAS_CXX20 + } else { + _Ty _Mask; + _CSTD memset(&_Mask, 0xff, sizeof(_Ty)); + __builtin_zero_non_value_bits(_STD addressof(_Mask)); + const long _Mask_val = _Atomic_reinterpret_as(_Mask); + + for (;;) { + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { + break; + } + _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); + } } +#endif _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -749,13 +829,38 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics bool compare_exchange_strong(_Ty& _Expected, const _Ty _Desired, const memory_order _Order = memory_order_seq_cst) noexcept { // CAS with given memory order - const long long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation + long long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation long long _Prev_bytes; - _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, - _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); - if (_Prev_bytes == _Expected_bytes) { - return true; + +#if _HAS_CXX20 + if constexpr (_STD has_unique_object_representations_v<_Ty>) { +#endif + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } +#if _HAS_CXX20 + } else { + _Ty _Mask; + _CSTD memset(&_Mask, 0xff, sizeof(_Ty)); + __builtin_zero_non_value_bits(_STD addressof(_Mask)); + const long long _Mask_val = _Atomic_reinterpret_as(_Mask); + + for (;;) { + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), + _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { + break; + } + _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); + } } +#endif _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; diff --git a/tests/std/test.lst b/tests/std/test.lst index 98930beaa8b..b0e7e3e869c 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -219,6 +219,7 @@ tests\P0433R2_deduction_guides tests\P0476R2_bit_cast tests\P0487R1_fixing_operator_shl_basic_istream_char_pointer tests\P0513R0_poisoning_the_hash +tests\P0528R3_cmpxchg_pad tests\P0553R4_bit_rotating_and_counting_functions tests\P0556R3_bit_integral_power_of_two_operations tests\P0586R2_integer_comparison diff --git a/tests/std/tests/P0528R3_cmpxchg_pad/env.lst b/tests/std/tests/P0528R3_cmpxchg_pad/env.lst new file mode 100644 index 00000000000..642f530ffad --- /dev/null +++ b/tests/std/tests/P0528R3_cmpxchg_pad/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\usual_latest_matrix.lst diff --git a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp new file mode 100644 index 00000000000..daf260c042e --- /dev/null +++ b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp @@ -0,0 +1,138 @@ +#include +#include + +struct X1 { + char x : 6; + + void set(char v) { + x = v; + } +}; + +struct X2 { + short x : 9; + + void set(char v) { + x = v; + } +}; + +#pragma pack(push,1) +struct X3 { + char x : 4; + char : 2; + char y : 1; + short z; + + void set(char v) { + x = v; + y = 0; + z = 0; + } +}; +#pragma pack(pop) + +#pragma warning(push) +#pragma warning(disable:4324) // '%s': structure was padded due to alignment specifier +struct alignas(4) X4 { + char x; + + void set(char v) { + x = v; + } +}; +#pragma warning(pop) + +struct X8 { + char x; + long y; + + void set(char v) { + x = v; + y = 0; + } +}; + +struct X16 { + long x; + char y; + long long z; + + void set(char v) { + x = v; + y = 0; + z = 0; + } +}; + +struct X20 { + long x; + long y[3]; + char z; + + void set(char v) { + x = v; + memset(&y, 0, sizeof(y)); + z = 0; + } +}; + + +template +void test() { + static_assert(sizeof(X) == S, "Unexpected size"); + X x2; + X x3; + X x4; + X x1; + memset(&x1, 0x00, sizeof(x1)); + memset(&x2, 0xff, sizeof(x1)); + memset(&x3, 0xff, sizeof(x1)); + x1.set(5); + x2.set(5); + x3.set(6); + x4.set(7); + + std::atomic v; + v.store(x1); + X x; + memcpy(&x, &x3, sizeof(x)); + assert(!v.compare_exchange_strong(x, x4)); + assert(v.load().x == 5); + + v.store(x1); + for (int retry = 0; retry != 10; ++retry) { + X xw; + memcpy(&xw, &x3, sizeof(x)); + assert(!v.compare_exchange_weak(xw, x4)); + assert(v.load().x == 5); + } + + v.store(x1); + memcpy(&x, &x2, sizeof(x)); + assert(v.compare_exchange_strong(x, x3)); + assert(v.load().x == 6); + + v.store(x1); + for (;;) { + X xw; + memcpy(&xw, &x2, sizeof(x)); + if (v.compare_exchange_weak(xw, x3)) + break; + } + assert(v.load().x == 6); +} + + + +int main() +{ + test(); + test(); + test(); + test(); + test(); + test(); + test(); + return 0; +} \ No newline at end of file From edd535b61804ae14b476e2911986440d34f5bc95 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 10 Jul 2020 22:46:26 +0300 Subject: [PATCH 02/30] formatting --- tests/std/tests/P0528R3_cmpxchg_pad/test.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp index daf260c042e..bc879e2bf33 100644 --- a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp +++ b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp @@ -17,7 +17,7 @@ struct X2 { } }; -#pragma pack(push,1) +#pragma pack(push, 1) struct X3 { char x : 4; char : 2; @@ -33,7 +33,7 @@ struct X3 { #pragma pack(pop) #pragma warning(push) -#pragma warning(disable:4324) // '%s': structure was padded due to alignment specifier +#pragma warning(disable : 4324) // '%s': structure was padded due to alignment specifier struct alignas(4) X4 { char x; @@ -78,7 +78,7 @@ struct X20 { }; -template +template void test() { static_assert(sizeof(X) == S, "Unexpected size"); X x2; @@ -117,16 +117,15 @@ void test() { for (;;) { X xw; memcpy(&xw, &x2, sizeof(x)); - if (v.compare_exchange_weak(xw, x3)) + if (v.compare_exchange_weak(xw, x3)) { break; + } } assert(v.load().x == 6); } - -int main() -{ +int main() { test(); test(); test(); @@ -135,4 +134,4 @@ int main() test(); test(); return 0; -} \ No newline at end of file +} From c3c6c1c380b3610ebac37f7a797cc5e3cbdf5bf0 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Sat, 11 Jul 2020 05:34:31 +0300 Subject: [PATCH 03/30] Address review comments --- stl/inc/atomic | 50 ++++++++++---------- tests/std/tests/P0528R3_cmpxchg_pad/test.cpp | 23 +++++---- 2 files changed, 40 insertions(+), 33 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 40ff803d430..1c5854c6366 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -354,15 +354,15 @@ struct _Atomic_storage { _Check_memory_order(_Order); const auto _Storage_ptr = _STD addressof(_Storage); const auto _Expected_ptr = _STD addressof(_Expected); -#if _HAS_CXX20 - if constexpr (!_STD has_unique_object_representations_v<_Ty>) { +#if _HAS_CXX20 && !defined(__clang__) + if constexpr (!has_unique_object_representations_v<_Ty>) { __builtin_zero_non_value_bits(_Expected_ptr); } #endif bool _Result; _Lock(); -#if _HAS_CXX20 - if constexpr (!_STD has_unique_object_representations_v<_Ty>) { +#if _HAS_CXX20 && !defined(__clang__) + if constexpr (!has_unique_object_representations_v<_Ty>) { __builtin_zero_non_value_bits(_Storage_ptr); } #endif @@ -493,20 +493,20 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics char _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation char _Prev_bytes; -#if _HAS_CXX20 - if constexpr (_STD has_unique_object_representations_v<_Ty>) { +#if _HAS_CXX20 && !defined(__clang__) + if constexpr (has_unique_object_representations_v<_Ty>) { #endif _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 +#if _HAS_CXX20 && !defined(__clang__) } else { _Ty _Mask; - _CSTD memset(&_Mask, 0xff, sizeof(_Ty)); + _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); __builtin_zero_non_value_bits(_STD addressof(_Mask)); - const short _Mask_val = _Atomic_reinterpret_as(_Mask); + const char _Mask_val = _Atomic_reinterpret_as(_Mask); for (;;) { _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, @@ -520,7 +520,7 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif +#endif // _HAS_CXX20 && !defined(__clang__) reinterpret_cast(_Expected) = _Prev_bytes; return false; @@ -598,18 +598,18 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics const memory_order _Order = memory_order_seq_cst) noexcept { // CAS with given memory order short _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation short _Prev_bytes; -#if _HAS_CXX20 - if constexpr (_STD has_unique_object_representations_v<_Ty>) { +#if _HAS_CXX20 && !defined(__clang__) + if constexpr (has_unique_object_representations_v<_Ty>) { #endif _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 +#if _HAS_CXX20 && !defined(__clang__) } else { _Ty _Mask; - _CSTD memset(&_Mask, 0xff, sizeof(_Ty)); + _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); __builtin_zero_non_value_bits(_STD addressof(_Mask)); const short _Mask_val = _Atomic_reinterpret_as(_Mask); @@ -625,7 +625,7 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif +#endif // _HAS_CXX20 && !defined(__clang__) _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -701,18 +701,18 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics const memory_order _Order = memory_order_seq_cst) noexcept { // CAS with given memory order long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation long _Prev_bytes; -#if _HAS_CXX20 - if constexpr (_STD has_unique_object_representations_v<_Ty>) { +#if _HAS_CXX20 && !defined(__clang__) + if constexpr (has_unique_object_representations_v<_Ty>) { #endif _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 +#if _HAS_CXX20 && !defined(__clang__) } else { _Ty _Mask; - _CSTD memset(&_Mask, 0xff, sizeof(_Ty)); + _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); __builtin_zero_non_value_bits(_STD addressof(_Mask)); const long _Mask_val = _Atomic_reinterpret_as(_Mask); @@ -728,7 +728,7 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif +#endif // _HAS_CXX20 && !defined(__clang__) _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -832,18 +832,18 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics long long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation long long _Prev_bytes; -#if _HAS_CXX20 - if constexpr (_STD has_unique_object_representations_v<_Ty>) { +#if _HAS_CXX20 && !defined(__clang__) + if constexpr (has_unique_object_representations_v<_Ty>) { #endif _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 +#if _HAS_CXX20 && !defined(__clang__) } else { _Ty _Mask; - _CSTD memset(&_Mask, 0xff, sizeof(_Ty)); + _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); __builtin_zero_non_value_bits(_STD addressof(_Mask)); const long long _Mask_val = _Atomic_reinterpret_as(_Mask); @@ -860,7 +860,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif +#endif // _HAS_CXX20 && !defined(__clang__) _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; diff --git a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp index bc879e2bf33..4e34390cb78 100644 --- a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp +++ b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp @@ -1,5 +1,10 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + #include #include +#include +#include struct X1 { char x : 6; @@ -47,6 +52,8 @@ struct X8 { char x; long y; + void operator&() const {} + void set(char v) { x = v; y = 0; @@ -72,7 +79,7 @@ struct X20 { void set(char v) { x = v; - memset(&y, 0, sizeof(y)); + std::memset(&y, 0, sizeof(y)); z = 0; } }; @@ -85,9 +92,9 @@ void test() { X x3; X x4; X x1; - memset(&x1, 0x00, sizeof(x1)); - memset(&x2, 0xff, sizeof(x1)); - memset(&x3, 0xff, sizeof(x1)); + std::memset(std::addressof(x1), 0x00, sizeof(x1)); + std::memset(std::addressof(x2), 0xff, sizeof(x1)); + std::memset(std::addressof(x3), 0xff, sizeof(x1)); x1.set(5); x2.set(5); x3.set(6); @@ -96,27 +103,27 @@ void test() { std::atomic v; v.store(x1); X x; - memcpy(&x, &x3, sizeof(x)); + std::memcpy(std::addressof(x), std::addressof(x3), sizeof(x)); assert(!v.compare_exchange_strong(x, x4)); assert(v.load().x == 5); v.store(x1); for (int retry = 0; retry != 10; ++retry) { X xw; - memcpy(&xw, &x3, sizeof(x)); + std::memcpy(std::addressof(xw), std::addressof(x3), sizeof(x)); assert(!v.compare_exchange_weak(xw, x4)); assert(v.load().x == 5); } v.store(x1); - memcpy(&x, &x2, sizeof(x)); + std::memcpy(std::addressof(x), std::addressof(x2), sizeof(x)); assert(v.compare_exchange_strong(x, x3)); assert(v.load().x == 6); v.store(x1); for (;;) { X xw; - memcpy(&xw, &x2, sizeof(x)); + std::memcpy(std::addressof(xw), std::addressof(x2), sizeof(x)); if (v.compare_exchange_weak(xw, x3)) { break; } From 00f0ce4c43cecff5dd28d6c7cfbeffd354ac4eeb Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Sat, 11 Jul 2020 05:40:05 +0300 Subject: [PATCH 04/30] more review comments --- stl/inc/atomic | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 1c5854c6366..b3ec177813d 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -358,14 +358,14 @@ struct _Atomic_storage { if constexpr (!has_unique_object_representations_v<_Ty>) { __builtin_zero_non_value_bits(_Expected_ptr); } -#endif +#endif // _HAS_CXX20 && !defined(__clang__) bool _Result; _Lock(); #if _HAS_CXX20 && !defined(__clang__) if constexpr (!has_unique_object_representations_v<_Ty>) { __builtin_zero_non_value_bits(_Storage_ptr); } -#endif +#endif // _HAS_CXX20 && !defined(__clang__) if (_CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0) { _CSTD memcpy(_Storage_ptr, _STD addressof(_Desired), sizeof(_Ty)); _Result = true; @@ -495,7 +495,7 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics #if _HAS_CXX20 && !defined(__clang__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif +#endif // _HAS_CXX20 && !defined(__clang__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { @@ -514,6 +514,7 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics if (_Prev_bytes == _Expected_bytes) { return true; } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { break; } @@ -600,7 +601,7 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics short _Prev_bytes; #if _HAS_CXX20 && !defined(__clang__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif +#endif // _HAS_CXX20 && !defined(__clang__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { @@ -619,6 +620,7 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics if (_Prev_bytes == _Expected_bytes) { return true; } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { break; } @@ -703,7 +705,7 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics long _Prev_bytes; #if _HAS_CXX20 && !defined(__clang__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif +#endif // _HAS_CXX20 && !defined(__clang__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { @@ -722,6 +724,7 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics if (_Prev_bytes == _Expected_bytes) { return true; } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { break; } @@ -834,7 +837,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics #if _HAS_CXX20 && !defined(__clang__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif +#endif // _HAS_CXX20 && !defined(__clang__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { @@ -854,6 +857,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics if (_Prev_bytes == _Expected_bytes) { return true; } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { break; } From 2b635e54843e8077f49ccc289237dd73653eb851 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Sat, 11 Jul 2020 14:37:40 +0300 Subject: [PATCH 05/30] Disable for EDG too --- stl/inc/atomic | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index b3ec177813d..864ac4cf563 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -354,18 +354,18 @@ struct _Atomic_storage { _Check_memory_order(_Order); const auto _Storage_ptr = _STD addressof(_Storage); const auto _Expected_ptr = _STD addressof(_Expected); -#if _HAS_CXX20 && !defined(__clang__) +#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) if constexpr (!has_unique_object_representations_v<_Ty>) { __builtin_zero_non_value_bits(_Expected_ptr); } -#endif // _HAS_CXX20 && !defined(__clang__) +#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) bool _Result; _Lock(); -#if _HAS_CXX20 && !defined(__clang__) +#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) if constexpr (!has_unique_object_representations_v<_Ty>) { __builtin_zero_non_value_bits(_Storage_ptr); } -#endif // _HAS_CXX20 && !defined(__clang__) +#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) if (_CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0) { _CSTD memcpy(_Storage_ptr, _STD addressof(_Desired), sizeof(_Ty)); _Result = true; @@ -493,15 +493,15 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics char _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation char _Prev_bytes; -#if _HAS_CXX20 && !defined(__clang__) +#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif // _HAS_CXX20 && !defined(__clang__) +#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 && !defined(__clang__) +#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) } else { _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); @@ -521,7 +521,7 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif // _HAS_CXX20 && !defined(__clang__) +#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) reinterpret_cast(_Expected) = _Prev_bytes; return false; @@ -599,15 +599,15 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics const memory_order _Order = memory_order_seq_cst) noexcept { // CAS with given memory order short _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation short _Prev_bytes; -#if _HAS_CXX20 && !defined(__clang__) +#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif // _HAS_CXX20 && !defined(__clang__) +#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 && !defined(__clang__) +#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) } else { _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); @@ -627,7 +627,7 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif // _HAS_CXX20 && !defined(__clang__) +#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -703,15 +703,15 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics const memory_order _Order = memory_order_seq_cst) noexcept { // CAS with given memory order long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation long _Prev_bytes; -#if _HAS_CXX20 && !defined(__clang__) +#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif // _HAS_CXX20 && !defined(__clang__) +#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 && !defined(__clang__) +#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) } else { _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); @@ -731,7 +731,7 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif // _HAS_CXX20 && !defined(__clang__) +#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -835,15 +835,15 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics long long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation long long _Prev_bytes; -#if _HAS_CXX20 && !defined(__clang__) +#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif // _HAS_CXX20 && !defined(__clang__) +#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 && !defined(__clang__) +#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) } else { _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); @@ -864,7 +864,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif // _HAS_CXX20 && !defined(__clang__) +#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; From facabae9be50f6f2840846bca92e5b8d7b3bbd8c Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Sat, 11 Jul 2020 14:42:47 +0300 Subject: [PATCH 06/30] Skip test for clang, also more interesting bits --- tests/std/tests/P0528R3_cmpxchg_pad/test.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp index 4e34390cb78..991e4c5fba7 100644 --- a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp +++ b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp @@ -92,9 +92,9 @@ void test() { X x3; X x4; X x1; - std::memset(std::addressof(x1), 0x00, sizeof(x1)); - std::memset(std::addressof(x2), 0xff, sizeof(x1)); - std::memset(std::addressof(x3), 0xff, sizeof(x1)); + std::memset(std::addressof(x1), 0xaa, sizeof(x1)); + std::memset(std::addressof(x2), 0x55, sizeof(x1)); + std::memset(std::addressof(x3), 0x55, sizeof(x1)); x1.set(5); x2.set(5); x3.set(6); @@ -133,6 +133,7 @@ void test() { int main() { +#ifndef __clang__ test(); test(); test(); @@ -140,5 +141,6 @@ int main() { test(); test(); test(); +#endif // !__clang__ return 0; } From c072791e6f3347974b1d8f4df7b1eb76336fb1fb Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Sat, 11 Jul 2020 15:48:15 +0300 Subject: [PATCH 07/30] TRANSITION, LLVM-46685 --- stl/inc/atomic | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 864ac4cf563..6fa6e4ad0d1 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -354,18 +354,18 @@ struct _Atomic_storage { _Check_memory_order(_Order); const auto _Storage_ptr = _STD addressof(_Storage); const auto _Expected_ptr = _STD addressof(_Expected); -#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) if constexpr (!has_unique_object_representations_v<_Ty>) { __builtin_zero_non_value_bits(_Expected_ptr); } -#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) bool _Result; _Lock(); -#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) if constexpr (!has_unique_object_representations_v<_Ty>) { __builtin_zero_non_value_bits(_Storage_ptr); } -#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) if (_CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0) { _CSTD memcpy(_Storage_ptr, _STD addressof(_Desired), sizeof(_Ty)); _Result = true; @@ -493,15 +493,15 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics char _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation char _Prev_bytes; -#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) } else { _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); @@ -521,7 +521,7 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) reinterpret_cast(_Expected) = _Prev_bytes; return false; @@ -599,15 +599,15 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics const memory_order _Order = memory_order_seq_cst) noexcept { // CAS with given memory order short _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation short _Prev_bytes; -#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) } else { _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); @@ -627,7 +627,7 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -703,15 +703,15 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics const memory_order _Order = memory_order_seq_cst) noexcept { // CAS with given memory order long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation long _Prev_bytes; -#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) } else { _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); @@ -731,7 +731,7 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -835,15 +835,15 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics long long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation long long _Prev_bytes; -#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) } else { _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); @@ -864,7 +864,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif // _HAS_CXX20 && !defined(__clang__) && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; From 2250eecc88ceb50223ec43e501741b22be250c3a Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Sat, 11 Jul 2020 17:48:33 +0300 Subject: [PATCH 08/30] Improve TRANSITION comment --- stl/inc/atomic | 20 ++++++++++---------- tests/std/tests/P0528R3_cmpxchg_pad/test.cpp | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 6fa6e4ad0d1..93bcf244ea8 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -358,14 +358,14 @@ struct _Atomic_storage { if constexpr (!has_unique_object_representations_v<_Ty>) { __builtin_zero_non_value_bits(_Expected_ptr); } -#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) bool _Result; _Lock(); #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) if constexpr (!has_unique_object_representations_v<_Ty>) { __builtin_zero_non_value_bits(_Storage_ptr); } -#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) if (_CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0) { _CSTD memcpy(_Storage_ptr, _STD addressof(_Desired), sizeof(_Ty)); _Result = true; @@ -495,7 +495,7 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { @@ -521,7 +521,7 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) reinterpret_cast(_Expected) = _Prev_bytes; return false; @@ -601,7 +601,7 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics short _Prev_bytes; #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { @@ -627,7 +627,7 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -705,7 +705,7 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics long _Prev_bytes; #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { @@ -731,7 +731,7 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -837,7 +837,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) if constexpr (has_unique_object_representations_v<_Ty>) { -#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { @@ -864,7 +864,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } } -#endif // _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; diff --git a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp index 991e4c5fba7..2857435d65e 100644 --- a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp +++ b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp @@ -133,7 +133,7 @@ void test() { int main() { -#ifndef __clang__ +#ifndef __clang__ // TRANSITION, LLVM-46685 test(); test(); test(); @@ -141,6 +141,6 @@ int main() { test(); test(); test(); -#endif // !__clang__ +#endif // !__clang__, TRANSITION, LLVM-46685 return 0; } From cec43ef9aba6811a5268cf802914970ba86af4d3 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Sun, 12 Jul 2020 09:01:26 +0300 Subject: [PATCH 09/30] Don't use has_unique_object_representations_v --- stl/inc/atomic | 117 ++++++++++++++++--------------------------------- 1 file changed, 38 insertions(+), 79 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 93bcf244ea8..64d6b4cc20d 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -355,16 +355,12 @@ struct _Atomic_storage { const auto _Storage_ptr = _STD addressof(_Storage); const auto _Expected_ptr = _STD addressof(_Expected); #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) - if constexpr (!has_unique_object_representations_v<_Ty>) { - __builtin_zero_non_value_bits(_Expected_ptr); - } + __builtin_zero_non_value_bits(_Expected_ptr); #endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) bool _Result; _Lock(); #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) - if constexpr (!has_unique_object_representations_v<_Ty>) { - __builtin_zero_non_value_bits(_Storage_ptr); - } + __builtin_zero_non_value_bits(_Storage_ptr); #endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) if (_CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0) { _CSTD memcpy(_Storage_ptr, _STD addressof(_Desired), sizeof(_Ty)); @@ -494,7 +490,12 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics char _Prev_bytes; #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) - if constexpr (has_unique_object_representations_v<_Ty>) { + _Ty _Mask; + _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); + __builtin_zero_non_value_bits(_STD addressof(_Mask)); + const char _Mask_val = _Atomic_reinterpret_as(_Mask); + + for (;;) { #endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); @@ -502,24 +503,10 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics return true; } #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) - } else { - _Ty _Mask; - _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); - __builtin_zero_non_value_bits(_STD addressof(_Mask)); - const char _Mask_val = _Atomic_reinterpret_as(_Mask); - - for (;;) { - _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, - _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); - if (_Prev_bytes == _Expected_bytes) { - return true; - } - - if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { - break; - } - _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { + break; } + _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } #endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) @@ -600,7 +587,12 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics short _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation short _Prev_bytes; #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) - if constexpr (has_unique_object_representations_v<_Ty>) { + _Ty _Mask; + _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); + __builtin_zero_non_value_bits(_STD addressof(_Mask)); + const short _Mask_val = _Atomic_reinterpret_as(_Mask); + + for (;;) { #endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); @@ -608,24 +600,10 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics return true; } #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) - } else { - _Ty _Mask; - _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); - __builtin_zero_non_value_bits(_STD addressof(_Mask)); - const short _Mask_val = _Atomic_reinterpret_as(_Mask); - - for (;;) { - _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, - _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); - if (_Prev_bytes == _Expected_bytes) { - return true; - } - - if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { - break; - } - _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { + break; } + _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } #endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) @@ -704,7 +682,12 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation long _Prev_bytes; #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) - if constexpr (has_unique_object_representations_v<_Ty>) { + _Ty _Mask; + _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); + __builtin_zero_non_value_bits(_STD addressof(_Mask)); + const long _Mask_val = _Atomic_reinterpret_as(_Mask); + + for (;;) { #endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); @@ -712,24 +695,10 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics return true; } #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) - } else { - _Ty _Mask; - _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); - __builtin_zero_non_value_bits(_STD addressof(_Mask)); - const long _Mask_val = _Atomic_reinterpret_as(_Mask); - - for (;;) { - _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, - _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); - if (_Prev_bytes == _Expected_bytes) { - return true; - } - - if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { - break; - } - _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { + break; } + _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } #endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) @@ -836,7 +805,12 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics long long _Prev_bytes; #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) - if constexpr (has_unique_object_representations_v<_Ty>) { + _Ty _Mask; + _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); + __builtin_zero_non_value_bits(_STD addressof(_Mask)); + const long long _Mask_val = _Atomic_reinterpret_as(_Mask); + + for (;;) { #endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); @@ -844,25 +818,10 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics return true; } #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) - } else { - _Ty _Mask; - _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); - __builtin_zero_non_value_bits(_STD addressof(_Mask)); - const long long _Mask_val = _Atomic_reinterpret_as(_Mask); - - for (;;) { - _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, - _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), - _Expected_bytes); - if (_Prev_bytes == _Expected_bytes) { - return true; - } - - if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { - break; - } - _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { + break; } + _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } #endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) From c6a761497e4b1fca23095bff11437a975d9b72be Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Sun, 12 Jul 2020 09:08:51 +0300 Subject: [PATCH 10/30] longer --- stl/inc/atomic | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 64d6b4cc20d..703dde5526f 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -685,7 +685,7 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); __builtin_zero_non_value_bits(_STD addressof(_Mask)); - const long _Mask_val = _Atomic_reinterpret_as(_Mask); + const long _Mask_val = _Atomic_reinterpret_as(_Mask); for (;;) { #endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) @@ -808,7 +808,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); __builtin_zero_non_value_bits(_STD addressof(_Mask)); - const long long _Mask_val = _Atomic_reinterpret_as(_Mask); + const long long _Mask_val = _Atomic_reinterpret_as(_Mask); for (;;) { #endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) From de3b54da963ad383051aa9b273f98c64208343ce Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Wed, 15 Jul 2020 09:26:18 +0300 Subject: [PATCH 11/30] centralize preprocessor --- stl/inc/atomic | 49 +++++++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 703dde5526f..c9b171883d0 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -115,6 +115,15 @@ _NODISCARD extern "C" bool __cdecl __std_atomic_has_cmpxchg16b() noexcept; #define ATOMIC_LLONG_LOCK_FREE 2 #define ATOMIC_POINTER_LOCK_FREE 2 +// Padding bits should not participate in cmpxchg comparison starting C++20. +// Clang does not have __builtin_zero_non_value_bits to exclude these bits to implement this C++20 feature. +// EDG front-end substitutes everything and runs into incomplete types passed to atomic +#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#define _CMPXCHG_MASK_OUT_PADDING_BITS 1 +#else +#define _CMPXCHG_MASK_OUT_PADDING_BITS 0 +#endif + _STD_BEGIN // FENCES @@ -354,14 +363,14 @@ struct _Atomic_storage { _Check_memory_order(_Order); const auto _Storage_ptr = _STD addressof(_Storage); const auto _Expected_ptr = _STD addressof(_Expected); -#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#if _CMPXCHG_MASK_OUT_PADDING_BITS __builtin_zero_non_value_bits(_Expected_ptr); -#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS bool _Result; _Lock(); -#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#if _CMPXCHG_MASK_OUT_PADDING_BITS __builtin_zero_non_value_bits(_Storage_ptr); -#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS if (_CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0) { _CSTD memcpy(_Storage_ptr, _STD addressof(_Desired), sizeof(_Ty)); _Result = true; @@ -489,26 +498,26 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics char _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation char _Prev_bytes; -#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#if _CMPXCHG_MASK_OUT_PADDING_BITS _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); __builtin_zero_non_value_bits(_STD addressof(_Mask)); const char _Mask_val = _Atomic_reinterpret_as(_Mask); for (;;) { -#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#if _CMPXCHG_MASK_OUT_PADDING_BITS if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { break; } _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } -#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS reinterpret_cast(_Expected) = _Prev_bytes; return false; @@ -586,26 +595,26 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics const memory_order _Order = memory_order_seq_cst) noexcept { // CAS with given memory order short _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation short _Prev_bytes; -#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#if _CMPXCHG_MASK_OUT_PADDING_BITS _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); __builtin_zero_non_value_bits(_STD addressof(_Mask)); const short _Mask_val = _Atomic_reinterpret_as(_Mask); for (;;) { -#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#if _CMPXCHG_MASK_OUT_PADDING_BITS if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { break; } _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } -#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -681,26 +690,26 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics const memory_order _Order = memory_order_seq_cst) noexcept { // CAS with given memory order long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation long _Prev_bytes; -#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#if _CMPXCHG_MASK_OUT_PADDING_BITS _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); __builtin_zero_non_value_bits(_STD addressof(_Mask)); const long _Mask_val = _Atomic_reinterpret_as(_Mask); for (;;) { -#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#if _CMPXCHG_MASK_OUT_PADDING_BITS if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { break; } _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } -#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -804,26 +813,26 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics long long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation long long _Prev_bytes; -#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#if _CMPXCHG_MASK_OUT_PADDING_BITS _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); __builtin_zero_non_value_bits(_STD addressof(_Mask)); const long long _Mask_val = _Atomic_reinterpret_as(_Mask); for (;;) { -#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { return true; } -#if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) +#if _CMPXCHG_MASK_OUT_PADDING_BITS if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { break; } _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } -#endif // _HAS_CXX20 && !defined(__clang__), TRANSITION, LLVM-46685 && !defined(__EDG__) +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; From 979cac7d1ca03fd9fedf6e2417a1df3965d87a9d Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Wed, 15 Jul 2020 09:28:50 +0300 Subject: [PATCH 12/30] test cleanup --- tests/std/tests/P0528R3_cmpxchg_pad/test.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp index 2857435d65e..94748b368c3 100644 --- a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp +++ b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp @@ -9,6 +9,8 @@ struct X1 { char x : 6; + void operator&() const = delete; + void set(char v) { x = v; } @@ -17,6 +19,8 @@ struct X1 { struct X2 { short x : 9; + void operator&() const = delete; + void set(char v) { x = v; } @@ -29,6 +33,8 @@ struct X3 { char y : 1; short z; + void operator&() const = delete; + void set(char v) { x = v; y = 0; @@ -52,7 +58,7 @@ struct X8 { char x; long y; - void operator&() const {} + void operator&() const = delete; void set(char v) { x = v; @@ -65,6 +71,8 @@ struct X16 { char y; long long z; + void operator&() const = delete; + void set(char v) { x = v; y = 0; @@ -77,6 +85,8 @@ struct X20 { long y[3]; char z; + void operator&() const = delete; + void set(char v) { x = v; std::memset(&y, 0, sizeof(y)); @@ -88,13 +98,14 @@ struct X20 { template void test() { static_assert(sizeof(X) == S, "Unexpected size"); + X x1; X x2; X x3; X x4; - X x1; std::memset(std::addressof(x1), 0xaa, sizeof(x1)); - std::memset(std::addressof(x2), 0x55, sizeof(x1)); - std::memset(std::addressof(x3), 0x55, sizeof(x1)); + std::memset(std::addressof(x2), 0x55, sizeof(x2)); + std::memset(std::addressof(x3), 0x55, sizeof(x3)); + std::memset(std::addressof(x4), 0x55, sizeof(x4)); x1.set(5); x2.set(5); x3.set(6); From 56793a7632a69d33b9d44548b19fc17c96ecd2a5 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Wed, 15 Jul 2020 09:29:58 +0300 Subject: [PATCH 13/30] undef --- stl/inc/atomic | 2 ++ 1 file changed, 2 insertions(+) diff --git a/stl/inc/atomic b/stl/inc/atomic index c9b171883d0..1bb3ad3de05 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -2180,6 +2180,8 @@ inline void atomic_flag_clear_explicit(volatile atomic_flag* _Flag, memory_order _STD_END +#undef _CMPXCHG_MASK_OUT_PADDING_BITS + #undef _ATOMIC_CHOOSE_INTRINSIC #undef _ATOMIC_HAS_DCAS From dcdfa482207701dd732f460c9e5ed71e240fcf1a Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Wed, 15 Jul 2020 12:03:05 +0300 Subject: [PATCH 14/30] Update stl/inc/atomic Co-authored-by: Stephan T. Lavavej --- stl/inc/atomic | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 1bb3ad3de05..83c2250ce85 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -117,7 +117,7 @@ _NODISCARD extern "C" bool __cdecl __std_atomic_has_cmpxchg16b() noexcept; // Padding bits should not participate in cmpxchg comparison starting C++20. // Clang does not have __builtin_zero_non_value_bits to exclude these bits to implement this C++20 feature. -// EDG front-end substitutes everything and runs into incomplete types passed to atomic +// The EDG front-end substitutes everything and runs into incomplete types passed to atomic. #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) #define _CMPXCHG_MASK_OUT_PADDING_BITS 1 #else From 97076e464c354178fa4f0583753114c6b4de9e3c Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Wed, 15 Jul 2020 12:03:48 +0300 Subject: [PATCH 15/30] Update stl/inc/atomic Co-authored-by: Stephan T. Lavavej --- stl/inc/atomic | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 83c2250ce85..6d11f4a9f2b 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -115,7 +115,7 @@ _NODISCARD extern "C" bool __cdecl __std_atomic_has_cmpxchg16b() noexcept; #define ATOMIC_LLONG_LOCK_FREE 2 #define ATOMIC_POINTER_LOCK_FREE 2 -// Padding bits should not participate in cmpxchg comparison starting C++20. +// Padding bits should not participate in cmpxchg comparison starting in C++20. // Clang does not have __builtin_zero_non_value_bits to exclude these bits to implement this C++20 feature. // The EDG front-end substitutes everything and runs into incomplete types passed to atomic. #if _HAS_CXX20 && !defined(__clang__) /* TRANSITION, LLVM-46685 */ && !defined(__EDG__) From 1a3e44e9ee1acf2a92b591b0d170cc704e431681 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Wed, 15 Jul 2020 12:05:11 +0300 Subject: [PATCH 16/30] delete the remaining & --- tests/std/tests/P0528R3_cmpxchg_pad/test.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp index 94748b368c3..2db35887a81 100644 --- a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp +++ b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp @@ -48,6 +48,8 @@ struct X3 { struct alignas(4) X4 { char x; + void operator&() const = delete; + void set(char v) { x = v; } From 465ebe17e40ac51293c14d719b5d8dc5bef8e827 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 17 Jul 2020 07:21:04 +0300 Subject: [PATCH 17/30] Don't penalize normal atomics in debug mode bring back has_unique_object_representations_v --- stl/inc/atomic | 172 +++++++++++++++++++++++++++++-------------------- 1 file changed, 102 insertions(+), 70 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 6d11f4a9f2b..23e25aa98f9 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -126,6 +126,12 @@ _NODISCARD extern "C" bool __cdecl __std_atomic_has_cmpxchg16b() noexcept; _STD_BEGIN +#if _HAS_CXX20 +template +inline constexpr bool _Cmpxchg_has_padding_bits_v = + !has_unique_object_representations_v<_Ty> && !is_floating_point_v<_Ty>; +#endif + // FENCES extern "C" inline void atomic_thread_fence(const memory_order _Order) noexcept { if (_Order == memory_order_relaxed) { @@ -364,12 +370,16 @@ struct _Atomic_storage { const auto _Storage_ptr = _STD addressof(_Storage); const auto _Expected_ptr = _STD addressof(_Expected); #if _CMPXCHG_MASK_OUT_PADDING_BITS - __builtin_zero_non_value_bits(_Expected_ptr); + if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + __builtin_zero_non_value_bits(_Expected_ptr); + } #endif // _CMPXCHG_MASK_OUT_PADDING_BITS bool _Result; _Lock(); #if _CMPXCHG_MASK_OUT_PADDING_BITS - __builtin_zero_non_value_bits(_Storage_ptr); + if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + __builtin_zero_non_value_bits(_Storage_ptr); + } #endif // _CMPXCHG_MASK_OUT_PADDING_BITS if (_CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0) { _CSTD memcpy(_Storage_ptr, _STD addressof(_Desired), sizeof(_Ty)); @@ -499,26 +509,32 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics char _Prev_bytes; #if _CMPXCHG_MASK_OUT_PADDING_BITS - _Ty _Mask; - _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); - __builtin_zero_non_value_bits(_STD addressof(_Mask)); - const char _Mask_val = _Atomic_reinterpret_as(_Mask); - - for (;;) { -#endif // _CMPXCHG_MASK_OUT_PADDING_BITS - _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, - _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); - if (_Prev_bytes == _Expected_bytes) { - return true; + if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + _Ty _Mask; + _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); + __builtin_zero_non_value_bits(_STD addressof(_Mask)); + const char _Mask_val = _Atomic_reinterpret_as(_Mask); + + for (;;) { + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } + + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { + reinterpret_cast(_Expected) = _Prev_bytes; + return false; + } + _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } -#if _CMPXCHG_MASK_OUT_PADDING_BITS - if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { - break; - } - _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } #endif // _CMPXCHG_MASK_OUT_PADDING_BITS - + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, _Atomic_address_as(_Storage), + _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } reinterpret_cast(_Expected) = _Prev_bytes; return false; } @@ -596,26 +612,31 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics short _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation short _Prev_bytes; #if _CMPXCHG_MASK_OUT_PADDING_BITS - _Ty _Mask; - _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); - __builtin_zero_non_value_bits(_STD addressof(_Mask)); - const short _Mask_val = _Atomic_reinterpret_as(_Mask); - - for (;;) { -#endif // _CMPXCHG_MASK_OUT_PADDING_BITS - _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, - _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); - if (_Prev_bytes == _Expected_bytes) { - return true; - } -#if _CMPXCHG_MASK_OUT_PADDING_BITS - if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { - break; + if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + _Ty _Mask; + _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); + __builtin_zero_non_value_bits(_STD addressof(_Mask)); + const short _Mask_val = _Atomic_reinterpret_as(_Mask); + + for (;;) { + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { + _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); + return false; + } + _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } - _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } #endif // _CMPXCHG_MASK_OUT_PADDING_BITS - + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; } @@ -691,26 +712,31 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation long _Prev_bytes; #if _CMPXCHG_MASK_OUT_PADDING_BITS - _Ty _Mask; - _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); - __builtin_zero_non_value_bits(_STD addressof(_Mask)); - const long _Mask_val = _Atomic_reinterpret_as(_Mask); - - for (;;) { -#endif // _CMPXCHG_MASK_OUT_PADDING_BITS - _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, - _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); - if (_Prev_bytes == _Expected_bytes) { - return true; - } -#if _CMPXCHG_MASK_OUT_PADDING_BITS - if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { - break; + if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + _Ty _Mask; + _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); + __builtin_zero_non_value_bits(_STD addressof(_Mask)); + const long _Mask_val = _Atomic_reinterpret_as(_Mask); + + if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { + _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); + return false; + } + _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } - _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } #endif // _CMPXCHG_MASK_OUT_PADDING_BITS - + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, _Atomic_address_as(_Storage), + _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; } @@ -814,26 +840,32 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics long long _Prev_bytes; #if _CMPXCHG_MASK_OUT_PADDING_BITS - _Ty _Mask; - _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); - __builtin_zero_non_value_bits(_STD addressof(_Mask)); - const long long _Mask_val = _Atomic_reinterpret_as(_Mask); - - for (;;) { -#endif // _CMPXCHG_MASK_OUT_PADDING_BITS - _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, - _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); - if (_Prev_bytes == _Expected_bytes) { - return true; + if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + _Ty _Mask; + _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); + __builtin_zero_non_value_bits(_STD addressof(_Mask)); + const long long _Mask_val = _Atomic_reinterpret_as(_Mask); + + for (;;) { + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), + _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { + _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); + return false; + } + _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } -#if _CMPXCHG_MASK_OUT_PADDING_BITS - if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { - break; - } - _Expected_bytes = (_Expected_bytes & _Mask_val) | (_Prev_bytes & ~_Mask_val); } #endif // _CMPXCHG_MASK_OUT_PADDING_BITS - + _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange64, + _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); + if (_Prev_bytes == _Expected_bytes) { + return true; + } _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; } From ccbf24bb505fd7359870df9bc0515dbf56d07b5f Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 17 Jul 2020 07:23:14 +0300 Subject: [PATCH 18/30] non-chained --- stl/inc/atomic | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/stl/inc/atomic b/stl/inc/atomic index 23e25aa98f9..273d580b7c2 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -535,6 +535,7 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics if (_Prev_bytes == _Expected_bytes) { return true; } + reinterpret_cast(_Expected) = _Prev_bytes; return false; } @@ -624,6 +625,7 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics if (_Prev_bytes == _Expected_bytes) { return true; } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -637,6 +639,7 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics if (_Prev_bytes == _Expected_bytes) { return true; } + _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; } @@ -724,6 +727,7 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics if (_Prev_bytes == _Expected_bytes) { return true; } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -737,6 +741,7 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics if (_Prev_bytes == _Expected_bytes) { return true; } + _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; } @@ -853,6 +858,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics if (_Prev_bytes == _Expected_bytes) { return true; } + if ((_Prev_bytes ^ _Expected_bytes) & _Mask_val) { _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; @@ -866,6 +872,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics if (_Prev_bytes == _Expected_bytes) { return true; } + _CSTD memcpy(_STD addressof(_Expected), &_Prev_bytes, sizeof(_Ty)); return false; } From ef818b7933934a782a509e53a7b5b75eea1e8bd0 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 17 Jul 2020 07:25:06 +0300 Subject: [PATCH 19/30] for --- stl/inc/atomic | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 273d580b7c2..a137c34f976 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -721,7 +721,7 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics __builtin_zero_non_value_bits(_STD addressof(_Mask)); const long _Mask_val = _Atomic_reinterpret_as(_Mask); - if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + for (;;) { _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange, _Atomic_address_as(_Storage), _Atomic_reinterpret_as(_Desired), _Expected_bytes); if (_Prev_bytes == _Expected_bytes) { From d9620b04902dad98fd9b0e29ca2436c4004cc36d Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 17 Jul 2020 07:39:10 +0300 Subject: [PATCH 20/30] +zero case --- tests/std/tests/P0528R3_cmpxchg_pad/test.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp index 2db35887a81..631b2c03305 100644 --- a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp +++ b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp @@ -6,6 +6,11 @@ #include #include +struct X0 { + void operator&() const = delete; +}; + + struct X1 { char x : 6; @@ -145,8 +150,23 @@ void test() { } +template +void test0() { + X x1; + X x2; + std::memset(std::addressof(x1), 0xaa, sizeof(x1)); + std::memset(std::addressof(x2), 0x55, sizeof(x2)); + + std::atomic v; + v.store(x1); + X x; + + assert(v.compare_exchange_strong(x, x2)); +} + int main() { #ifndef __clang__ // TRANSITION, LLVM-46685 + test0(); test(); test(); test(); From 69eb997c1d8c2672c61f056227de41e08f1c6251 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 17 Jul 2020 07:45:48 +0300 Subject: [PATCH 21/30] clang format --- stl/inc/atomic | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index a137c34f976..439ced3b834 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -509,7 +509,7 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics char _Prev_bytes; #if _CMPXCHG_MASK_OUT_PADDING_BITS - if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { _Ty _Mask; _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); __builtin_zero_non_value_bits(_STD addressof(_Mask)); From 1662f1c74a72f04e8bd927facb501c77ac67372c Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Fri, 17 Jul 2020 07:51:06 +0300 Subject: [PATCH 22/30] finish up zero case --- tests/std/tests/P0528R3_cmpxchg_pad/test.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp index 631b2c03305..a232e55201b 100644 --- a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp +++ b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp @@ -154,12 +154,15 @@ template void test0() { X x1; X x2; + X x3; std::memset(std::addressof(x1), 0xaa, sizeof(x1)); std::memset(std::addressof(x2), 0x55, sizeof(x2)); + std::memset(std::addressof(x3), 0x55, sizeof(x3)); std::atomic v; v.store(x1); X x; + std::memcpy(std::addressof(x), std::addressof(x3), sizeof(x)); assert(v.compare_exchange_strong(x, x2)); } From 0a35a47ecd35303de7d3507114d9f0b1654f06e8 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Sat, 18 Jul 2020 14:56:59 +0300 Subject: [PATCH 23/30] Review comments on non-lock-free case As @BillyONeal suggested, avoid writing to shared data on CAS fail and avoid check for lone __builtin_zero_non_value_bits as it is no-op for normal types --- stl/inc/atomic | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 439ced3b834..36ba1a7b4a9 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -369,24 +369,26 @@ struct _Atomic_storage { _Check_memory_order(_Order); const auto _Storage_ptr = _STD addressof(_Storage); const auto _Expected_ptr = _STD addressof(_Expected); -#if _CMPXCHG_MASK_OUT_PADDING_BITS - if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { - __builtin_zero_non_value_bits(_Expected_ptr); - } -#endif // _CMPXCHG_MASK_OUT_PADDING_BITS bool _Result; + __builtin_zero_non_value_bits(_Expected_ptr); _Lock(); #if _CMPXCHG_MASK_OUT_PADDING_BITS if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { - __builtin_zero_non_value_bits(_Storage_ptr); + _Ty _Local; + const auto _Local_ptr = _STD addressof(_Local); + _CSTD memcpy(_Local_ptr, _Storage_ptr, sizeof(_Ty)); + __builtin_zero_non_value_bits(_STD addressof(_Local)); + _Result = _CSTD memcmp(_Local_ptr, _Expected_ptr, sizeof(_Ty)) == 0; + } else { + _Result = _CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0; } +#else + _Result = _CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0; #endif // _CMPXCHG_MASK_OUT_PADDING_BITS - if (_CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0) { + if (_Result) { _CSTD memcpy(_Storage_ptr, _STD addressof(_Desired), sizeof(_Ty)); - _Result = true; } else { _CSTD memcpy(_Expected_ptr, _Storage_ptr, sizeof(_Ty)); - _Result = false; } _Unlock(); From 9d96b8e56b95eb91e1c8ab8960ab98d0b50d008f Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Sat, 18 Jul 2020 14:57:55 +0300 Subject: [PATCH 24/30] -addressof --- stl/inc/atomic | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 36ba1a7b4a9..763480ee112 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -377,7 +377,7 @@ struct _Atomic_storage { _Ty _Local; const auto _Local_ptr = _STD addressof(_Local); _CSTD memcpy(_Local_ptr, _Storage_ptr, sizeof(_Ty)); - __builtin_zero_non_value_bits(_STD addressof(_Local)); + __builtin_zero_non_value_bits(_Local_ptr); _Result = _CSTD memcmp(_Local_ptr, _Expected_ptr, sizeof(_Ty)) == 0; } else { _Result = _CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0; From 00e7416e67e4dd8d76f133b54246a92fe1c1c1a3 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Sat, 18 Jul 2020 17:23:37 +0300 Subject: [PATCH 25/30] more sizes --- tests/std/tests/P0528R3_cmpxchg_pad/test.cpp | 92 +++++++++++++++++--- 1 file changed, 82 insertions(+), 10 deletions(-) diff --git a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp index a232e55201b..8cd1e430d4f 100644 --- a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp +++ b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp @@ -16,9 +16,13 @@ struct X1 { void operator&() const = delete; - void set(char v) { + void set(const char v) { x = v; } + + bool check(const char v) const { + return x == v; + } }; struct X2 { @@ -26,9 +30,13 @@ struct X2 { void operator&() const = delete; - void set(char v) { + void set(const char v) { x = v; } + + bool check(const char v) const { + return x == v; + } }; #pragma pack(push, 1) @@ -40,10 +48,14 @@ struct X3 { void operator&() const = delete; - void set(char v) { + void set(const char v) { x = v; y = 0; - z = 0; + z = ~v; + } + + bool check(const char v) const { + return x == v && z == ~v; } }; #pragma pack(pop) @@ -58,8 +70,35 @@ struct alignas(4) X4 { void set(char v) { x = v; } + + bool check(const char v) const { + return x == v; + } +}; +#pragma warning(pop) + +#pragma pack(push) +#pragma warning(push) +#pragma warning(disable : 4324) // '%s': structure was padded due to alignment specifier +struct X6 { + char x; + alignas(2) char y[2]; + char z; + + void operator&() const = delete; + + void set(char v) { + x = v; + std::memset(&y, 0, sizeof(y)); + z = ~v; + } + + bool check(const char v) const { + return x == v && z == ~v; + } }; #pragma warning(pop) +#pragma pack(pop) struct X8 { char x; @@ -71,8 +110,30 @@ struct X8 { x = v; y = 0; } + + bool check(const char v) const { + return x == v; + } }; +#pragma pack(push, 1) +struct X9 { + X8 x; + char z; + + void operator&() const = delete; + + void set(char v) { + x.set(v); + z = ~v; + } + + bool check(const char v) const { + return x.check(v) && z == ~v; + } +}; +#pragma pack(pop) + struct X16 { long x; char y; @@ -83,7 +144,11 @@ struct X16 { void set(char v) { x = v; y = 0; - z = 0; + z = ~v; + } + + bool check(const char v) const { + return x == v && z == ~v; } }; @@ -97,7 +162,11 @@ struct X20 { void set(char v) { x = v; std::memset(&y, 0, sizeof(y)); - z = 0; + z = ~v; + } + + bool check(const char v) const { + return x == v && z == ~v; } }; @@ -105,6 +174,7 @@ struct X20 { template void test() { static_assert(sizeof(X) == S, "Unexpected size"); + static_assert(!std::has_unique_object_representations_v, "No padding type"); X x1; X x2; X x3; @@ -123,20 +193,20 @@ void test() { X x; std::memcpy(std::addressof(x), std::addressof(x3), sizeof(x)); assert(!v.compare_exchange_strong(x, x4)); - assert(v.load().x == 5); + assert(v.load().check(5)); v.store(x1); for (int retry = 0; retry != 10; ++retry) { X xw; std::memcpy(std::addressof(xw), std::addressof(x3), sizeof(x)); assert(!v.compare_exchange_weak(xw, x4)); - assert(v.load().x == 5); + assert(v.load().check(5)); } v.store(x1); std::memcpy(std::addressof(x), std::addressof(x2), sizeof(x)); assert(v.compare_exchange_strong(x, x3)); - assert(v.load().x == 6); + assert(v.load().check(6)); v.store(x1); for (;;) { @@ -146,7 +216,7 @@ void test() { break; } } - assert(v.load().x == 6); + assert(v.load().check(6)); } @@ -174,7 +244,9 @@ int main() { test(); test(); test(); + test(); test(); + test(); test(); test(); #endif // !__clang__, TRANSITION, LLVM-46685 From 2c05e4f5918d67150edea8959b1b1b49f7044892 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Sat, 18 Jul 2020 21:27:06 +0300 Subject: [PATCH 26/30] missing macro wrap --- stl/inc/atomic | 2 ++ 1 file changed, 2 insertions(+) diff --git a/stl/inc/atomic b/stl/inc/atomic index 763480ee112..8a823f8c6e2 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -370,7 +370,9 @@ struct _Atomic_storage { const auto _Storage_ptr = _STD addressof(_Storage); const auto _Expected_ptr = _STD addressof(_Expected); bool _Result; +#if _CMPXCHG_MASK_OUT_PADDING_BITS __builtin_zero_non_value_bits(_Expected_ptr); +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS _Lock(); #if _CMPXCHG_MASK_OUT_PADDING_BITS if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { From 1b75b4e1651ef5e538d2fea6cc12decda882e4e8 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 20 Jul 2020 16:48:28 -0700 Subject: [PATCH 27/30] Fix compare_exchange when _Ty has a nontrivial default ctor using _Storage_for formerly of . --- stl/inc/atomic | 50 ++++++++++++++++++++++++++++++++--------------- stl/inc/execution | 14 ------------- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 8a823f8c6e2..7712f3e8ac2 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -125,6 +125,32 @@ _NODISCARD extern "C" bool __cdecl __std_atomic_has_cmpxchg16b() noexcept; #endif _STD_BEGIN +// STRUCT TEMPLATE _Storage_for +struct _Form_mask_t {}; +_INLINE_VAR constexpr _Form_mask_t _Form_mask; + +template +struct _Storage_for { + // uninitialized space to store a _Ty + alignas(_Ty) unsigned char _Storage[sizeof(_Ty)]; + + _Storage_for() = default; + _Storage_for(const _Storage_for&) = delete; + _Storage_for& operator=(const _Storage_for&) = delete; + + explicit _Storage_for(_Form_mask_t) noexcept { + _CSTD memset(_Storage, 0xff, sizeof(_Ty)); + __builtin_zero_non_value_bits(_Ptr()); + } + + _Ty& _Ref() noexcept { + return reinterpret_cast<_Ty&>(_Storage); + } + + _Ty* _Ptr() noexcept { + return reinterpret_cast<_Ty*>(_STD addressof(_Storage)); + } +}; #if _HAS_CXX20 template @@ -376,8 +402,8 @@ struct _Atomic_storage { _Lock(); #if _CMPXCHG_MASK_OUT_PADDING_BITS if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { - _Ty _Local; - const auto _Local_ptr = _STD addressof(_Local); + _Storage_for<_Ty> _Local; + const auto _Local_ptr = _Local._Ptr(); _CSTD memcpy(_Local_ptr, _Storage_ptr, sizeof(_Ty)); __builtin_zero_non_value_bits(_Local_ptr); _Result = _CSTD memcmp(_Local_ptr, _Expected_ptr, sizeof(_Ty)) == 0; @@ -514,10 +540,8 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics #if _CMPXCHG_MASK_OUT_PADDING_BITS if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { - _Ty _Mask; - _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); - __builtin_zero_non_value_bits(_STD addressof(_Mask)); - const char _Mask_val = _Atomic_reinterpret_as(_Mask); + _Storage_for<_Ty> _Mask{_Form_mask}; + const long long _Mask_val = _Atomic_reinterpret_as(_Mask._Ref()); for (;;) { _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, @@ -618,10 +642,8 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics short _Prev_bytes; #if _CMPXCHG_MASK_OUT_PADDING_BITS if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { - _Ty _Mask; - _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); - __builtin_zero_non_value_bits(_STD addressof(_Mask)); - const short _Mask_val = _Atomic_reinterpret_as(_Mask); + _Storage_for<_Ty> _Mask{_Form_mask}; + const short _Mask_val = _Atomic_reinterpret_as(_Mask._Ref()); for (;;) { _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange16, @@ -720,9 +742,7 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics long _Prev_bytes; #if _CMPXCHG_MASK_OUT_PADDING_BITS if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { - _Ty _Mask; - _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); - __builtin_zero_non_value_bits(_STD addressof(_Mask)); + _Storage_for<_Ty> _Mask{_Form_mask}; const long _Mask_val = _Atomic_reinterpret_as(_Mask); for (;;) { @@ -850,9 +870,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics #if _CMPXCHG_MASK_OUT_PADDING_BITS if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { - _Ty _Mask; - _CSTD memset(_STD addressof(_Mask), 0xff, sizeof(_Ty)); - __builtin_zero_non_value_bits(_STD addressof(_Mask)); + _Storage_for<_Ty> _Mask{_Form_mask}; const long long _Mask_val = _Atomic_reinterpret_as(_Mask); for (;;) { diff --git a/stl/inc/execution b/stl/inc/execution index d632e4c4788..18ccbc1d496 100644 --- a/stl/inc/execution +++ b/stl/inc/execution @@ -3590,20 +3590,6 @@ _FwdIt partition(_ExPo&&, _FwdIt _First, const _FwdIt _Last, _Pr _Pred) noexcept } // PARALLEL FUNCTION TEMPLATE set_intersection -template -struct _Storage_for { - // uninitialized space to store a _Ty - alignas(_Ty) unsigned char _Storage[sizeof(_Ty)]; - - _Storage_for() = default; - _Storage_for(const _Storage_for&) = delete; - _Storage_for& operator=(const _Storage_for&) = delete; - - _Ty& _Ref() { - return reinterpret_cast<_Ty&>(_Storage); - } -}; - inline constexpr unsigned char _Local_available = 1; inline constexpr unsigned char _Sum_available = 2; From 76de2ad030a65e4fbcab0217e105100a073664de Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 20 Jul 2020 18:55:39 -0700 Subject: [PATCH 28/30] Fix STL CR comments. --- stl/inc/atomic | 32 +++++++++++--------- tests/std/tests/P0528R3_cmpxchg_pad/test.cpp | 18 +++++------ 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 7712f3e8ac2..4d6268034ab 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -126,8 +126,10 @@ _NODISCARD extern "C" bool __cdecl __std_atomic_has_cmpxchg16b() noexcept; _STD_BEGIN // STRUCT TEMPLATE _Storage_for +#if _CMPXCHG_MASK_OUT_PADDING_BITS struct _Form_mask_t {}; -_INLINE_VAR constexpr _Form_mask_t _Form_mask; +_INLINE_VAR constexpr _Form_mask_t _Form_mask{}; +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS template struct _Storage_for { @@ -138,25 +140,27 @@ struct _Storage_for { _Storage_for(const _Storage_for&) = delete; _Storage_for& operator=(const _Storage_for&) = delete; +#if _CMPXCHG_MASK_OUT_PADDING_BITS explicit _Storage_for(_Form_mask_t) noexcept { _CSTD memset(_Storage, 0xff, sizeof(_Ty)); __builtin_zero_non_value_bits(_Ptr()); } +#endif - _Ty& _Ref() noexcept { + _NODISCARD _Ty& _Ref() noexcept { return reinterpret_cast<_Ty&>(_Storage); } - _Ty* _Ptr() noexcept { - return reinterpret_cast<_Ty*>(_STD addressof(_Storage)); + _NODISCARD _Ty* _Ptr() noexcept { + return reinterpret_cast<_Ty*>(&_Storage); } }; -#if _HAS_CXX20 +#if _CMPXCHG_MASK_OUT_PADDING_BITS template -inline constexpr bool _Cmpxchg_has_padding_bits_v = +inline constexpr bool _Might_have_non_value_bits = !has_unique_object_representations_v<_Ty> && !is_floating_point_v<_Ty>; -#endif +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS // FENCES extern "C" inline void atomic_thread_fence(const memory_order _Order) noexcept { @@ -401,7 +405,7 @@ struct _Atomic_storage { #endif // _CMPXCHG_MASK_OUT_PADDING_BITS _Lock(); #if _CMPXCHG_MASK_OUT_PADDING_BITS - if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + if constexpr (_Might_have_non_value_bits<_Ty>) { _Storage_for<_Ty> _Local; const auto _Local_ptr = _Local._Ptr(); _CSTD memcpy(_Local_ptr, _Storage_ptr, sizeof(_Ty)); @@ -410,7 +414,7 @@ struct _Atomic_storage { } else { _Result = _CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0; } -#else +#else // _CMPXCHG_MASK_OUT_PADDING_BITS _Result = _CSTD memcmp(_Storage_ptr, _Expected_ptr, sizeof(_Ty)) == 0; #endif // _CMPXCHG_MASK_OUT_PADDING_BITS if (_Result) { @@ -539,9 +543,9 @@ struct _Atomic_storage<_Ty, 1> { // lock-free using 1-byte intrinsics char _Prev_bytes; #if _CMPXCHG_MASK_OUT_PADDING_BITS - if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + if constexpr (_Might_have_non_value_bits<_Ty>) { _Storage_for<_Ty> _Mask{_Form_mask}; - const long long _Mask_val = _Atomic_reinterpret_as(_Mask._Ref()); + const char _Mask_val = _Atomic_reinterpret_as(_Mask._Ref()); for (;;) { _ATOMIC_CHOOSE_INTRINSIC(_Order, _Prev_bytes, _InterlockedCompareExchange8, @@ -641,7 +645,7 @@ struct _Atomic_storage<_Ty, 2> { // lock-free using 2-byte intrinsics short _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation short _Prev_bytes; #if _CMPXCHG_MASK_OUT_PADDING_BITS - if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + if constexpr (_Might_have_non_value_bits<_Ty>) { _Storage_for<_Ty> _Mask{_Form_mask}; const short _Mask_val = _Atomic_reinterpret_as(_Mask._Ref()); @@ -741,7 +745,7 @@ struct _Atomic_storage<_Ty, 4> { // lock-free using 4-byte intrinsics long _Expected_bytes = _Atomic_reinterpret_as(_Expected); // read before atomic operation long _Prev_bytes; #if _CMPXCHG_MASK_OUT_PADDING_BITS - if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + if constexpr (_Might_have_non_value_bits<_Ty>) { _Storage_for<_Ty> _Mask{_Form_mask}; const long _Mask_val = _Atomic_reinterpret_as(_Mask); @@ -869,7 +873,7 @@ struct _Atomic_storage<_Ty, 8> { // lock-free using 8-byte intrinsics long long _Prev_bytes; #if _CMPXCHG_MASK_OUT_PADDING_BITS - if constexpr (_Cmpxchg_has_padding_bits_v<_Ty>) { + if constexpr (_Might_have_non_value_bits<_Ty>) { _Storage_for<_Ty> _Mask{_Form_mask}; const long long _Mask_val = _Atomic_reinterpret_as(_Mask); diff --git a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp index 8cd1e430d4f..748e3c1f34e 100644 --- a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp +++ b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp @@ -5,6 +5,7 @@ #include #include #include +#include struct X0 { void operator&() const = delete; @@ -67,7 +68,7 @@ struct alignas(4) X4 { void operator&() const = delete; - void set(char v) { + void set(const char v) { x = v; } @@ -77,7 +78,6 @@ struct alignas(4) X4 { }; #pragma warning(pop) -#pragma pack(push) #pragma warning(push) #pragma warning(disable : 4324) // '%s': structure was padded due to alignment specifier struct X6 { @@ -87,7 +87,7 @@ struct X6 { void operator&() const = delete; - void set(char v) { + void set(const char v) { x = v; std::memset(&y, 0, sizeof(y)); z = ~v; @@ -98,7 +98,6 @@ struct X6 { } }; #pragma warning(pop) -#pragma pack(pop) struct X8 { char x; @@ -106,7 +105,7 @@ struct X8 { void operator&() const = delete; - void set(char v) { + void set(const char v) { x = v; y = 0; } @@ -123,7 +122,7 @@ struct X9 { void operator&() const = delete; - void set(char v) { + void set(const char v) { x.set(v); z = ~v; } @@ -141,7 +140,7 @@ struct X16 { void operator&() const = delete; - void set(char v) { + void set(const char v) { x = v; y = 0; z = ~v; @@ -159,7 +158,7 @@ struct X20 { void operator&() const = delete; - void set(char v) { + void set(const char v) { x = v; std::memset(&y, 0, sizeof(y)); z = ~v; @@ -174,7 +173,8 @@ struct X20 { template void test() { static_assert(sizeof(X) == S, "Unexpected size"); - static_assert(!std::has_unique_object_representations_v, "No padding type"); + static_assert( + !std::has_unique_object_representations_v, "Type has no padding which makes testing for P0528 ineffective"); X x1; X x2; X x3; From 5280628c394f1f19e67790cbae29553942780435 Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Mon, 20 Jul 2020 19:20:18 -0700 Subject: [PATCH 29/30] Update tests/std/tests/P0528R3_cmpxchg_pad/test.cpp Co-authored-by: Casey Carter --- tests/std/tests/P0528R3_cmpxchg_pad/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp index 748e3c1f34e..44419f57410 100644 --- a/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp +++ b/tests/std/tests/P0528R3_cmpxchg_pad/test.cpp @@ -174,7 +174,7 @@ template void test() { static_assert(sizeof(X) == S, "Unexpected size"); static_assert( - !std::has_unique_object_representations_v, "Type has no padding which makes testing for P0528 ineffective"); + !std::has_unique_object_representations_v, "Type without padding is not useful for testing P0528."); X x1; X x2; X x3; From c2feae94c98bf7182a9416080c5e0f75d3b2ef1e Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Mon, 20 Jul 2020 20:51:54 -0700 Subject: [PATCH 30/30] Update stl/inc/atomic Co-authored-by: Stephan T. Lavavej --- stl/inc/atomic | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 4d6268034ab..80377e2c722 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -145,7 +145,7 @@ struct _Storage_for { _CSTD memset(_Storage, 0xff, sizeof(_Ty)); __builtin_zero_non_value_bits(_Ptr()); } -#endif +#endif // _CMPXCHG_MASK_OUT_PADDING_BITS _NODISCARD _Ty& _Ref() noexcept { return reinterpret_cast<_Ty&>(_Storage);