[libcxx-commits] [PATCH] D103846: [libcxx][atomic] Fix failure mapping in compare_exchange_{strong, weak}.

[Jordan Rupprecht via Phabricator via libcxx-commits]

rupprecht created this revision.
rupprecht added reviewers: ldionne, EricWF, dvyukov.
Herald added a subscriber: jfb.
rupprecht requested review of this revision.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

https://eel.is/c++draft/atomics.types.operations#23 says: ... the value of failure is order except that a value of `memory_order::acq_rel` shall be replaced by the value `memory_order::acquire` and a value of `memory_order::release` shall be replaced by the value `memory_order::relaxed`.

This failure mapping is only handled for `_LIBCPP_HAS_GCC_ATOMIC_IMP`. We are seeing bad code generation for `compare_exchange_strong(cmp, 1, std::memory_order_acq_rel)` when using libc++ in place of libstdc++: https://godbolt.org/z/v3onrrq4G.

This was caught by tsan tests after D99434 <https://reviews.llvm.org/D99434>, `[TSAN] Honor failure memory orders in AtomicCAS`, but appears to be an issue in non-tsan code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103846

Files:
  libcxx/include/atomic


Index: libcxx/include/atomic
===================================================================
--- libcxx/include/atomic
+++ libcxx/include/atomic
@@ -1017,26 +1017,33 @@
     return __c11_atomic_exchange(&__a->__a_value, __value, static_cast<__memory_order_underlying_t>(__order));
 }
 
+_LIBCPP_INLINE_VISIBILITY inline _LIBCPP_CONSTEXPR memory_order __to_failure_order(memory_order __order) {
+  // Avoid switch statement to make this a constexpr.
+  return __order == memory_order_release ? memory_order_relaxed:
+         (__order == memory_order_acq_rel ? memory_order_acquire:
+             __order);
+}
+
 template<class _Tp>
 _LIBCPP_INLINE_VISIBILITY
 bool __cxx_atomic_compare_exchange_strong(__cxx_atomic_base_impl<_Tp> volatile* __a, _Tp* __expected, _Tp __value, memory_order __success, memory_order __failure) _NOEXCEPT {
-    return __c11_atomic_compare_exchange_strong(&__a->__a_value, __expected, __value, static_cast<__memory_order_underlying_t>(__success), static_cast<__memory_order_underlying_t>(__failure));
+    return __c11_atomic_compare_exchange_strong(&__a->__a_value, __expected, __value, static_cast<__memory_order_underlying_t>(__success), static_cast<__memory_order_underlying_t>(__to_failure_order(__failure)));
 }
 template<class _Tp>
 _LIBCPP_INLINE_VISIBILITY
 bool __cxx_atomic_compare_exchange_strong(__cxx_atomic_base_impl<_Tp> * __a, _Tp* __expected, _Tp __value, memory_order __success, memory_order __failure) _NOEXCEPT {
-    return __c11_atomic_compare_exchange_strong(&__a->__a_value, __expected, __value, static_cast<__memory_order_underlying_t>(__success), static_cast<__memory_order_underlying_t>(__failure));
+    return __c11_atomic_compare_exchange_strong(&__a->__a_value, __expected, __value, static_cast<__memory_order_underlying_t>(__success), static_cast<__memory_order_underlying_t>(__to_failure_order(__failure)));
 }
 
 template<class _Tp>
 _LIBCPP_INLINE_VISIBILITY
 bool __cxx_atomic_compare_exchange_weak(__cxx_atomic_base_impl<_Tp> volatile* __a, _Tp* __expected, _Tp __value, memory_order __success, memory_order __failure) _NOEXCEPT {
-    return __c11_atomic_compare_exchange_weak(&__a->__a_value, __expected, __value, static_cast<__memory_order_underlying_t>(__success), static_cast<__memory_order_underlying_t>(__failure));
+    return __c11_atomic_compare_exchange_weak(&__a->__a_value, __expected, __value, static_cast<__memory_order_underlying_t>(__success), static_cast<__memory_order_underlying_t>(__to_failure_order(__failure)));
 }
 template<class _Tp>
 _LIBCPP_INLINE_VISIBILITY
 bool __cxx_atomic_compare_exchange_weak(__cxx_atomic_base_impl<_Tp> * __a, _Tp* __expected, _Tp __value, memory_order __success, memory_order __failure) _NOEXCEPT {
-    return __c11_atomic_compare_exchange_weak(&__a->__a_value, __expected, __value,  static_cast<__memory_order_underlying_t>(__success), static_cast<__memory_order_underlying_t>(__failure));
+    return __c11_atomic_compare_exchange_weak(&__a->__a_value, __expected, __value,  static_cast<__memory_order_underlying_t>(__success), static_cast<__memory_order_underlying_t>(__to_failure_order(__failure)));
 }
 
 template<class _Tp>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D103846.350424.patch
Type: text/x-patch
Size: 3154 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20210607/fa2b0e5e/attachment.bin>