Skip to content

Commit c172082

Browse files
authored
[libc++][atomic_ref] Use __atomic_fetch_{add,sub} builtins on floating-points whenever possible (#135685)
Fix #135109 Clang is able to emit an `atomicrmw` instruction from the `__atomic_fetch_add` and `__atomic_fetch_sub` builtins on floating-point types.
1 parent 3a1111d commit c172082

File tree

7 files changed

+81
-36
lines changed

7 files changed

+81
-36
lines changed

libcxx/include/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ set(files
216216
__atomic/check_memory_order.h
217217
__atomic/contention_t.h
218218
__atomic/fence.h
219+
__atomic/floating_point_helper.h
219220
__atomic/is_always_lock_free.h
220221
__atomic/kill_dependency.h
221222
__atomic/memory_order.h

libcxx/include/__atomic/atomic.h

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include <__atomic/atomic_sync.h>
1313
#include <__atomic/check_memory_order.h>
14+
#include <__atomic/floating_point_helper.h>
1415
#include <__atomic/is_always_lock_free.h>
1516
#include <__atomic/memory_order.h>
1617
#include <__atomic/support.h>
@@ -332,41 +333,17 @@ template <class _Tp>
332333
requires is_floating_point_v<_Tp>
333334
struct atomic<_Tp> : __atomic_base<_Tp> {
334335
private:
335-
_LIBCPP_HIDE_FROM_ABI static constexpr bool __is_fp80_long_double() {
336-
// Only x87-fp80 long double has 64-bit mantissa
337-
return __LDBL_MANT_DIG__ == 64 && std::is_same_v<_Tp, long double>;
338-
}
339-
340-
_LIBCPP_HIDE_FROM_ABI static constexpr bool __has_rmw_builtin() {
341-
# ifndef _LIBCPP_COMPILER_CLANG_BASED
342-
return false;
343-
# else
344-
// The builtin __cxx_atomic_fetch_add errors during compilation for
345-
// long double on platforms with fp80 format.
346-
// For more details, see
347-
// lib/Sema/SemaChecking.cpp function IsAllowedValueType
348-
// LLVM Parser does not allow atomicrmw with x86_fp80 type.
349-
// if (ValType->isSpecificBuiltinType(BuiltinType::LongDouble) &&
350-
// &Context.getTargetInfo().getLongDoubleFormat() ==
351-
// &llvm::APFloat::x87DoubleExtended())
352-
// For more info
353-
// https://llvm.org/PR68602
354-
// https://reviews.llvm.org/D53965
355-
return !__is_fp80_long_double();
356-
# endif
357-
}
358-
359336
template <class _This, class _Operation, class _BuiltinOp>
360337
_LIBCPP_HIDE_FROM_ABI static _Tp
361338
__rmw_op(_This&& __self, _Tp __operand, memory_order __m, _Operation __operation, _BuiltinOp __builtin_op) {
362-
if constexpr (__has_rmw_builtin()) {
339+
if constexpr (std::__has_rmw_builtin<_Tp>()) {
363340
return __builtin_op(std::addressof(std::forward<_This>(__self).__a_), __operand, __m);
364341
} else {
365342
_Tp __old = __self.load(memory_order_relaxed);
366343
_Tp __new = __operation(__old, __operand);
367344
while (!__self.compare_exchange_weak(__old, __new, __m, memory_order_relaxed)) {
368345
# ifdef _LIBCPP_COMPILER_CLANG_BASED
369-
if constexpr (__is_fp80_long_double()) {
346+
if constexpr (std::__is_fp80_long_double<_Tp>()) {
370347
// https://llvm.org/PR47978
371348
// clang bug: __old is not updated on failure for atomic<long double>::compare_exchange_weak
372349
// Note __old = __self.load(memory_order_relaxed) will not work

libcxx/include/__atomic/atomic_ref.h

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <__assert>
2121
#include <__atomic/atomic_sync.h>
2222
#include <__atomic/check_memory_order.h>
23+
#include <__atomic/floating_point_helper.h>
2324
#include <__atomic/memory_order.h>
2425
#include <__atomic/to_gcc_order.h>
2526
#include <__concepts/arithmetic.h>
@@ -322,20 +323,28 @@ struct atomic_ref<_Tp> : public __atomic_ref_base<_Tp> {
322323
atomic_ref& operator=(const atomic_ref&) = delete;
323324

324325
_LIBCPP_HIDE_FROM_ABI _Tp fetch_add(_Tp __arg, memory_order __order = memory_order_seq_cst) const noexcept {
325-
_Tp __old = this->load(memory_order_relaxed);
326-
_Tp __new = __old + __arg;
327-
while (!this->compare_exchange_weak(__old, __new, __order, memory_order_relaxed)) {
328-
__new = __old + __arg;
326+
if constexpr (std::__has_rmw_builtin<_Tp>()) {
327+
return __atomic_fetch_add(this->__ptr_, __arg, std::__to_gcc_order(__order));
328+
} else {
329+
_Tp __old = this->load(memory_order_relaxed);
330+
_Tp __new = __old + __arg;
331+
while (!this->compare_exchange_weak(__old, __new, __order, memory_order_relaxed)) {
332+
__new = __old + __arg;
333+
}
334+
return __old;
329335
}
330-
return __old;
331336
}
332337
_LIBCPP_HIDE_FROM_ABI _Tp fetch_sub(_Tp __arg, memory_order __order = memory_order_seq_cst) const noexcept {
333-
_Tp __old = this->load(memory_order_relaxed);
334-
_Tp __new = __old - __arg;
335-
while (!this->compare_exchange_weak(__old, __new, __order, memory_order_relaxed)) {
336-
__new = __old - __arg;
338+
if constexpr (std::__has_rmw_builtin<_Tp>()) {
339+
return __atomic_fetch_sub(this->__ptr_, __arg, std::__to_gcc_order(__order));
340+
} else {
341+
_Tp __old = this->load(memory_order_relaxed);
342+
_Tp __new = __old - __arg;
343+
while (!this->compare_exchange_weak(__old, __new, __order, memory_order_relaxed)) {
344+
__new = __old - __arg;
345+
}
346+
return __old;
337347
}
338-
return __old;
339348
}
340349

341350
_LIBCPP_HIDE_FROM_ABI _Tp operator+=(_Tp __arg) const noexcept { return fetch_add(__arg) + __arg; }
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef _LIBCPP___ATOMIC_FLOATING_POINT_HELPER_H
10+
#define _LIBCPP___ATOMIC_FLOATING_POINT_HELPER_H
11+
12+
#include <__config>
13+
#include <__type_traits/is_floating_point.h>
14+
#include <__type_traits/is_same.h>
15+
16+
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
17+
# pragma GCC system_header
18+
#endif
19+
20+
_LIBCPP_BEGIN_NAMESPACE_STD
21+
22+
#if _LIBCPP_STD_VER >= 20
23+
24+
template <class _Tp>
25+
_LIBCPP_HIDE_FROM_ABI constexpr bool __is_fp80_long_double() {
26+
// Only x87-fp80 long double has 64-bit mantissa
27+
return __LDBL_MANT_DIG__ == 64 && std::is_same_v<_Tp, long double>;
28+
}
29+
30+
template <class _Tp>
31+
_LIBCPP_HIDE_FROM_ABI constexpr bool __has_rmw_builtin() {
32+
static_assert(std::is_floating_point_v<_Tp>);
33+
# ifndef _LIBCPP_COMPILER_CLANG_BASED
34+
return false;
35+
# else
36+
// The builtin __cxx_atomic_fetch_add errors during compilation for
37+
// long double on platforms with fp80 format.
38+
// For more details, see
39+
// lib/Sema/SemaChecking.cpp function IsAllowedValueType
40+
// LLVM Parser does not allow atomicrmw with x86_fp80 type.
41+
// if (ValType->isSpecificBuiltinType(BuiltinType::LongDouble) &&
42+
// &Context.getTargetInfo().getLongDoubleFormat() ==
43+
// &llvm::APFloat::x87DoubleExtended())
44+
// For more info
45+
// https://llvm.org/PR68602
46+
// https://reviews.llvm.org/D53965
47+
return !std::__is_fp80_long_double<_Tp>();
48+
# endif
49+
}
50+
51+
#endif
52+
53+
_LIBCPP_END_NAMESPACE_STD
54+
55+
#endif // _LIBCPP___ATOMIC_FLOATING_POINT_HELPER_H

libcxx/include/module.modulemap.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -885,6 +885,7 @@ module std [system] {
885885
module check_memory_order { header "__atomic/check_memory_order.h" }
886886
module contention_t { header "__atomic/contention_t.h" }
887887
module fence { header "__atomic/fence.h" }
888+
module floating_point_helper { header "__atomic/floating_point_helper.h" }
888889
module is_always_lock_free { header "__atomic/is_always_lock_free.h" }
889890
module kill_dependency { header "__atomic/kill_dependency.h" }
890891
module memory_order { header "__atomic/memory_order.h" }

libcxx/test/std/atomics/atomics.ref/fetch_add.pass.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
// UNSUPPORTED: c++03, c++11, c++14, c++17
99
// XFAIL: !has-64-bit-atomics
10+
// XFAIL: target={{x86_64-.*}} && tsan
1011

1112
// integral-type fetch_add(integral-type, memory_order = memory_order::seq_cst) const noexcept;
1213
// floating-point-type fetch_add(floating-point-type, memory_order = memory_order::seq_cst) const noexcept;

libcxx/test/std/atomics/atomics.ref/fetch_sub.pass.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
// UNSUPPORTED: c++03, c++11, c++14, c++17
99
// XFAIL: !has-64-bit-atomics
10+
// XFAIL: target={{x86_64-.*}} && tsan
1011

1112
// integral-type fetch_sub(integral-type, memory_order = memory_order::seq_cst) const noexcept;
1213
// floating-point-type fetch_sub(floating-point-type, memory_order = memory_order::seq_cst) const noexcept;

0 commit comments

Comments
 (0)