From d8d9b5a53e438a27a8d3aefb681dd96e2d0b8232 Mon Sep 17 00:00:00 2001 From: Damien L-G Date: Mon, 14 Apr 2025 17:21:34 -0400 Subject: [PATCH 1/5] [libc++][atomic_ref] Use __atomic_fetch_{add,sub} builtins on floating-points whenever possible Fix #135109 Clang is able to emit an `atomicrmw` instruction from the `__atomic_fetch_add` and `__atomic_fetch_sub` builtins on floating-point types. --- libcxx/include/__atomic/atomic_ref.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libcxx/include/__atomic/atomic_ref.h b/libcxx/include/__atomic/atomic_ref.h index b5493662c518e..baffd1ffb6094 100644 --- a/libcxx/include/__atomic/atomic_ref.h +++ b/libcxx/include/__atomic/atomic_ref.h @@ -322,20 +322,28 @@ struct atomic_ref<_Tp> : public __atomic_ref_base<_Tp> { atomic_ref& operator=(const atomic_ref&) = delete; _LIBCPP_HIDE_FROM_ABI _Tp fetch_add(_Tp __arg, memory_order __order = memory_order_seq_cst) const noexcept { +# ifdef _LIBCPP_COMPILER_CLANG_BASED + return __atomic_fetch_add(this->__ptr_, __arg, std::__to_gcc_order(__order)); +# else _Tp __old = this->load(memory_order_relaxed); _Tp __new = __old + __arg; while (!this->compare_exchange_weak(__old, __new, __order, memory_order_relaxed)) { __new = __old + __arg; } return __old; +# endif } _LIBCPP_HIDE_FROM_ABI _Tp fetch_sub(_Tp __arg, memory_order __order = memory_order_seq_cst) const noexcept { +# ifdef _LIBCPP_COMPILER_CLANG_BASED + return __atomic_fetch_sub(this->__ptr_, __arg, std::__to_gcc_order(__order)); +# else _Tp __old = this->load(memory_order_relaxed); _Tp __new = __old - __arg; while (!this->compare_exchange_weak(__old, __new, __order, memory_order_relaxed)) { __new = __old - __arg; } return __old; +# endif } _LIBCPP_HIDE_FROM_ABI _Tp operator+=(_Tp __arg) const noexcept { return fetch_add(__arg) + __arg; } From 246d885f0dcbc65d5a51bb7ba1488194c77dc1b6 Mon Sep 17 00:00:00 2001 From: Damien L-G Date: Sat, 3 May 2025 20:39:25 -0400 Subject: [PATCH 2/5] Move x87-fp80 helper into new header and reuse --- libcxx/include/CMakeLists.txt | 1 + libcxx/include/__atomic/atomic.h | 29 ++--------- libcxx/include/__atomic/atomic_ref.h | 37 +++++++------- .../include/__atomic/floating_point_helper.h | 51 +++++++++++++++++++ libcxx/include/module.modulemap | 1 + .../gn/secondary/libcxx/include/BUILD.gn | 1 + 6 files changed, 76 insertions(+), 44 deletions(-) create mode 100644 libcxx/include/__atomic/floating_point_helper.h diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt index c225131101ce2..3d8e447c6b428 100644 --- a/libcxx/include/CMakeLists.txt +++ b/libcxx/include/CMakeLists.txt @@ -215,6 +215,7 @@ set(files __atomic/check_memory_order.h __atomic/contention_t.h __atomic/fence.h + __atomic/floating_point_helper.h __atomic/is_always_lock_free.h __atomic/kill_dependency.h __atomic/memory_order.h diff --git a/libcxx/include/__atomic/atomic.h b/libcxx/include/__atomic/atomic.h index eead49dde6192..05a3fa715c6dc 100644 --- a/libcxx/include/__atomic/atomic.h +++ b/libcxx/include/__atomic/atomic.h @@ -11,6 +11,7 @@ #include <__atomic/atomic_sync.h> #include <__atomic/check_memory_order.h> +#include <__atomic/floating_point_helper.h> #include <__atomic/is_always_lock_free.h> #include <__atomic/memory_order.h> #include <__atomic/support.h> @@ -332,41 +333,17 @@ template requires is_floating_point_v<_Tp> struct atomic<_Tp> : __atomic_base<_Tp> { private: - _LIBCPP_HIDE_FROM_ABI static constexpr bool __is_fp80_long_double() { - // Only x87-fp80 long double has 64-bit mantissa - return __LDBL_MANT_DIG__ == 64 && std::is_same_v<_Tp, long double>; - } - - _LIBCPP_HIDE_FROM_ABI static constexpr bool __has_rmw_builtin() { -# ifndef _LIBCPP_COMPILER_CLANG_BASED - return false; -# else - // The builtin __cxx_atomic_fetch_add errors during compilation for - // long double on platforms with fp80 format. - // For more details, see - // lib/Sema/SemaChecking.cpp function IsAllowedValueType - // LLVM Parser does not allow atomicrmw with x86_fp80 type. - // if (ValType->isSpecificBuiltinType(BuiltinType::LongDouble) && - // &Context.getTargetInfo().getLongDoubleFormat() == - // &llvm::APFloat::x87DoubleExtended()) - // For more info - // https://github.com/llvm/llvm-project/issues/68602 - // https://reviews.llvm.org/D53965 - return !__is_fp80_long_double(); -# endif - } - template _LIBCPP_HIDE_FROM_ABI static _Tp __rmw_op(_This&& __self, _Tp __operand, memory_order __m, _Operation __operation, _BuiltinOp __builtin_op) { - if constexpr (__has_rmw_builtin()) { + if constexpr (__has_rmw_builtin<_Tp>()) { return __builtin_op(std::addressof(std::forward<_This>(__self).__a_), __operand, __m); } else { _Tp __old = __self.load(memory_order_relaxed); _Tp __new = __operation(__old, __operand); while (!__self.compare_exchange_weak(__old, __new, __m, memory_order_relaxed)) { # ifdef _LIBCPP_COMPILER_CLANG_BASED - if constexpr (__is_fp80_long_double()) { + if constexpr (__is_fp80_long_double<_Tp>()) { // https://github.com/llvm/llvm-project/issues/47978 // clang bug: __old is not updated on failure for atomic::compare_exchange_weak // Note __old = __self.load(memory_order_relaxed) will not work diff --git a/libcxx/include/__atomic/atomic_ref.h b/libcxx/include/__atomic/atomic_ref.h index baffd1ffb6094..5ab31e357b5cc 100644 --- a/libcxx/include/__atomic/atomic_ref.h +++ b/libcxx/include/__atomic/atomic_ref.h @@ -20,6 +20,7 @@ #include <__assert> #include <__atomic/atomic_sync.h> #include <__atomic/check_memory_order.h> +#include <__atomic/floating_point_helper.h> #include <__atomic/memory_order.h> #include <__atomic/to_gcc_order.h> #include <__concepts/arithmetic.h> @@ -322,28 +323,28 @@ struct atomic_ref<_Tp> : public __atomic_ref_base<_Tp> { atomic_ref& operator=(const atomic_ref&) = delete; _LIBCPP_HIDE_FROM_ABI _Tp fetch_add(_Tp __arg, memory_order __order = memory_order_seq_cst) const noexcept { -# ifdef _LIBCPP_COMPILER_CLANG_BASED - return __atomic_fetch_add(this->__ptr_, __arg, std::__to_gcc_order(__order)); -# else - _Tp __old = this->load(memory_order_relaxed); - _Tp __new = __old + __arg; - while (!this->compare_exchange_weak(__old, __new, __order, memory_order_relaxed)) { - __new = __old + __arg; + if constexpr (__has_rmw_builtin<_Tp>()) { + return __atomic_fetch_add(this->__ptr_, __arg, std::__to_gcc_order(__order)); + } else { + _Tp __old = this->load(memory_order_relaxed); + _Tp __new = __old + __arg; + while (!this->compare_exchange_weak(__old, __new, __order, memory_order_relaxed)) { + __new = __old + __arg; + } + return __old; } - return __old; -# endif } _LIBCPP_HIDE_FROM_ABI _Tp fetch_sub(_Tp __arg, memory_order __order = memory_order_seq_cst) const noexcept { -# ifdef _LIBCPP_COMPILER_CLANG_BASED - return __atomic_fetch_sub(this->__ptr_, __arg, std::__to_gcc_order(__order)); -# else - _Tp __old = this->load(memory_order_relaxed); - _Tp __new = __old - __arg; - while (!this->compare_exchange_weak(__old, __new, __order, memory_order_relaxed)) { - __new = __old - __arg; + if constexpr (__has_rmw_builtin<_Tp>()) { + return __atomic_fetch_sub(this->__ptr_, __arg, std::__to_gcc_order(__order)); + } else { + _Tp __old = this->load(memory_order_relaxed); + _Tp __new = __old - __arg; + while (!this->compare_exchange_weak(__old, __new, __order, memory_order_relaxed)) { + __new = __old - __arg; + } + return __old; } - return __old; -# endif } _LIBCPP_HIDE_FROM_ABI _Tp operator+=(_Tp __arg) const noexcept { return fetch_add(__arg) + __arg; } diff --git a/libcxx/include/__atomic/floating_point_helper.h b/libcxx/include/__atomic/floating_point_helper.h new file mode 100644 index 0000000000000..68dfbda4c7a9b --- /dev/null +++ b/libcxx/include/__atomic/floating_point_helper.h @@ -0,0 +1,51 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef _LIBCPP___ATOMIC_FLOATING_POINT_HELPER_H +#define _LIBCPP___ATOMIC_FLOATING_POINT_HELPER_H + +#include <__config> +#include <__type_traits/is_floating_point.h> +#include <__type_traits/is_same.h> + +_LIBCPP_BEGIN_NAMESPACE_STD + +#if _LIBCPP_STD_VER >= 20 + +template +_LIBCPP_HIDE_FROM_ABI constexpr bool __is_fp80_long_double() { + // Only x87-fp80 long double has 64-bit mantissa + return __LDBL_MANT_DIG__ == 64 && std::is_same_v<_Tp, long double>; +} + +template +_LIBCPP_HIDE_FROM_ABI constexpr bool __has_rmw_builtin() { + static_assert(std::is_floating_point_v<_Tp>); +# ifndef _LIBCPP_COMPILER_CLANG_BASED + return false; +# else + // The builtin __cxx_atomic_fetch_add errors during compilation for + // long double on platforms with fp80 format. + // For more details, see + // lib/Sema/SemaChecking.cpp function IsAllowedValueType + // LLVM Parser does not allow atomicrmw with x86_fp80 type. + // if (ValType->isSpecificBuiltinType(BuiltinType::LongDouble) && + // &Context.getTargetInfo().getLongDoubleFormat() == + // &llvm::APFloat::x87DoubleExtended()) + // For more info + // https://github.com/llvm/llvm-project/issues/68602 + // https://reviews.llvm.org/D53965 + return !__is_fp80_long_double<_Tp>(); +# endif +} + +#endif + +_LIBCPP_END_NAMESPACE_STD + +#endif // _LIBCPP___ATOMIC_FLOATING_POINT_HELPER_H diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap index 65123eb268150..dc5a6db224f39 100644 --- a/libcxx/include/module.modulemap +++ b/libcxx/include/module.modulemap @@ -890,6 +890,7 @@ module std [system] { module check_memory_order { header "__atomic/check_memory_order.h" } module contention_t { header "__atomic/contention_t.h" } module fence { header "__atomic/fence.h" } + module floating_point_helper { header "__atomic/floating_point_helper.h" } module is_always_lock_free { header "__atomic/is_always_lock_free.h" } module kill_dependency { header "__atomic/kill_dependency.h" } module memory_order { header "__atomic/memory_order.h" } diff --git a/llvm/utils/gn/secondary/libcxx/include/BUILD.gn b/llvm/utils/gn/secondary/libcxx/include/BUILD.gn index 542fa9d94a39e..0c4e3319a8168 100644 --- a/llvm/utils/gn/secondary/libcxx/include/BUILD.gn +++ b/llvm/utils/gn/secondary/libcxx/include/BUILD.gn @@ -283,6 +283,7 @@ if (current_toolchain == default_toolchain) { "__atomic/check_memory_order.h", "__atomic/contention_t.h", "__atomic/fence.h", + "__atomic/floating_point_helper.h", "__atomic/is_always_lock_free.h", "__atomic/kill_dependency.h", "__atomic/memory_order.h", From c3453b64de52d79e0c3cd72b68a56e9aa8da062a Mon Sep 17 00:00:00 2001 From: Damien L-G Date: Sun, 4 May 2025 07:33:34 -0400 Subject: [PATCH 3/5] Fixup robust against ADL --- libcxx/include/__atomic/atomic.h | 4 ++-- libcxx/include/__atomic/atomic_ref.h | 4 ++-- libcxx/include/__atomic/floating_point_helper.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libcxx/include/__atomic/atomic.h b/libcxx/include/__atomic/atomic.h index 05a3fa715c6dc..31e21514f25de 100644 --- a/libcxx/include/__atomic/atomic.h +++ b/libcxx/include/__atomic/atomic.h @@ -336,14 +336,14 @@ struct atomic<_Tp> : __atomic_base<_Tp> { template _LIBCPP_HIDE_FROM_ABI static _Tp __rmw_op(_This&& __self, _Tp __operand, memory_order __m, _Operation __operation, _BuiltinOp __builtin_op) { - if constexpr (__has_rmw_builtin<_Tp>()) { + if constexpr (std::__has_rmw_builtin<_Tp>()) { return __builtin_op(std::addressof(std::forward<_This>(__self).__a_), __operand, __m); } else { _Tp __old = __self.load(memory_order_relaxed); _Tp __new = __operation(__old, __operand); while (!__self.compare_exchange_weak(__old, __new, __m, memory_order_relaxed)) { # ifdef _LIBCPP_COMPILER_CLANG_BASED - if constexpr (__is_fp80_long_double<_Tp>()) { + if constexpr (std::__is_fp80_long_double<_Tp>()) { // https://github.com/llvm/llvm-project/issues/47978 // clang bug: __old is not updated on failure for atomic::compare_exchange_weak // Note __old = __self.load(memory_order_relaxed) will not work diff --git a/libcxx/include/__atomic/atomic_ref.h b/libcxx/include/__atomic/atomic_ref.h index 5ab31e357b5cc..9bdc6b1160d2c 100644 --- a/libcxx/include/__atomic/atomic_ref.h +++ b/libcxx/include/__atomic/atomic_ref.h @@ -323,7 +323,7 @@ struct atomic_ref<_Tp> : public __atomic_ref_base<_Tp> { atomic_ref& operator=(const atomic_ref&) = delete; _LIBCPP_HIDE_FROM_ABI _Tp fetch_add(_Tp __arg, memory_order __order = memory_order_seq_cst) const noexcept { - if constexpr (__has_rmw_builtin<_Tp>()) { + if constexpr (std::__has_rmw_builtin<_Tp>()) { return __atomic_fetch_add(this->__ptr_, __arg, std::__to_gcc_order(__order)); } else { _Tp __old = this->load(memory_order_relaxed); @@ -335,7 +335,7 @@ struct atomic_ref<_Tp> : public __atomic_ref_base<_Tp> { } } _LIBCPP_HIDE_FROM_ABI _Tp fetch_sub(_Tp __arg, memory_order __order = memory_order_seq_cst) const noexcept { - if constexpr (__has_rmw_builtin<_Tp>()) { + if constexpr (std::__has_rmw_builtin<_Tp>()) { return __atomic_fetch_sub(this->__ptr_, __arg, std::__to_gcc_order(__order)); } else { _Tp __old = this->load(memory_order_relaxed); diff --git a/libcxx/include/__atomic/floating_point_helper.h b/libcxx/include/__atomic/floating_point_helper.h index 68dfbda4c7a9b..5ef802a084465 100644 --- a/libcxx/include/__atomic/floating_point_helper.h +++ b/libcxx/include/__atomic/floating_point_helper.h @@ -40,7 +40,7 @@ _LIBCPP_HIDE_FROM_ABI constexpr bool __has_rmw_builtin() { // For more info // https://github.com/llvm/llvm-project/issues/68602 // https://reviews.llvm.org/D53965 - return !__is_fp80_long_double<_Tp>(); + return !std::__is_fp80_long_double<_Tp>(); # endif } From e3879b3e3264ed591b4d33529b571318e4b7e51f Mon Sep 17 00:00:00 2001 From: Damien L-G Date: Sun, 4 May 2025 07:35:44 -0400 Subject: [PATCH 4/5] Add missing pragma GCC system_header --- libcxx/include/__atomic/floating_point_helper.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libcxx/include/__atomic/floating_point_helper.h b/libcxx/include/__atomic/floating_point_helper.h index 5ef802a084465..c4347a10892c8 100644 --- a/libcxx/include/__atomic/floating_point_helper.h +++ b/libcxx/include/__atomic/floating_point_helper.h @@ -13,6 +13,10 @@ #include <__type_traits/is_floating_point.h> #include <__type_traits/is_same.h> +#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) +# pragma GCC system_header +#endif + _LIBCPP_BEGIN_NAMESPACE_STD #if _LIBCPP_STD_VER >= 20 From ed5f402d841995e79fa68f2ab4be21d57fcacc49 Mon Sep 17 00:00:00 2001 From: Damien L-G Date: Fri, 27 Jun 2025 10:24:18 -0400 Subject: [PATCH 5/5] Annotate fetch_{add,sub} with XFAIL when using ThreadSanitizer --- libcxx/test/std/atomics/atomics.ref/fetch_add.pass.cpp | 1 + libcxx/test/std/atomics/atomics.ref/fetch_sub.pass.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/libcxx/test/std/atomics/atomics.ref/fetch_add.pass.cpp b/libcxx/test/std/atomics/atomics.ref/fetch_add.pass.cpp index 8e2605fef9d31..65a457a6129d5 100644 --- a/libcxx/test/std/atomics/atomics.ref/fetch_add.pass.cpp +++ b/libcxx/test/std/atomics/atomics.ref/fetch_add.pass.cpp @@ -7,6 +7,7 @@ // UNSUPPORTED: c++03, c++11, c++14, c++17 // XFAIL: !has-64-bit-atomics +// XFAIL: target={{x86_64-.*}} && tsan // integral-type fetch_add(integral-type, memory_order = memory_order::seq_cst) const noexcept; // floating-point-type fetch_add(floating-point-type, memory_order = memory_order::seq_cst) const noexcept; diff --git a/libcxx/test/std/atomics/atomics.ref/fetch_sub.pass.cpp b/libcxx/test/std/atomics/atomics.ref/fetch_sub.pass.cpp index 04def076a301f..ab89ebdbde261 100644 --- a/libcxx/test/std/atomics/atomics.ref/fetch_sub.pass.cpp +++ b/libcxx/test/std/atomics/atomics.ref/fetch_sub.pass.cpp @@ -7,6 +7,7 @@ // UNSUPPORTED: c++03, c++11, c++14, c++17 // XFAIL: !has-64-bit-atomics +// XFAIL: target={{x86_64-.*}} && tsan // integral-type fetch_sub(integral-type, memory_order = memory_order::seq_cst) const noexcept; // floating-point-type fetch_sub(floating-point-type, memory_order = memory_order::seq_cst) const noexcept;