Skip to content

[libc][CPP] correct cmpxchg failure ordering inference #133127

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.

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 5 commits into from
Mar 26, 2025

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