[libc-commits] [libc] [libc] fix and explicit atomic difference from c++ (PR #119715)
Schrodinger ZHU Yifan via libc-commits
libc-commits at lists.llvm.org
Thu Dec 12 09:20:43 PST 2024
https://github.com/SchrodingerZhu updated https://github.com/llvm/llvm-project/pull/119715
>From 7979fb9fcd0bd295ebb69a58803e46373ec44dd7 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <i at zhuyi.fan>
Date: Thu, 12 Dec 2024 07:32:54 -0800
Subject: [PATCH 1/3] [libc] fix and explicit atomic difference from c++
- align alignment requirement with atomic_ref in libc++, which is more
modern
- use `addressof` to avoid problems if future users override their
address operator
- check `has_unique_object_representations` and rejects padded objects
in libc's implementations
---
libc/src/__support/CPP/atomic.h | 77 +++++++++++--------
libc/src/__support/CPP/type_traits.h | 2 +
.../has_unique_object_representations.h | 28 +++++++
3 files changed, 77 insertions(+), 30 deletions(-)
create mode 100644 libc/src/__support/CPP/type_traits/has_unique_object_representations.h
diff --git a/libc/src/__support/CPP/atomic.h b/libc/src/__support/CPP/atomic.h
index a9fc7e61610ccc..dfe1dc9591e12b 100644
--- a/libc/src/__support/CPP/atomic.h
+++ b/libc/src/__support/CPP/atomic.h
@@ -9,6 +9,7 @@
#ifndef LLVM_LIBC_SRC___SUPPORT_CPP_ATOMIC_H
#define LLVM_LIBC_SRC___SUPPORT_CPP_ATOMIC_H
+#include "src/__support/CPP/type_traits/has_unique_object_representations.h"
#include "src/__support/macros/attributes.h"
#include "src/__support/macros/config.h"
#include "src/__support/macros/properties/architectures.h"
@@ -47,12 +48,11 @@ template <typename T> struct Atomic {
"constructible, move constructible, copy assignable, "
"and move assignable.");
+ static_assert(cpp::has_unique_object_representations_v<T>,
+ "atomic<T> in libc only support types whose values has unique "
+ "object representations.");
+
private:
- // The value stored should be appropriately aligned so that
- // hardware instructions used to perform atomic operations work
- // correctly.
- static constexpr int ALIGNMENT = sizeof(T) > alignof(T) ? sizeof(T)
- : alignof(T);
// type conversion helper to avoid long c++ style casts
LIBC_INLINE static int order(MemoryOrder mem_ord) {
return static_cast<int>(mem_ord);
@@ -62,6 +62,18 @@ template <typename T> struct Atomic {
return static_cast<int>(mem_scope);
}
+ 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
+ LIBC_INLINE_VAR static constexpr size_t MIN_ALIGNMENT =
+ (sizeof(T) & (sizeof(T) - 1)) || (sizeof(T) > 16) ? 0 : sizeof(T);
+
+ LIBC_INLINE_VAR static constexpr size_t ALIGNMENT = alignof(T) > MIN_ALIGNMENT
+ ? alignof(T)
+ : MIN_ALIGNMENT;
+
public:
using value_type = T;
@@ -87,9 +99,10 @@ template <typename T> struct Atomic {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
T res;
#if __has_builtin(__scoped_atomic_load)
- __scoped_atomic_load(&val, &res, order(mem_ord), scope(mem_scope));
+ __scoped_atomic_load(addressof(val), addressof(res), order(mem_ord),
+ scope(mem_scope));
#else
- __atomic_load(&val, &res, order(mem_ord));
+ __atomic_load(addressof(val), addressof(res), order(mem_ord));
#endif
return res;
}
@@ -104,9 +117,10 @@ 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(&val, &rhs, order(mem_ord), scope(mem_scope));
+ __scoped_atomic_store(addressof(val), addressof(rhs), order(mem_ord),
+ scope(mem_scope));
#else
- __atomic_store(&val, &rhs, order(mem_ord));
+ __atomic_store(addressof(val), addressof(rhs), order(mem_ord));
#endif
}
@@ -114,8 +128,9 @@ 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(&val, &expected, &desired, false,
- order(mem_ord), order(mem_ord));
+ return __atomic_compare_exchange(addressof(val), addressof(expected),
+ addressof(desired), false, order(mem_ord),
+ order(mem_ord));
}
// Atomic compare exchange (separate success and failure memory orders)
@@ -123,17 +138,18 @@ template <typename T> struct Atomic {
T &expected, T desired, MemoryOrder success_order,
MemoryOrder failure_order,
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
- return __atomic_compare_exchange(&val, &expected, &desired, false,
- order(success_order),
- order(failure_order));
+ return __atomic_compare_exchange(
+ addressof(val), addressof(expected), addressof(desired), false,
+ order(success_order), 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(&val, &expected, &desired, true,
- order(mem_ord), order(mem_ord));
+ return __atomic_compare_exchange(addressof(val), addressof(expected),
+ addressof(desired), true, order(mem_ord),
+ order(mem_ord));
}
// Atomic compare exchange (weak version with separate success and failure
@@ -142,9 +158,9 @@ template <typename T> struct Atomic {
T &expected, T desired, MemoryOrder success_order,
MemoryOrder failure_order,
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
- return __atomic_compare_exchange(&val, &expected, &desired, true,
- order(success_order),
- order(failure_order));
+ return __atomic_compare_exchange(
+ addressof(val), addressof(expected), addressof(desired), true,
+ order(success_order), order(failure_order));
}
LIBC_INLINE T
@@ -152,10 +168,11 @@ template <typename T> struct Atomic {
[[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) {
T ret;
#if __has_builtin(__scoped_atomic_exchange)
- __scoped_atomic_exchange(&val, &desired, &ret, order(mem_ord),
- scope(mem_scope));
+ __scoped_atomic_exchange(addressof(val), addressof(desired), addressof(ret),
+ order(mem_ord), scope(mem_scope));
#else
- __atomic_exchange(&val, &desired, &ret, order(mem_ord));
+ __atomic_exchange(addressof(val), addressof(desired), addressof(ret),
+ order(mem_ord));
#endif
return ret;
}
@@ -165,10 +182,10 @@ 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(&val, increment, order(mem_ord),
+ return __scoped_atomic_fetch_add(addressof(val), increment, order(mem_ord),
scope(mem_scope));
#else
- return __atomic_fetch_add(&val, increment, order(mem_ord));
+ return __atomic_fetch_add(addressof(val), increment, order(mem_ord));
#endif
}
@@ -177,10 +194,10 @@ 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(&val, mask, order(mem_ord),
+ return __scoped_atomic_fetch_or(addressof(val), mask, order(mem_ord),
scope(mem_scope));
#else
- return __atomic_fetch_or(&val, mask, order(mem_ord));
+ return __atomic_fetch_or(addressof(val), mask, order(mem_ord));
#endif
}
@@ -189,10 +206,10 @@ 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(&val, mask, order(mem_ord),
+ return __scoped_atomic_fetch_and(addressof(val), mask, order(mem_ord),
scope(mem_scope));
#else
- return __atomic_fetch_and(&val, mask, order(mem_ord));
+ return __atomic_fetch_and(addressof(val), mask, order(mem_ord));
#endif
}
@@ -201,10 +218,10 @@ 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(&val, decrement, order(mem_ord),
+ return __scoped_atomic_fetch_sub(addressof(val), decrement, order(mem_ord),
scope(mem_scope));
#else
- return __atomic_fetch_sub(&val, decrement, order(mem_ord));
+ return __atomic_fetch_sub(addressof(val), decrement, order(mem_ord));
#endif
}
diff --git a/libc/src/__support/CPP/type_traits.h b/libc/src/__support/CPP/type_traits.h
index b9bc5b85684415..2b3914521cbe98 100644
--- a/libc/src/__support/CPP/type_traits.h
+++ b/libc/src/__support/CPP/type_traits.h
@@ -18,6 +18,7 @@
#include "src/__support/CPP/type_traits/decay.h"
#include "src/__support/CPP/type_traits/enable_if.h"
#include "src/__support/CPP/type_traits/false_type.h"
+#include "src/__support/CPP/type_traits/has_unique_object_representations.h"
#include "src/__support/CPP/type_traits/integral_constant.h"
#include "src/__support/CPP/type_traits/invoke.h"
#include "src/__support/CPP/type_traits/invoke_result.h"
@@ -65,4 +66,5 @@
#include "src/__support/CPP/type_traits/type_identity.h"
#include "src/__support/CPP/type_traits/void_t.h"
+
#endif // LLVM_LIBC_SRC___SUPPORT_CPP_TYPE_TRAITS_H
diff --git a/libc/src/__support/CPP/type_traits/has_unique_object_representations.h b/libc/src/__support/CPP/type_traits/has_unique_object_representations.h
new file mode 100644
index 00000000000000..baed7da69d3199
--- /dev/null
+++ b/libc/src/__support/CPP/type_traits/has_unique_object_representations.h
@@ -0,0 +1,28 @@
+//===-- has_unique_object_representations type_traits ------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_LIBC_SRC___SUPPORT_CPP_TYPE_TRAITS_HAS_UNIQUE_OBJECT_REPRESENTATIONS_H
+#define LLVM_LIBC_SRC___SUPPORT_CPP_TYPE_TRAITS_HAS_UNIQUE_OBJECT_REPRESENTATIONS_H
+
+#include "src/__support/CPP/type_traits/integral_constant.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+namespace cpp {
+
+template <class T>
+struct has_unique_object_representations
+ : public integral_constant<bool, __has_unique_object_representations(T)> {};
+
+template <class T>
+LIBC_INLINE_VAR constexpr bool has_unique_object_representations_v =
+ has_unique_object_representations<T>::value;
+
+} // namespace cpp
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC___SUPPORT_CPP_TYPE_TRAITS_HAS_UNIQUE_OBJECT_REPRESENTATIONS_H
>From 374a6a58d19d419316343d283cf1b4946f10295b Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <i at zhuyi.fan>
Date: Thu, 12 Dec 2024 09:14:04 -0800
Subject: [PATCH 2/3] adjust format
---
libc/src/__support/CPP/type_traits.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/libc/src/__support/CPP/type_traits.h b/libc/src/__support/CPP/type_traits.h
index 2b3914521cbe98..910cebbb8d0591 100644
--- a/libc/src/__support/CPP/type_traits.h
+++ b/libc/src/__support/CPP/type_traits.h
@@ -66,5 +66,4 @@
#include "src/__support/CPP/type_traits/type_identity.h"
#include "src/__support/CPP/type_traits/void_t.h"
-
#endif // LLVM_LIBC_SRC___SUPPORT_CPP_TYPE_TRAITS_H
>From ef900d747c5b1a60d8e473a80bfe8388c2f240bf Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Thu, 12 Dec 2024 12:20:34 -0500
Subject: [PATCH 3/3] Update libc/src/__support/CPP/atomic.h
Co-authored-by: Nick Desaulniers <nickdesaulniers at users.noreply.github.com>
---
libc/src/__support/CPP/atomic.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/libc/src/__support/CPP/atomic.h b/libc/src/__support/CPP/atomic.h
index dfe1dc9591e12b..287dcac98fbb66 100644
--- a/libc/src/__support/CPP/atomic.h
+++ b/libc/src/__support/CPP/atomic.h
@@ -64,9 +64,8 @@ template <typename T> struct Atomic {
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
+ // 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.
LIBC_INLINE_VAR static constexpr size_t MIN_ALIGNMENT =
(sizeof(T) & (sizeof(T) - 1)) || (sizeof(T) > 16) ? 0 : sizeof(T);
More information about the libc-commits
mailing list