[libcxx-commits] [libcxx] [libc++] Implement P0493R5: Atomic minimum/maximum (PR #180333)

[Connector Switch via libcxx-commits]

c8ef wrote:

```diff
diff --git a/libcxx/include/__atomic/support/c11.h b/libcxx/include/__atomic/support/c11.h
index 899502422570..c42041499d5e 100644
--- a/libcxx/include/__atomic/support/c11.h
+++ b/libcxx/include/__atomic/support/c11.h
@@ -261,9 +261,90 @@ __cxx_atomic_fetch_xor(__cxx_atomic_base_impl<_Tp>* __a, _Tp __pattern, memory_o
       std::addressof(__a->__a_value), __pattern, static_cast<__memory_order_underlying_t>(__order));
 }
 
-// Clang's __c11_atomic_fetch_max/min builtins do not accept pointer
-// arguments, so dispatch to a CAS loop for pointer types.
-// TODO: Use the builtin for pointer types once Clang accepts them.
+#if _LIBCPP_STD_VER >= 20
+
+// In C++20 and later, dispatch to the builtin if it supports the type,
+// otherwise fallback to a CAS loop (e.g. for pointer types on older Clang builds).
+template <class _Tp>
+concept __has_c11_atomic_minmax = requires(__cxx_atomic_base_impl<_Tp>* __a, _Tp __val) {
+  __c11_atomic_fetch_max(
+      std::addressof(__a->__a_value), __val, static_cast<__memory_order_underlying_t>(memory_order_relaxed));
+  __c11_atomic_fetch_min(
+      std::addressof(__a->__a_value), __val, static_cast<__memory_order_underlying_t>(memory_order_relaxed));
+};
+
+template <class _Tp>
+_LIBCPP_HIDE_FROM_ABI _Tp
+__cxx_atomic_fetch_max(__cxx_atomic_base_impl<_Tp> volatile* __a, _Tp __val, memory_order __order) _NOEXCEPT {
+  if constexpr (__has_c11_atomic_minmax<_Tp>) {
+    return __c11_atomic_fetch_max(
+        std::addressof(__a->__a_value), __val, static_cast<__memory_order_underlying_t>(__order));
+  } else {
+    _Tp __ret = std::__cxx_atomic_load(__a, memory_order_relaxed);
+    _Tp __value;
+    do {
+      __value = __ret > __val ? __ret : __val;
+    } while (
+        !std::__cxx_atomic_compare_exchange_weak(__a, std::addressof(__ret), __value, __order, memory_order_relaxed));
+    return __ret;
+  }
+}
+
+template <class _Tp>
+_LIBCPP_HIDE_FROM_ABI _Tp
+__cxx_atomic_fetch_max(__cxx_atomic_base_impl<_Tp>* __a, _Tp __val, memory_order __order) _NOEXCEPT {
+  if constexpr (__has_c11_atomic_minmax<_Tp>) {
+    return __c11_atomic_fetch_max(
+        std::addressof(__a->__a_value), __val, static_cast<__memory_order_underlying_t>(__order));
+  } else {
+    _Tp __ret = std::__cxx_atomic_load(__a, memory_order_relaxed);
+    _Tp __value;
+    do {
+      __value = __ret > __val ? __ret : __val;
+    } while (
+        !std::__cxx_atomic_compare_exchange_weak(__a, std::addressof(__ret), __value, __order, memory_order_relaxed));
+    return __ret;
+  }
+}
+
+template <class _Tp>
+_LIBCPP_HIDE_FROM_ABI _Tp
+__cxx_atomic_fetch_min(__cxx_atomic_base_impl<_Tp> volatile* __a, _Tp __val, memory_order __order) _NOEXCEPT {
+  if constexpr (__has_c11_atomic_minmax<_Tp>) {
+    return __c11_atomic_fetch_min(
+        std::addressof(__a->__a_value), __val, static_cast<__memory_order_underlying_t>(__order));
+  } else {
+    _Tp __ret = std::__cxx_atomic_load(__a, memory_order_relaxed);
+    _Tp __value;
+    do {
+      __value = __ret < __val ? __ret : __val;
+    } while (
+        !std::__cxx_atomic_compare_exchange_weak(__a, std::addressof(__ret), __value, __order, memory_order_relaxed));
+    return __ret;
+  }
+}
+
+template <class _Tp>
+_LIBCPP_HIDE_FROM_ABI _Tp
+__cxx_atomic_fetch_min(__cxx_atomic_base_impl<_Tp>* __a, _Tp __val, memory_order __order) _NOEXCEPT {
+  if constexpr (__has_c11_atomic_minmax<_Tp>) {
+    return __c11_atomic_fetch_min(
+        std::addressof(__a->__a_value), __val, static_cast<__memory_order_underlying_t>(__order));
+  } else {
+    _Tp __ret = std::__cxx_atomic_load(__a, memory_order_relaxed);
+    _Tp __value;
+    do {
+      __value = __ret < __val ? __ret : __val;
+    } while (
+        !std::__cxx_atomic_compare_exchange_weak(__a, std::addressof(__ret), __value, __order, memory_order_relaxed));
+    return __ret;
+  }
+}
+
+#else // _LIBCPP_STD_VER >= 20
+
+// Clang's __c11_atomic_fetch_max/min builtins didn't originally accept pointer
+// arguments, so dispatch to a CAS loop for pointer types in older standards.
 template <class _Tp, __enable_if_t<!is_pointer<_Tp>::value, int> = 0>
 _LIBCPP_HIDE_FROM_ABI _Tp
 __cxx_atomic_fetch_max(__cxx_atomic_base_impl<_Tp> volatile* __a, _Tp __val, memory_order __order) _NOEXCEPT {
@@ -336,6 +417,8 @@ __cxx_atomic_fetch_min(__cxx_atomic_base_impl<_Tp>* __a, _Tp __val, memory_order
   return __ret;
 }
 
+#endif // _LIBCPP_STD_VER >= 20
+
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___ATOMIC_SUPPORT_C11_H
```

I think if we go with the concept approach, simply applying the above diff to the current branch will suffice. @huixie90 @ldionne, could you please take a look and let me know if applying this patch is preferable to keeping the current branch as is? Really hoping to resolve this final blocker!

https://github.com/llvm/llvm-project/pull/180333