Skip to content

[libc++][atomic_ref] Use __atomic_fetch_{add,sub} builtins on floating-points whenever possible #135685

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Sep 23, 2025
Merged

[libc++][atomic_ref] Use __atomic_fetch_{add,sub} builtins on floating-points whenever possible #135685

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d8d9b5a
[libc++][atomic_ref] Use __atomic_fetch_{add,sub} builtins on floatin…
dalg24 Apr 14, 2025
246d885
Move x87-fp80 helper into new header and reuse
dalg24 May 4, 2025
f8de9b0
Merge branch 'main' of https://github.com/llvm/llvm-project into atom…
dalg24 May 4, 2025
c3453b6
Fixup robust against ADL
dalg24 May 4, 2025
e3879b3
Add missing pragma GCC system_header
dalg24 May 4, 2025
b0d63bd
Merge branch 'main' of https://github.com/llvm/llvm-project into atom…
dalg24 Jun 27, 2025
ed5f402
Annotate fetch_{add,sub} with XFAIL when using ThreadSanitizer
dalg24 Jun 27, 2025
1fbf2ab
Revert change to generated file per review
dalg24 Sep 22, 2025
8cca6e5
Merge branch 'main' of https://github.com/llvm/llvm-project into atom…
dalg24 Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions libcxx/include/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,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
Expand Down
29 changes: 3 additions & 26 deletions libcxx/include/__atomic/atomic.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -332,41 +333,17 @@ template <class _Tp>
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 <class _This, class _Operation, class _BuiltinOp>
_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 (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()) {
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<long double>::compare_exchange_weak
// Note __old = __self.load(memory_order_relaxed) will not work
Expand Down
29 changes: 19 additions & 10 deletions libcxx/include/__atomic/atomic_ref.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -322,20 +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 {
_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 (std::__has_rmw_builtin<_Tp>()) {
return __atomic_fetch_add(this->__ptr_, __arg, std::__to_gcc_order(__order));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not an issue but just a note to myself and others who might be confused in the future.

In std::atomic, we have been using the "generic" API that abstracts the platform dependent code is __cxx_atomic_fetch_add, where on clang it calls __c11_atomic_fetch_add, and __atomic_fetch_add on gcc.

However, the abstraction layer takes a __cxx_atomic_base_impl, which does not exist in atomic_ref. So, instead of introducing a new abstraction layer that works for atomic_ref, we decided early on that we only use the gcc builtin __atomic_fetch_add.
clang has the gcc builtin as well

llvm-project/clang/include/clang/Basic/Builtins.td

Line 1999 in b3a1c77

let Spellings = ["__atomic_fetch_add"];

and its implementation is the same as the __c11_atomic_fetch_add

llvm-project/clang/lib/CodeGen/CGAtomic.cpp

Line 634 in b3a1c77

case AtomicExpr::AO__atomic_fetch_add:

So it is fine

} 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;
}
_LIBCPP_HIDE_FROM_ABI _Tp fetch_sub(_Tp __arg, memory_order __order = memory_order_seq_cst) const noexcept {
_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 (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);
_Tp __new = __old - __arg;
while (!this->compare_exchange_weak(__old, __new, __order, memory_order_relaxed)) {
__new = __old - __arg;
}
return __old;
}
return __old;
}

_LIBCPP_HIDE_FROM_ABI _Tp operator+=(_Tp __arg) const noexcept { return fetch_add(__arg) + __arg; }
Expand Down
55 changes: 55 additions & 0 deletions libcxx/include/__atomic/floating_point_helper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
//===----------------------------------------------------------------------===//
//
// 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>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
#endif

_LIBCPP_BEGIN_NAMESPACE_STD

#if _LIBCPP_STD_VER >= 20

template <class _Tp>
_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 <class _Tp>
_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 !std::__is_fp80_long_double<_Tp>();
# endif
}

#endif

_LIBCPP_END_NAMESPACE_STD

#endif // _LIBCPP___ATOMIC_FLOATING_POINT_HELPER_H
1 change: 1 addition & 0 deletions libcxx/include/module.modulemap.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,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" }
Expand Down
1 change: 1 addition & 0 deletions libcxx/test/std/atomics/atomics.ref/fetch_add.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

// UNSUPPORTED: c++03, c++11, c++14, c++17
// XFAIL: !has-64-bit-atomics
// XFAIL: target={{x86_64-.*}} && tsan
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copied from

llvm-project/libcxx/test/std/atomics/atomics.types.generic/atomics.types.float/fetch_add.pass.cpp

Lines 15 to 16 in 08d747c

// https://github.com/llvm/llvm-project/issues/72893
// XFAIL: target={{x86_64-.*}} && tsan

Note that the #72893 issue referenced in the comment seems to indicate that only long double was problematic with ThreadSanitizer but it is not accurate because the instantiation of the tests are commented out for long double

llvm-project/libcxx/test/std/atomics/atomics.types.generic/atomics.types.float/fetch_add.pass.cpp

Lines 119 to 120 in 08d747c

// TODO https://github.com/llvm/llvm-project/issues/47978
// test<long double>();

Per a conversation on Discord with @huixie90, I propose to not hold up this PR and just use the same XFAIL annotation that we have in the std::atomic<Floating>::fetch_{add,sub} tests.


// 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;
Expand Down
1 change: 1 addition & 0 deletions libcxx/test/std/atomics/atomics.ref/fetch_sub.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions llvm/utils/gn/secondary/libcxx/include/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,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",
Expand Down
Loading