From 7979fb9fcd0bd295ebb69a58803e46373ec44dd7 Mon Sep 17 00:00:00 2001 From: Schrodinger ZHU Yifan Date: Thu, 12 Dec 2024 07:32:54 -0800 Subject: [PATCH 1/4] [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 a9fc7e61610cc..dfe1dc9591e12 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 struct Atomic { "constructible, move constructible, copy assignable, " "and move assignable."); + static_assert(cpp::has_unique_object_representations_v, + "atomic 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(mem_ord); @@ -62,6 +62,18 @@ template struct Atomic { return static_cast(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 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 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 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 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 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 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 struct Atomic { [[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) { static_assert(cpp::is_integral_v, "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 struct Atomic { [[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) { static_assert(cpp::is_integral_v, "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 struct Atomic { [[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) { static_assert(cpp::is_integral_v, "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 struct Atomic { [[maybe_unused]] MemoryScope mem_scope = MemoryScope::DEVICE) { static_assert(cpp::is_integral_v, "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 b9bc5b8568441..2b3914521cbe9 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 0000000000000..baed7da69d319 --- /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 +struct has_unique_object_representations + : public integral_constant {}; + +template +LIBC_INLINE_VAR constexpr bool has_unique_object_representations_v = + has_unique_object_representations::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 Date: Thu, 12 Dec 2024 09:14:04 -0800 Subject: [PATCH 2/4] 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 2b3914521cbe9..910cebbb8d059 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 Date: Thu, 12 Dec 2024 12:20:34 -0500 Subject: [PATCH 3/4] Update libc/src/__support/CPP/atomic.h Co-authored-by: Nick Desaulniers --- 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 dfe1dc9591e12..287dcac98fbb6 100644 --- a/libc/src/__support/CPP/atomic.h +++ b/libc/src/__support/CPP/atomic.h @@ -64,9 +64,8 @@ template 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); From 36bfc22b38e1584771c926c2edc7e267e601acea Mon Sep 17 00:00:00 2001 From: Schrodinger ZHU Yifan Date: Fri, 13 Dec 2024 10:16:29 -0500 Subject: [PATCH 4/4] fix --- .../has_unique_object_representations.h | 4 +++- .../src/__support/CPP/type_traits_test.cpp | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) 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 index baed7da69d319..639fb69d27203 100644 --- a/libc/src/__support/CPP/type_traits/has_unique_object_representations.h +++ b/libc/src/__support/CPP/type_traits/has_unique_object_representations.h @@ -9,6 +9,7 @@ #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/CPP/type_traits/remove_all_extents.h" #include "src/__support/macros/config.h" namespace LIBC_NAMESPACE_DECL { @@ -16,7 +17,8 @@ namespace cpp { template struct has_unique_object_representations - : public integral_constant {}; + : public integral_constant)> {}; template LIBC_INLINE_VAR constexpr bool has_unique_object_representations_v = diff --git a/libc/test/src/__support/CPP/type_traits_test.cpp b/libc/test/src/__support/CPP/type_traits_test.cpp index fa5298a12d3fc..4b3e48c6a6c0f 100644 --- a/libc/test/src/__support/CPP/type_traits_test.cpp +++ b/libc/test/src/__support/CPP/type_traits_test.cpp @@ -439,6 +439,28 @@ TEST(LlvmLibcTypeTraitsTest, is_object) { TEST(LlvmLibcTypeTraitsTest, true_type) { EXPECT_TRUE((true_type::value)); } +struct CompilerLeadingPadded { + char b; + int a; +}; + +struct CompilerTrailingPadded { + int a; + char b; +}; + +struct alignas(long long) ManuallyPadded { + int b; + char padding[sizeof(long long) - sizeof(int)]; +}; + +TEST(LlvmLibcTypeTraitsTest, has_unique_object_representations) { + EXPECT_TRUE(has_unique_object_representations::value); + EXPECT_FALSE(has_unique_object_representations_v); + EXPECT_FALSE(has_unique_object_representations_v); + EXPECT_TRUE(has_unique_object_representations_v); +} + // TODO type_identity // TODO void_t