Skip to content

Commit abd5e45

Browse files
authored
[compiler-rt] Use __atomic builtins whenever possible
The code in this file dates back to 2012 when Clang's support for atomic builtins was still quite limited. The bugs referenced in the comment at the top of the file have long been fixed and using the compiler builtins directly should now generate slightly better code. Additionally, this allows using the atomic builtin header for platforms where the __sync_builtins are lacking (e.g. Arm Morello). This change does not introduce any code generation changes for __tsan_read*/__tsan_write* or __tsan_func_{entry,exit} on x86, which indicates the previously noted compiler issues have been fixed. We also have to touch the non-clang codepaths here since the only way we can make this work easily is by making the memory_order enum match the compiler-provided macros, so we have to update the debug checks that assumed the enum was always a bitflag. The one downside of this change is that 32-bit MIPS now definitely requires libatomic (but that may already have been needed for RMW ops). Reviewed By: dvyukov Pull Request: #84439
1 parent 8656d4c commit abd5e45

File tree

8 files changed

+56
-370
lines changed

8 files changed

+56
-370
lines changed

compiler-rt/lib/sanitizer_common/CMakeLists.txt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,6 @@ set(SANITIZER_IMPL_HEADERS
122122
sanitizer_asm.h
123123
sanitizer_atomic.h
124124
sanitizer_atomic_clang.h
125-
sanitizer_atomic_clang_mips.h
126-
sanitizer_atomic_clang_other.h
127-
sanitizer_atomic_clang_x86.h
128125
sanitizer_atomic_msvc.h
129126
sanitizer_bitvector.h
130127
sanitizer_bvgraph.h

compiler-rt/lib/sanitizer_common/sanitizer_atomic.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,24 @@
1818
namespace __sanitizer {
1919

2020
enum memory_order {
21+
// If the __atomic atomic builtins are supported (Clang/GCC), use the
22+
// compiler provided macro values so that we can map the atomic operations
23+
// to __atomic_* directly.
24+
#ifdef __ATOMIC_SEQ_CST
25+
memory_order_relaxed = __ATOMIC_RELAXED,
26+
memory_order_consume = __ATOMIC_CONSUME,
27+
memory_order_acquire = __ATOMIC_ACQUIRE,
28+
memory_order_release = __ATOMIC_RELEASE,
29+
memory_order_acq_rel = __ATOMIC_ACQ_REL,
30+
memory_order_seq_cst = __ATOMIC_SEQ_CST
31+
#else
2132
memory_order_relaxed = 1 << 0,
2233
memory_order_consume = 1 << 1,
2334
memory_order_acquire = 1 << 2,
2435
memory_order_release = 1 << 3,
2536
memory_order_acq_rel = 1 << 4,
2637
memory_order_seq_cst = 1 << 5
38+
#endif
2739
};
2840

2941
struct atomic_uint8_t {

compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h

Lines changed: 40 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -14,60 +14,63 @@
1414
#ifndef SANITIZER_ATOMIC_CLANG_H
1515
#define SANITIZER_ATOMIC_CLANG_H
1616

17-
#if defined(__i386__) || defined(__x86_64__)
18-
# include "sanitizer_atomic_clang_x86.h"
19-
#else
20-
# include "sanitizer_atomic_clang_other.h"
21-
#endif
22-
2317
namespace __sanitizer {
2418

25-
// We would like to just use compiler builtin atomic operations
26-
// for loads and stores, but they are mostly broken in clang:
27-
// - they lead to vastly inefficient code generation
28-
// (http://llvm.org/bugs/show_bug.cgi?id=17281)
29-
// - 64-bit atomic operations are not implemented on x86_32
30-
// (http://llvm.org/bugs/show_bug.cgi?id=15034)
31-
// - they are not implemented on ARM
32-
// error: undefined reference to '__atomic_load_4'
19+
// We use the compiler builtin atomic operations for loads and stores, which
20+
// generates correct code for all architectures, but may require libatomic
21+
// on platforms where e.g. 64-bit atomics are not supported natively.
3322

3423
// See http://www.cl.cam.ac.uk/~pes20/cpp/cpp0xmappings.html
3524
// for mappings of the memory model to different processors.
3625

37-
inline void atomic_signal_fence(memory_order) {
26+
inline void atomic_signal_fence(memory_order mo) { __atomic_signal_fence(mo); }
27+
28+
inline void atomic_thread_fence(memory_order mo) { __atomic_thread_fence(mo); }
29+
30+
inline void proc_yield(int cnt) {
31+
__asm__ __volatile__("" ::: "memory");
32+
#if defined(__i386__) || defined(__x86_64__)
33+
for (int i = 0; i < cnt; i++) __asm__ __volatile__("pause");
3834
__asm__ __volatile__("" ::: "memory");
35+
#endif
3936
}
4037

41-
inline void atomic_thread_fence(memory_order) {
42-
__sync_synchronize();
38+
template <typename T>
39+
inline typename T::Type atomic_load(const volatile T *a, memory_order mo) {
40+
DCHECK(mo == memory_order_relaxed || mo == memory_order_consume ||
41+
mo == memory_order_acquire || mo == memory_order_seq_cst);
42+
DCHECK(!((uptr)a % sizeof(*a)));
43+
return __atomic_load_n(&a->val_dont_use, mo);
4344
}
4445

45-
template<typename T>
46-
inline typename T::Type atomic_fetch_add(volatile T *a,
47-
typename T::Type v, memory_order mo) {
48-
(void)mo;
46+
template <typename T>
47+
inline void atomic_store(volatile T *a, typename T::Type v, memory_order mo) {
48+
DCHECK(mo == memory_order_relaxed || mo == memory_order_release ||
49+
mo == memory_order_seq_cst);
4950
DCHECK(!((uptr)a % sizeof(*a)));
50-
return __sync_fetch_and_add(&a->val_dont_use, v);
51+
__atomic_store_n(&a->val_dont_use, v, mo);
5152
}
5253

53-
template<typename T>
54-
inline typename T::Type atomic_fetch_sub(volatile T *a,
55-
typename T::Type v, memory_order mo) {
54+
template <typename T>
55+
inline typename T::Type atomic_fetch_add(volatile T *a, typename T::Type v,
56+
memory_order mo) {
57+
DCHECK(!((uptr)a % sizeof(*a)));
58+
return __atomic_fetch_add(&a->val_dont_use, v, mo);
59+
}
60+
61+
template <typename T>
62+
inline typename T::Type atomic_fetch_sub(volatile T *a, typename T::Type v,
63+
memory_order mo) {
5664
(void)mo;
5765
DCHECK(!((uptr)a % sizeof(*a)));
58-
return __sync_fetch_and_add(&a->val_dont_use, -v);
66+
return __atomic_fetch_sub(&a->val_dont_use, v, mo);
5967
}
6068

61-
template<typename T>
62-
inline typename T::Type atomic_exchange(volatile T *a,
63-
typename T::Type v, memory_order mo) {
69+
template <typename T>
70+
inline typename T::Type atomic_exchange(volatile T *a, typename T::Type v,
71+
memory_order mo) {
6472
DCHECK(!((uptr)a % sizeof(*a)));
65-
if (mo & (memory_order_release | memory_order_acq_rel | memory_order_seq_cst))
66-
__sync_synchronize();
67-
v = __sync_lock_test_and_set(&a->val_dont_use, v);
68-
if (mo == memory_order_seq_cst)
69-
__sync_synchronize();
70-
return v;
73+
return __atomic_exchange_n(&a->val_dont_use, v, mo);
7174
}
7275

7376
template <typename T>
@@ -82,23 +85,15 @@ inline bool atomic_compare_exchange_strong(volatile T *a, typename T::Type *cmp,
8285
__ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
8386
}
8487

85-
template<typename T>
86-
inline bool atomic_compare_exchange_weak(volatile T *a,
87-
typename T::Type *cmp,
88+
template <typename T>
89+
inline bool atomic_compare_exchange_weak(volatile T *a, typename T::Type *cmp,
8890
typename T::Type xchg,
8991
memory_order mo) {
9092
return atomic_compare_exchange_strong(a, cmp, xchg, mo);
9193
}
9294

9395
} // namespace __sanitizer
9496

95-
// This include provides explicit template instantiations for atomic_uint64_t
96-
// on MIPS32, which does not directly support 8 byte atomics. It has to
97-
// proceed the template definitions above.
98-
#if defined(_MIPS_SIM) && defined(_ABIO32) && _MIPS_SIM == _ABIO32
99-
# include "sanitizer_atomic_clang_mips.h"
100-
#endif
101-
10297
#undef ATOMIC_ORDER
10398

10499
#endif // SANITIZER_ATOMIC_CLANG_H

compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang_mips.h

Lines changed: 0 additions & 117 deletions
This file was deleted.

compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang_other.h

Lines changed: 0 additions & 85 deletions
This file was deleted.

0 commit comments

Comments
 (0)