[libc-commits] [libc] [libc][CPP] correct cmpxchg failure ordering inference (PR #133127)
Schrodinger ZHU Yifan via libc-commits
libc-commits at lists.llvm.org
Wed Mar 26 11:42:25 PDT 2025
https://github.com/SchrodingerZhu updated https://github.com/llvm/llvm-project/pull/133127
>From 46f0710ca04a670846e85e7de8163f8d9667c4fc Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Wed, 26 Mar 2025 13:12:56 -0400
Subject: [PATCH 1/5] [libc][CPP] correct cmpxchg failure ordering inference
---
libc/src/__support/CPP/atomic.h | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/libc/src/__support/CPP/atomic.h b/libc/src/__support/CPP/atomic.h
index 15242a131c63b..3a30c7d33faea 100644
--- a/libc/src/__support/CPP/atomic.h
+++ b/libc/src/__support/CPP/atomic.h
@@ -62,6 +62,14 @@ template <typename T> struct Atomic {
return static_cast<int>(mem_scope);
}
+ LIBC_INLINE static constexpr int infer_failure_order(MemoryOrder mem_ord) {
+ if (mem_ord == MemoryOrder::RELEASE)
+ return order(MemoryOrder::RELAXED);
+ if (mem_ord == MemoryOrder::ACQ_REL)
+ return order(MemoryOrder::ACQUIRE);
+ return order(mem_ord);
+ }
+
LIBC_INLINE static T *addressof(T &ref) { return __builtin_addressof(ref); }
// Require types that are 1, 2, 4, 8, or 16 bytes in length to be aligned to
@@ -129,7 +137,7 @@ template <typename T> struct Atomic {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
return __atomic_compare_exchange(addressof(val), addressof(expected),
addressof(desired), false, order(mem_ord),
- order(mem_ord));
+ infer_failure_order(mem_ord));
}
// Atomic compare exchange (separate success and failure memory orders)
@@ -148,7 +156,7 @@ template <typename T> struct Atomic {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
return __atomic_compare_exchange(addressof(val), addressof(expected),
addressof(desired), true, order(mem_ord),
- order(mem_ord));
+ infer_failure_order(mem_ord));
}
// Atomic compare exchange (weak version with separate success and failure
@@ -252,6 +260,14 @@ template <typename T> struct AtomicRef {
return static_cast<int>(mem_scope);
}
+ LIBC_INLINE static constexpr int infer_failure_order(MemoryOrder mem_ord) {
+ if (mem_ord == MemoryOrder::RELEASE)
+ return order(MemoryOrder::RELAXED);
+ if (mem_ord == MemoryOrder::ACQ_REL)
+ return order(MemoryOrder::ACQUIRE);
+ return order(mem_ord);
+ }
+
public:
// Constructor from T reference
LIBC_INLINE explicit constexpr AtomicRef(T &obj) : ptr(&obj) {}
@@ -298,7 +314,8 @@ template <typename T> struct AtomicRef {
T &expected, T desired, MemoryOrder mem_ord = MemoryOrder::SEQ_CST,
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) const {
return __atomic_compare_exchange(ptr, &expected, &desired, false,
- order(mem_ord), order(mem_ord));
+ order(mem_ord),
+ infer_failure_order(mem_ord));
}
// Atomic compare exchange (strong, separate success/failure memory orders)
>From 6c9f6995c90abcd7d4e9827efe6298d5d8933d9b Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Wed, 26 Mar 2025 13:36:32 -0400
Subject: [PATCH 2/5] address comments
---
libc/src/__support/CPP/atomic.h | 42 ++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 17 deletions(-)
diff --git a/libc/src/__support/CPP/atomic.h b/libc/src/__support/CPP/atomic.h
index 3a30c7d33faea..be37734e3e08a 100644
--- a/libc/src/__support/CPP/atomic.h
+++ b/libc/src/__support/CPP/atomic.h
@@ -40,6 +40,27 @@ enum class MemoryScope : int {
#endif
};
+namespace atomic {
+
+LIBC_INLINE constexpr int order(MemoryOrder mem_ord) {
+ return static_cast<int>(mem_ord);
+}
+
+LIBC_INLINE constexpr int scope(MemoryScope mem_scope) {
+ return static_cast<int>(mem_scope);
+}
+template <class T> LIBC_INLINE T *addressof(T &ref) {
+ return __builtin_addressof(ref);
+}
+
+LIBC_INLINE constexpr int infer_failure_order(MemoryOrder mem_ord) {
+ if (mem_ord == MemoryOrder::RELEASE)
+ return order(MemoryOrder::RELAXED);
+ if (mem_ord == MemoryOrder::ACQ_REL)
+ return order(MemoryOrder::ACQUIRE);
+ return order(mem_ord);
+}
+
template <typename T> struct Atomic {
static_assert(is_trivially_copyable_v<T> && is_copy_constructible_v<T> &&
is_move_constructible_v<T> && is_copy_assignable_v<T> &&
@@ -54,23 +75,6 @@ template <typename T> struct Atomic {
private:
// type conversion helper to avoid long c++ style casts
- LIBC_INLINE static int order(MemoryOrder mem_ord) {
- return static_cast<int>(mem_ord);
- }
-
- LIBC_INLINE static int scope(MemoryScope mem_scope) {
- return static_cast<int>(mem_scope);
- }
-
- LIBC_INLINE static constexpr int infer_failure_order(MemoryOrder mem_ord) {
- if (mem_ord == MemoryOrder::RELEASE)
- return order(MemoryOrder::RELAXED);
- if (mem_ord == MemoryOrder::ACQ_REL)
- return order(MemoryOrder::ACQUIRE);
- return order(mem_ord);
- }
-
- LIBC_INLINE static T *addressof(T &ref) { return __builtin_addressof(ref); }
// Require types that are 1, 2, 4, 8, or 16 bytes in length to be aligned to
// at least their size to be potentially used lock-free.
@@ -419,6 +423,10 @@ LIBC_INLINE void atomic_signal_fence([[maybe_unused]] MemoryOrder mem_ord) {
asm volatile("" ::: "memory");
#endif
}
+} // namespace atomic
+
+using atomic::Atomic;
+using atomic::AtomicRef;
} // namespace cpp
} // namespace LIBC_NAMESPACE_DECL
>From 4d23450c97e0bdfe5d2087c253158d724f983eda Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Wed, 26 Mar 2025 13:37:51 -0400
Subject: [PATCH 3/5] address comments
---
libc/src/__support/CPP/atomic.h | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/libc/src/__support/CPP/atomic.h b/libc/src/__support/CPP/atomic.h
index be37734e3e08a..166eb367b5054 100644
--- a/libc/src/__support/CPP/atomic.h
+++ b/libc/src/__support/CPP/atomic.h
@@ -256,22 +256,6 @@ template <typename T> struct AtomicRef {
private:
T *ptr;
- LIBC_INLINE static int order(MemoryOrder mem_ord) {
- return static_cast<int>(mem_ord);
- }
-
- LIBC_INLINE static int scope(MemoryScope mem_scope) {
- return static_cast<int>(mem_scope);
- }
-
- LIBC_INLINE static constexpr int infer_failure_order(MemoryOrder mem_ord) {
- if (mem_ord == MemoryOrder::RELEASE)
- return order(MemoryOrder::RELAXED);
- if (mem_ord == MemoryOrder::ACQ_REL)
- return order(MemoryOrder::ACQUIRE);
- return order(mem_ord);
- }
-
public:
// Constructor from T reference
LIBC_INLINE explicit constexpr AtomicRef(T &obj) : ptr(&obj) {}
>From c3811ddf72bbdf0be1597015fd948b5e8b708be6 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Wed, 26 Mar 2025 13:38:52 -0400
Subject: [PATCH 4/5] unify spacing
---
libc/src/__support/CPP/atomic.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/libc/src/__support/CPP/atomic.h b/libc/src/__support/CPP/atomic.h
index 166eb367b5054..08703d65227c7 100644
--- a/libc/src/__support/CPP/atomic.h
+++ b/libc/src/__support/CPP/atomic.h
@@ -49,6 +49,7 @@ LIBC_INLINE constexpr int order(MemoryOrder mem_ord) {
LIBC_INLINE constexpr int scope(MemoryScope mem_scope) {
return static_cast<int>(mem_scope);
}
+
template <class T> LIBC_INLINE T *addressof(T &ref) {
return __builtin_addressof(ref);
}
>From d50e1f6d7e16215ff577018c7e46fc48c0a19735 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Wed, 26 Mar 2025 14:42:05 -0400
Subject: [PATCH 5/5] update
---
libc/src/__support/CPP/atomic.h | 134 +++++++++++++++++---------------
1 file changed, 72 insertions(+), 62 deletions(-)
diff --git a/libc/src/__support/CPP/atomic.h b/libc/src/__support/CPP/atomic.h
index 08703d65227c7..f21755293102e 100644
--- a/libc/src/__support/CPP/atomic.h
+++ b/libc/src/__support/CPP/atomic.h
@@ -40,8 +40,7 @@ enum class MemoryScope : int {
#endif
};
-namespace atomic {
-
+namespace impl {
LIBC_INLINE constexpr int order(MemoryOrder mem_ord) {
return static_cast<int>(mem_ord);
}
@@ -61,6 +60,7 @@ LIBC_INLINE constexpr int infer_failure_order(MemoryOrder mem_ord) {
return order(MemoryOrder::ACQUIRE);
return order(mem_ord);
}
+} // namespace impl
template <typename T> struct Atomic {
static_assert(is_trivially_copyable_v<T> && is_copy_constructible_v<T> &&
@@ -111,10 +111,11 @@ template <typename T> struct Atomic {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
T res;
#if __has_builtin(__scoped_atomic_load)
- __scoped_atomic_load(addressof(val), addressof(res), order(mem_ord),
- scope(mem_scope));
+ __scoped_atomic_load(impl::addressof(val), impl::addressof(res),
+ impl::order(mem_ord), impl::scope(mem_scope));
#else
- __atomic_load(addressof(val), addressof(res), order(mem_ord));
+ __atomic_load(impl::addressof(val), impl::addressof(res),
+ impl::order(mem_ord));
#endif
return res;
}
@@ -129,10 +130,11 @@ template <typename T> struct Atomic {
store(T rhs, MemoryOrder mem_ord = MemoryOrder::SEQ_CST,
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
#if __has_builtin(__scoped_atomic_store)
- __scoped_atomic_store(addressof(val), addressof(rhs), order(mem_ord),
- scope(mem_scope));
+ __scoped_atomic_store(impl::addressof(val), impl::addressof(rhs),
+ impl::order(mem_ord), impl::scope(mem_scope));
#else
- __atomic_store(addressof(val), addressof(rhs), order(mem_ord));
+ __atomic_store(impl::addressof(val), impl::addressof(rhs),
+ impl::order(mem_ord));
#endif
}
@@ -140,9 +142,10 @@ template <typename T> struct Atomic {
LIBC_INLINE bool compare_exchange_strong(
T &expected, T desired, MemoryOrder mem_ord = MemoryOrder::SEQ_CST,
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
- return __atomic_compare_exchange(addressof(val), addressof(expected),
- addressof(desired), false, order(mem_ord),
- infer_failure_order(mem_ord));
+ return __atomic_compare_exchange(
+ impl::addressof(val), impl::addressof(expected),
+ impl::addressof(desired), false, impl::order(mem_ord),
+ impl::infer_failure_order(mem_ord));
}
// Atomic compare exchange (separate success and failure memory orders)
@@ -151,17 +154,19 @@ template <typename T> struct Atomic {
MemoryOrder failure_order,
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
return __atomic_compare_exchange(
- addressof(val), addressof(expected), addressof(desired), false,
- order(success_order), order(failure_order));
+ impl::addressof(val), impl::addressof(expected),
+ impl::addressof(desired), false, impl::order(success_order),
+ impl::order(failure_order));
}
// Atomic compare exchange (weak version)
LIBC_INLINE bool compare_exchange_weak(
T &expected, T desired, MemoryOrder mem_ord = MemoryOrder::SEQ_CST,
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
- return __atomic_compare_exchange(addressof(val), addressof(expected),
- addressof(desired), true, order(mem_ord),
- infer_failure_order(mem_ord));
+ return __atomic_compare_exchange(
+ impl::addressof(val), impl::addressof(expected),
+ impl::addressof(desired), true, impl::order(mem_ord),
+ impl::infer_failure_order(mem_ord));
}
// Atomic compare exchange (weak version with separate success and failure
@@ -171,8 +176,9 @@ template <typename T> struct Atomic {
MemoryOrder failure_order,
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
return __atomic_compare_exchange(
- addressof(val), addressof(expected), addressof(desired), true,
- order(success_order), order(failure_order));
+ impl::addressof(val), impl::addressof(expected),
+ impl::addressof(desired), true, impl::order(success_order),
+ impl::order(failure_order));
}
LIBC_INLINE T
@@ -180,11 +186,12 @@ template <typename T> struct Atomic {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
T ret;
#if __has_builtin(__scoped_atomic_exchange)
- __scoped_atomic_exchange(addressof(val), addressof(desired), addressof(ret),
- order(mem_ord), scope(mem_scope));
+ __scoped_atomic_exchange(impl::addressof(val), impl::addressof(desired),
+ impl::addressof(ret), impl::order(mem_ord),
+ impl::scope(mem_scope));
#else
- __atomic_exchange(addressof(val), addressof(desired), addressof(ret),
- order(mem_ord));
+ __atomic_exchange(impl::addressof(val), impl::addressof(desired),
+ impl::addressof(ret), impl::order(mem_ord));
#endif
return ret;
}
@@ -194,10 +201,12 @@ template <typename T> struct Atomic {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
static_assert(cpp::is_integral_v<T>, "T must be an integral type.");
#if __has_builtin(__scoped_atomic_fetch_add)
- return __scoped_atomic_fetch_add(addressof(val), increment, order(mem_ord),
- scope(mem_scope));
+ return __scoped_atomic_fetch_add(impl::addressof(val), increment,
+ impl::order(mem_ord),
+ impl::scope(mem_scope));
#else
- return __atomic_fetch_add(addressof(val), increment, order(mem_ord));
+ return __atomic_fetch_add(impl::addressof(val), increment,
+ impl::order(mem_ord));
#endif
}
@@ -206,10 +215,11 @@ template <typename T> struct Atomic {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
static_assert(cpp::is_integral_v<T>, "T must be an integral type.");
#if __has_builtin(__scoped_atomic_fetch_or)
- return __scoped_atomic_fetch_or(addressof(val), mask, order(mem_ord),
- scope(mem_scope));
+ return __scoped_atomic_fetch_or(impl::addressof(val), mask,
+ impl::order(mem_ord),
+ impl::scope(mem_scope));
#else
- return __atomic_fetch_or(addressof(val), mask, order(mem_ord));
+ return __atomic_fetch_or(impl::addressof(val), mask, impl::order(mem_ord));
#endif
}
@@ -218,10 +228,11 @@ template <typename T> struct Atomic {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
static_assert(cpp::is_integral_v<T>, "T must be an integral type.");
#if __has_builtin(__scoped_atomic_fetch_and)
- return __scoped_atomic_fetch_and(addressof(val), mask, order(mem_ord),
- scope(mem_scope));
+ return __scoped_atomic_fetch_and(impl::addressof(val), mask,
+ impl::order(mem_ord),
+ impl::scope(mem_scope));
#else
- return __atomic_fetch_and(addressof(val), mask, order(mem_ord));
+ return __atomic_fetch_and(impl::addressof(val), mask, impl::order(mem_ord));
#endif
}
@@ -230,10 +241,12 @@ template <typename T> struct Atomic {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
static_assert(cpp::is_integral_v<T>, "T must be an integral type.");
#if __has_builtin(__scoped_atomic_fetch_sub)
- return __scoped_atomic_fetch_sub(addressof(val), decrement, order(mem_ord),
- scope(mem_scope));
+ return __scoped_atomic_fetch_sub(impl::addressof(val), decrement,
+ impl::order(mem_ord),
+ impl::scope(mem_scope));
#else
- return __atomic_fetch_sub(addressof(val), decrement, order(mem_ord));
+ return __atomic_fetch_sub(impl::addressof(val), decrement,
+ impl::order(mem_ord));
#endif
}
@@ -275,9 +288,10 @@ template <typename T> struct AtomicRef {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) const {
T res;
#if __has_builtin(__scoped_atomic_load)
- __scoped_atomic_load(ptr, &res, order(mem_ord), scope(mem_scope));
+ __scoped_atomic_load(ptr, &res, impl::order(mem_ord),
+ impl::scope(mem_scope));
#else
- __atomic_load(ptr, &res, order(mem_ord));
+ __atomic_load(ptr, &res, impl::order(mem_ord));
#endif
return res;
}
@@ -292,9 +306,10 @@ template <typename T> struct AtomicRef {
store(T rhs, MemoryOrder mem_ord = MemoryOrder::SEQ_CST,
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) const {
#if __has_builtin(__scoped_atomic_store)
- __scoped_atomic_store(ptr, &rhs, order(mem_ord), scope(mem_scope));
+ __scoped_atomic_store(ptr, &rhs, impl::order(mem_ord),
+ impl::scope(mem_scope));
#else
- __atomic_store(ptr, &rhs, order(mem_ord));
+ __atomic_store(ptr, &rhs, impl::order(mem_ord));
#endif
}
@@ -303,8 +318,8 @@ template <typename T> struct AtomicRef {
T &expected, T desired, MemoryOrder mem_ord = MemoryOrder::SEQ_CST,
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) const {
return __atomic_compare_exchange(ptr, &expected, &desired, false,
- order(mem_ord),
- infer_failure_order(mem_ord));
+ impl::order(mem_ord),
+ impl::infer_failure_order(mem_ord));
}
// Atomic compare exchange (strong, separate success/failure memory orders)
@@ -313,8 +328,8 @@ template <typename T> struct AtomicRef {
MemoryOrder failure_order,
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) const {
return __atomic_compare_exchange(ptr, &expected, &desired, false,
- order(success_order),
- order(failure_order));
+ impl::order(success_order),
+ impl::order(failure_order));
}
// Atomic exchange
@@ -323,10 +338,10 @@ template <typename T> struct AtomicRef {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) const {
T ret;
#if __has_builtin(__scoped_atomic_exchange)
- __scoped_atomic_exchange(ptr, &desired, &ret, order(mem_ord),
- scope(mem_scope));
+ __scoped_atomic_exchange(ptr, &desired, &ret, impl::order(mem_ord),
+ impl::scope(mem_scope));
#else
- __atomic_exchange(ptr, &desired, &ret, order(mem_ord));
+ __atomic_exchange(ptr, &desired, &ret, impl::order(mem_ord));
#endif
return ret;
}
@@ -336,10 +351,10 @@ template <typename T> struct AtomicRef {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) const {
static_assert(cpp::is_integral_v<T>, "T must be an integral type.");
#if __has_builtin(__scoped_atomic_fetch_add)
- return __scoped_atomic_fetch_add(ptr, increment, order(mem_ord),
- scope(mem_scope));
+ return __scoped_atomic_fetch_add(ptr, increment, impl::order(mem_ord),
+ impl::scope(mem_scope));
#else
- return __atomic_fetch_add(ptr, increment, order(mem_ord));
+ return __atomic_fetch_add(ptr, increment, impl::order(mem_ord));
#endif
}
@@ -348,10 +363,10 @@ template <typename T> struct AtomicRef {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) const {
static_assert(cpp::is_integral_v<T>, "T must be an integral type.");
#if __has_builtin(__scoped_atomic_fetch_or)
- return __scoped_atomic_fetch_or(ptr, mask, order(mem_ord),
- scope(mem_scope));
+ return __scoped_atomic_fetch_or(ptr, mask, impl::order(mem_ord),
+ impl::scope(mem_scope));
#else
- return __atomic_fetch_or(ptr, mask, order(mem_ord));
+ return __atomic_fetch_or(ptr, mask, impl::order(mem_ord));
#endif
}
@@ -360,10 +375,10 @@ template <typename T> struct AtomicRef {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) const {
static_assert(cpp::is_integral_v<T>, "T must be an integral type.");
#if __has_builtin(__scoped_atomic_fetch_and)
- return __scoped_atomic_fetch_and(ptr, mask, order(mem_ord),
- scope(mem_scope));
+ return __scoped_atomic_fetch_and(ptr, mask, impl::order(mem_ord),
+ impl::scope(mem_scope));
#else
- return __atomic_fetch_and(ptr, mask, order(mem_ord));
+ return __atomic_fetch_and(ptr, mask, impl::order(mem_ord));
#endif
}
@@ -372,10 +387,10 @@ template <typename T> struct AtomicRef {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) const {
static_assert(cpp::is_integral_v<T>, "T must be an integral type.");
#if __has_builtin(__scoped_atomic_fetch_sub)
- return __scoped_atomic_fetch_sub(ptr, decrement, order(mem_ord),
- scope(mem_scope));
+ return __scoped_atomic_fetch_sub(ptr, decrement, impl::order(mem_ord),
+ impl::scope(mem_scope));
#else
- return __atomic_fetch_sub(ptr, decrement, order(mem_ord));
+ return __atomic_fetch_sub(ptr, decrement, impl::order(mem_ord));
#endif
}
};
@@ -408,11 +423,6 @@ LIBC_INLINE void atomic_signal_fence([[maybe_unused]] MemoryOrder mem_ord) {
asm volatile("" ::: "memory");
#endif
}
-} // namespace atomic
-
-using atomic::Atomic;
-using atomic::AtomicRef;
-
} // namespace cpp
} // namespace LIBC_NAMESPACE_DECL
More information about the libc-commits
mailing list