From db8583b319b12e45efb1f3f7fc04dfeb69eb5316 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 14 Mar 2024 12:14:03 +0200 Subject: [PATCH 01/13] 16-byte atomic_ref is always lock free with DCAS --- stl/inc/atomic | 11 ++++++++--- tests/std/tests/P0019R8_atomic_ref/test.cpp | 13 +++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index d96288e013..0ef604ebe5 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -2343,11 +2343,16 @@ public: atomic_ref& operator=(const atomic_ref&) = delete; - static constexpr bool is_always_lock_free = _Is_always_lock_free; - static constexpr bool _Is_potentially_lock_free = sizeof(_Ty) <= 2 * sizeof(void*) && (sizeof(_Ty) & (sizeof(_Ty) - 1)) == 0; + static constexpr bool is_always_lock_free = +#if _ATOMIC_HAS_DCAS + _Is_potentially_lock_free; +#else // ^^^ _ATOMIC_HAS_DCAS / !_ATOMIC_HAS_DCAS vvv + _Is_potentially_lock_free && sizeof(_Ty) < 2 * sizeof(void*); +#endif // ^^^ !_ATOMIC_HAS_DCAS ^^^ + static constexpr size_t required_alignment = _Is_potentially_lock_free ? sizeof(_Ty) : alignof(_Ty); _NODISCARD bool is_lock_free() const noexcept { @@ -2359,7 +2364,7 @@ public: } else { return __std_atomic_has_cmpxchg16b() != 0; } -#endif // _ATOMIC_HAS_DCAS +#endif // ^^^ !_ATOMIC_HAS_DCAS ^^^ } void store(const _Ty _Value) const noexcept { diff --git a/tests/std/tests/P0019R8_atomic_ref/test.cpp b/tests/std/tests/P0019R8_atomic_ref/test.cpp index c5ecce949e..c80aa9ce34 100644 --- a/tests/std/tests/P0019R8_atomic_ref/test.cpp +++ b/tests/std/tests/P0019R8_atomic_ref/test.cpp @@ -373,6 +373,19 @@ void test_incomplete_associated_class_all() { // COMPILE-ONLY } #endif // ^^^ no workaround ^^^ +void test_gh4472() { + struct two_pointers_t { + void* left; + void* right; + } two_pointers; + + static_assert(std::atomic_ref::required_alignment == sizeof(two_pointers_t)); + static_assert(std::atomic_ref::is_always_lock_free == _STD_ATOMIC_ALWAYS_USE_CMPXCHG16B); + + // we expect tests to run on DCAS machine. Win8+ require that + assert(std::atomic_ref(two_pointers).is_lock_free()); +} + int main() { test_ops(); test_ops(); From 33d16d46a660f059963f69e35af0a0ca62de901f Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 14 Mar 2024 12:15:05 +0200 Subject: [PATCH 02/13] coverage for x64 DCAS mode need actual DCAS on target machine --- tests/std/tests/P0019R8_atomic_ref/env.lst | 3 +++ tests/std/tests/usual_20_matrix.lst | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/std/tests/P0019R8_atomic_ref/env.lst b/tests/std/tests/P0019R8_atomic_ref/env.lst index 351a8293d9..8c9c79c363 100644 --- a/tests/std/tests/P0019R8_atomic_ref/env.lst +++ b/tests/std/tests/P0019R8_atomic_ref/env.lst @@ -2,3 +2,6 @@ # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception RUNALL_INCLUDE ..\usual_20_matrix.lst +RUNALL_CROSSLIST +PM_CL="" +PM_CL="/D_STD_ATOMIC_ALWAYS_USE_CMPXCHG16B=1" diff --git a/tests/std/tests/usual_20_matrix.lst b/tests/std/tests/usual_20_matrix.lst index 8964140309..fa7879e9f5 100644 --- a/tests/std/tests/usual_20_matrix.lst +++ b/tests/std/tests/usual_20_matrix.lst @@ -35,6 +35,6 @@ PM_CL="/clr /MD /std:c++20" PM_CL="/clr /MDd /std:c++20" PM_CL="/BE /c /EHsc /MD /std:c++20 /permissive-" PM_CL="/BE /c /EHsc /MTd /std:c++latest /permissive-" -PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /std:c++20 /permissive- /MD --start-no-unused-arguments" -PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /std:c++latest /permissive- /MTd /fp:strict --start-no-unused-arguments" -PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /std:c++latest /permissive- /MT /fp:strict -fsanitize=undefined -fno-sanitize-recover=undefined --start-no-unused-arguments" +PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /std:c++20 /permissive- /MD --start-no-unused-arguments -mcx16" +PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /std:c++latest /permissive- /MTd /fp:strict --start-no-unused-arguments -mcx16" +PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /std:c++latest /permissive- /MT /fp:strict -fsanitize=undefined -fno-sanitize-recover=undefined --start-no-unused-arguments -mcx16" From 249733cc0d935cb7e24d309df5bf4ff497074062 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 14 Mar 2024 12:24:51 +0200 Subject: [PATCH 03/13] x64 only --- tests/std/tests/P0019R8_atomic_ref/test.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/std/tests/P0019R8_atomic_ref/test.cpp b/tests/std/tests/P0019R8_atomic_ref/test.cpp index c80aa9ce34..51d84bf558 100644 --- a/tests/std/tests/P0019R8_atomic_ref/test.cpp +++ b/tests/std/tests/P0019R8_atomic_ref/test.cpp @@ -380,7 +380,12 @@ void test_gh4472() { } two_pointers; static_assert(std::atomic_ref::required_alignment == sizeof(two_pointers_t)); + +#ifdef _WIN64 static_assert(std::atomic_ref::is_always_lock_free == _STD_ATOMIC_ALWAYS_USE_CMPXCHG16B); +#else + static_assert(std::atomic_ref::is_always_lock_free); +#endif // we expect tests to run on DCAS machine. Win8+ require that assert(std::atomic_ref(two_pointers).is_lock_free()); From d240d707a4cec00380f0f2d2320d442885b455bd Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 14 Mar 2024 12:48:43 +0200 Subject: [PATCH 04/13] feature --- tests/utils/stl/test/features.py | 1 + tests/utils/stl/test/tests.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/tests/utils/stl/test/features.py b/tests/utils/stl/test/features.py index 4d286378f3..193831ecd4 100644 --- a/tests/utils/stl/test/features.py +++ b/tests/utils/stl/test/features.py @@ -58,6 +58,7 @@ def getDefaultFeatures(config, litConfig): DEFAULT_FEATURES.append(Feature(name='edg')) DEFAULT_FEATURES.append(Feature(name='arch_avx2')) DEFAULT_FEATURES.append(Feature(name='x64')) + DEFAULT_FEATURES.append(Feature(name='cx16')) elif litConfig.target_arch.casefold() == 'arm'.casefold(): DEFAULT_FEATURES.append(Feature(name='arch_vfpv4')) diff --git a/tests/utils/stl/test/tests.py b/tests/utils/stl/test/tests.py index 5ce0ae8345..43631b4dea 100644 --- a/tests/utils/stl/test/tests.py +++ b/tests/utils/stl/test/tests.py @@ -314,6 +314,8 @@ def _parseFlags(self, litConfig): foundCRT = True elif flag[1:] == 'analyze:plugin': afterAnalyzePlugin = True + elif flag[1:] == 'D_STD_ATOMIC_ALWAYS_USE_CMPXCHG16B=1': + self.requires.append('cx16') if not foundStd: self._addCustomFeature('c++14') From 59d6bea7dd73ec1624c467ca152e0b78b6c163a3 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 14 Mar 2024 13:32:08 +0200 Subject: [PATCH 05/13] but what if we actually run the test --- tests/std/tests/P0019R8_atomic_ref/test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/std/tests/P0019R8_atomic_ref/test.cpp b/tests/std/tests/P0019R8_atomic_ref/test.cpp index 51d84bf558..fdba356264 100644 --- a/tests/std/tests/P0019R8_atomic_ref/test.cpp +++ b/tests/std/tests/P0019R8_atomic_ref/test.cpp @@ -373,7 +373,7 @@ void test_incomplete_associated_class_all() { // COMPILE-ONLY } #endif // ^^^ no workaround ^^^ -void test_gh4472() { +void test_gh_4472() { struct two_pointers_t { void* left; void* right; @@ -443,4 +443,5 @@ int main() { test_ptr_ops(); test_gh_1497(); + test_gh_4472(); } From f17494fb4c7aaf2158a0bac98e4c4559ad34e969 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 14 Mar 2024 13:55:38 +0200 Subject: [PATCH 06/13] alignment required --- tests/std/tests/P0019R8_atomic_ref/test.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/std/tests/P0019R8_atomic_ref/test.cpp b/tests/std/tests/P0019R8_atomic_ref/test.cpp index fdba356264..56ef39d22b 100644 --- a/tests/std/tests/P0019R8_atomic_ref/test.cpp +++ b/tests/std/tests/P0019R8_atomic_ref/test.cpp @@ -377,7 +377,9 @@ void test_gh_4472() { struct two_pointers_t { void* left; void* right; - } two_pointers; + }; + + alignas(std::atomic_ref::required_alignment) two_pointers_t two_pointers; static_assert(std::atomic_ref::required_alignment == sizeof(two_pointers_t)); From 85e6c3f4f22df0a34fb000284c55a9354a1165e2 Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 14 Mar 2024 17:40:46 +0200 Subject: [PATCH 07/13] confusing #endifs overdrive by --- stl/inc/atomic | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 0ef604ebe5..475fd7350c 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -1656,7 +1656,7 @@ _INLINE_VAR constexpr bool _Is_always_lock_free = _TypeSize <= 2 * sizeof(void*) #else // ^^^ _ATOMIC_HAS_DCAS / !_ATOMIC_HAS_DCAS vvv template _INLINE_VAR constexpr bool _Is_always_lock_free = _TypeSize <= sizeof(void*); -#endif // _ATOMIC_HAS_DCAS +#endif // ^^^ !_ATOMIC_HAS_DCAS ^^^ #endif // ^^^ break ABI ^^^ template > @@ -2179,7 +2179,7 @@ public: return sizeof(_Ty) <= 2 * sizeof(void*); #else // ^^^ _ATOMIC_HAS_DCAS / !_ATOMIC_HAS_DCAS vvv return sizeof(_Ty) <= sizeof(void*) || (sizeof(_Ty) <= 2 * sizeof(void*) && __std_atomic_has_cmpxchg16b()); -#endif // _ATOMIC_HAS_DCAS +#endif // ^^^ !_ATOMIC_HAS_DCAS ^^^ } #endif // ^^^ break ABI ^^^ From 1a8a602c0feff8f40cd06cd10c7af0c5fe5fff3f Mon Sep 17 00:00:00 2001 From: Alex Guteniev Date: Thu, 14 Mar 2024 22:09:27 +0200 Subject: [PATCH 08/13] more logical comparison --- stl/inc/atomic | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index 475fd7350c..18d1b3de57 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -2350,7 +2350,7 @@ public: #if _ATOMIC_HAS_DCAS _Is_potentially_lock_free; #else // ^^^ _ATOMIC_HAS_DCAS / !_ATOMIC_HAS_DCAS vvv - _Is_potentially_lock_free && sizeof(_Ty) < 2 * sizeof(void*); + _Is_potentially_lock_free && sizeof(_Ty) <= sizeof(void*); #endif // ^^^ !_ATOMIC_HAS_DCAS ^^^ static constexpr size_t required_alignment = _Is_potentially_lock_free ? sizeof(_Ty) : alignof(_Ty); From 9eb937424ea84bcd14983bd20d3b2d0e91bf2a85 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Mar 2024 17:16:59 -0700 Subject: [PATCH 09/13] Add comment with bug title. --- tests/std/tests/P0019R8_atomic_ref/test.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/std/tests/P0019R8_atomic_ref/test.cpp b/tests/std/tests/P0019R8_atomic_ref/test.cpp index 56ef39d22b..58659ee833 100644 --- a/tests/std/tests/P0019R8_atomic_ref/test.cpp +++ b/tests/std/tests/P0019R8_atomic_ref/test.cpp @@ -373,6 +373,8 @@ void test_incomplete_associated_class_all() { // COMPILE-ONLY } #endif // ^^^ no workaround ^^^ +// GH-4472 ": With _STD_ATOMIC_ALWAYS_USE_CMPXCHG16B defined to 1, +// atomic_ref<16 bytes> does not report is_lock_free and is_always_lock_free correctly" void test_gh_4472() { struct two_pointers_t { void* left; From 28ad98d335ad716e6f1b43ed0160727022c17048 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Mar 2024 17:17:25 -0700 Subject: [PATCH 10/13] Improve comment grammar. --- tests/std/tests/P0019R8_atomic_ref/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/P0019R8_atomic_ref/test.cpp b/tests/std/tests/P0019R8_atomic_ref/test.cpp index 58659ee833..92be58ae3d 100644 --- a/tests/std/tests/P0019R8_atomic_ref/test.cpp +++ b/tests/std/tests/P0019R8_atomic_ref/test.cpp @@ -391,7 +391,7 @@ void test_gh_4472() { static_assert(std::atomic_ref::is_always_lock_free); #endif - // we expect tests to run on DCAS machine. Win8+ require that + // We expect tests to run on machines that support DCAS, which is required by Win8+. assert(std::atomic_ref(two_pointers).is_lock_free()); } From db6f0e8078c05fda1653bef4cdc5939aac70d976 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Mar 2024 17:17:53 -0700 Subject: [PATCH 11/13] Extract variable before assertion. --- tests/std/tests/P0019R8_atomic_ref/test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/std/tests/P0019R8_atomic_ref/test.cpp b/tests/std/tests/P0019R8_atomic_ref/test.cpp index 92be58ae3d..6b6eb77425 100644 --- a/tests/std/tests/P0019R8_atomic_ref/test.cpp +++ b/tests/std/tests/P0019R8_atomic_ref/test.cpp @@ -392,7 +392,8 @@ void test_gh_4472() { #endif // We expect tests to run on machines that support DCAS, which is required by Win8+. - assert(std::atomic_ref(two_pointers).is_lock_free()); + std::atomic_ref ar{two_pointers}; + assert(ar.is_lock_free()); } int main() { From 9fe9052665188fdd4767398f0e8f4dbf19acca0c Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Mar 2024 17:13:51 -0700 Subject: [PATCH 12/13] Add star tags to respect ASAN. --- tests/std/tests/P0019R8_atomic_ref/env.lst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/P0019R8_atomic_ref/env.lst b/tests/std/tests/P0019R8_atomic_ref/env.lst index 8c9c79c363..2d542dd7c7 100644 --- a/tests/std/tests/P0019R8_atomic_ref/env.lst +++ b/tests/std/tests/P0019R8_atomic_ref/env.lst @@ -3,5 +3,5 @@ RUNALL_INCLUDE ..\usual_20_matrix.lst RUNALL_CROSSLIST -PM_CL="" -PM_CL="/D_STD_ATOMIC_ALWAYS_USE_CMPXCHG16B=1" +* PM_CL="" +* PM_CL="/D_STD_ATOMIC_ALWAYS_USE_CMPXCHG16B=1" From fbbc618ff50b215b155be1878328b5f170b48c33 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Mar 2024 17:36:21 -0700 Subject: [PATCH 13/13] Skip the optional mode for Clang and non-x64 in a way that's friendly to the internal test harness. --- tests/std/tests/P0019R8_atomic_ref/env.lst | 2 +- tests/std/tests/P0019R8_atomic_ref/test.cpp | 8 ++++++++ tests/std/tests/usual_20_matrix.lst | 6 +++--- tests/utils/stl/test/features.py | 1 - tests/utils/stl/test/tests.py | 2 -- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/std/tests/P0019R8_atomic_ref/env.lst b/tests/std/tests/P0019R8_atomic_ref/env.lst index 2d542dd7c7..b3f3ea4864 100644 --- a/tests/std/tests/P0019R8_atomic_ref/env.lst +++ b/tests/std/tests/P0019R8_atomic_ref/env.lst @@ -4,4 +4,4 @@ RUNALL_INCLUDE ..\usual_20_matrix.lst RUNALL_CROSSLIST * PM_CL="" -* PM_CL="/D_STD_ATOMIC_ALWAYS_USE_CMPXCHG16B=1" +* PM_CL="/D_STD_ATOMIC_ALWAYS_USE_CMPXCHG16B=1 /DTEST_CMPXCHG16B" diff --git a/tests/std/tests/P0019R8_atomic_ref/test.cpp b/tests/std/tests/P0019R8_atomic_ref/test.cpp index 6b6eb77425..a687cc97da 100644 --- a/tests/std/tests/P0019R8_atomic_ref/test.cpp +++ b/tests/std/tests/P0019R8_atomic_ref/test.cpp @@ -1,6 +1,12 @@ // Copyright (c) Microsoft Corporation. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +#if defined(TEST_CMPXCHG16B) && (defined(__clang__) || !defined(_M_X64)) +// Skip Clang because it would require the -mcx16 compiler option. +// Skip non-x64 because _STD_ATOMIC_ALWAYS_USE_CMPXCHG16B is always 1 for ARM64, and is forbidden to be 1 for 32-bit. +int main() {} +#else // ^^^ skip test / run test vvv + #include #include #include @@ -450,3 +456,5 @@ int main() { test_gh_1497(); test_gh_4472(); } + +#endif // ^^^ run test ^^^ diff --git a/tests/std/tests/usual_20_matrix.lst b/tests/std/tests/usual_20_matrix.lst index fa7879e9f5..8964140309 100644 --- a/tests/std/tests/usual_20_matrix.lst +++ b/tests/std/tests/usual_20_matrix.lst @@ -35,6 +35,6 @@ PM_CL="/clr /MD /std:c++20" PM_CL="/clr /MDd /std:c++20" PM_CL="/BE /c /EHsc /MD /std:c++20 /permissive-" PM_CL="/BE /c /EHsc /MTd /std:c++latest /permissive-" -PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /std:c++20 /permissive- /MD --start-no-unused-arguments -mcx16" -PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /std:c++latest /permissive- /MTd /fp:strict --start-no-unused-arguments -mcx16" -PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /std:c++latest /permissive- /MT /fp:strict -fsanitize=undefined -fno-sanitize-recover=undefined --start-no-unused-arguments -mcx16" +PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /std:c++20 /permissive- /MD --start-no-unused-arguments" +PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /std:c++latest /permissive- /MTd /fp:strict --start-no-unused-arguments" +PM_COMPILER="clang-cl" PM_CL="-fno-ms-compatibility -fno-delayed-template-parsing -Wno-unqualified-std-cast-call /EHsc /std:c++latest /permissive- /MT /fp:strict -fsanitize=undefined -fno-sanitize-recover=undefined --start-no-unused-arguments" diff --git a/tests/utils/stl/test/features.py b/tests/utils/stl/test/features.py index 193831ecd4..4d286378f3 100644 --- a/tests/utils/stl/test/features.py +++ b/tests/utils/stl/test/features.py @@ -58,7 +58,6 @@ def getDefaultFeatures(config, litConfig): DEFAULT_FEATURES.append(Feature(name='edg')) DEFAULT_FEATURES.append(Feature(name='arch_avx2')) DEFAULT_FEATURES.append(Feature(name='x64')) - DEFAULT_FEATURES.append(Feature(name='cx16')) elif litConfig.target_arch.casefold() == 'arm'.casefold(): DEFAULT_FEATURES.append(Feature(name='arch_vfpv4')) diff --git a/tests/utils/stl/test/tests.py b/tests/utils/stl/test/tests.py index 43631b4dea..5ce0ae8345 100644 --- a/tests/utils/stl/test/tests.py +++ b/tests/utils/stl/test/tests.py @@ -314,8 +314,6 @@ def _parseFlags(self, litConfig): foundCRT = True elif flag[1:] == 'analyze:plugin': afterAnalyzePlugin = True - elif flag[1:] == 'D_STD_ATOMIC_ALWAYS_USE_CMPXCHG16B=1': - self.requires.append('cx16') if not foundStd: self._addCustomFeature('c++14')