Skip to content

Conversation

@SchrodingerZhu
Copy link
Contributor

@SchrodingerZhu SchrodingerZhu commented Mar 26, 2025

See https://en.cppreference.com/w/cpp/atomic/atomic/compare_exchange. The failure order should be inferred from the success order if it is not explicitly specified.

@llvmbot llvmbot added the libc label Mar 26, 2025
@SchrodingerZhu SchrodingerZhu requested a review from jhuber6 March 26, 2025 17:14
@llvmbot
Copy link
Member

llvmbot commented Mar 26, 2025

@llvm/pr-subscribers-libc

Author: Schrodinger ZHU Yifan (SchrodingerZhu)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/133127.diff

1 Files Affected:

  • (modified) libc/src/__support/CPP/atomic.h (+20-3)
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)

@SchrodingerZhu SchrodingerZhu requested a review from jhuber6 March 26, 2025 18:21
@SchrodingerZhu SchrodingerZhu merged commit 7b130f4 into llvm:main Mar 26, 2025
4 of 5 checks passed
@SchrodingerZhu SchrodingerZhu deleted the fix-atomic-cmpxchg-ordering branch March 26, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants