[libcxx-commits] [libcxx] [libc++] Increase `atomic_ref`'s required alignment for small types (PR #99654)
Damien L-G via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Aug 1 04:57:33 PDT 2024
https://github.com/dalg24 updated https://github.com/llvm/llvm-project/pull/99654
>From 8d1fc929828cbc5a5d9e88180f928b545a703f91 Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Fri, 19 Jul 2024 10:09:17 -0400
Subject: [PATCH 1/6] [libc++] Increase atomic_ref's required alignment for
small types
---
libcxx/include/__atomic/atomic_ref.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libcxx/include/__atomic/atomic_ref.h b/libcxx/include/__atomic/atomic_ref.h
index 156f1961151c1..49f6982a27f1f 100644
--- a/libcxx/include/__atomic/atomic_ref.h
+++ b/libcxx/include/__atomic/atomic_ref.h
@@ -95,10 +95,14 @@ struct __atomic_ref_base {
friend struct __atomic_waitable_traits<__atomic_ref_base<_Tp>>;
+ // require types that are 1, 2, 4, 8, or 16 bytes in length to be aligned to at least their size to allow them to be
+ // used lock-free
+ static constexpr bool __min_alignement = (sizeof(_Tp) & sizeof(_Tp - 1)) || (sizeof(_Tp) > 16) ? 0 : sizeof(_Tp);
+
public:
using value_type = _Tp;
- static constexpr size_t required_alignment = alignof(_Tp);
+ static constexpr size_t required_alignment = alignof(_Tp) > __min_alignement ? alignof(_Tp) : __min_alignement;
// The __atomic_always_lock_free builtin takes into account the alignment of the pointer if provided,
// so we create a fake pointer with a suitable alignment when querying it. Note that we are guaranteed
>From 8545aeb682c31c7eb43e2eb3ba6f55c84147c4f6 Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Fri, 19 Jul 2024 10:26:05 -0400
Subject: [PATCH 2/6] fixup! [libc++] Increase atomic_ref's required alignment
for small types
---
libcxx/include/__atomic/atomic_ref.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/include/__atomic/atomic_ref.h b/libcxx/include/__atomic/atomic_ref.h
index 49f6982a27f1f..99ec8add82c39 100644
--- a/libcxx/include/__atomic/atomic_ref.h
+++ b/libcxx/include/__atomic/atomic_ref.h
@@ -97,7 +97,7 @@ struct __atomic_ref_base {
// require types that are 1, 2, 4, 8, or 16 bytes in length to be aligned to at least their size to allow them to be
// used lock-free
- static constexpr bool __min_alignement = (sizeof(_Tp) & sizeof(_Tp - 1)) || (sizeof(_Tp) > 16) ? 0 : sizeof(_Tp);
+ static constexpr bool __min_alignement = (sizeof(_Tp) & (sizeof(_Tp) - 1)) || (sizeof(_Tp) > 16) ? 0 : sizeof(_Tp);
public:
using value_type = _Tp;
>From 6027a625423e3af4472e8181f02bdaf022364d20 Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Fri, 19 Jul 2024 10:41:47 -0400
Subject: [PATCH 3/6] Fix typo and follow the suggestion from the review
---
libcxx/include/__atomic/atomic_ref.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libcxx/include/__atomic/atomic_ref.h b/libcxx/include/__atomic/atomic_ref.h
index 99ec8add82c39..7093895506d24 100644
--- a/libcxx/include/__atomic/atomic_ref.h
+++ b/libcxx/include/__atomic/atomic_ref.h
@@ -95,14 +95,14 @@ struct __atomic_ref_base {
friend struct __atomic_waitable_traits<__atomic_ref_base<_Tp>>;
- // require types that are 1, 2, 4, 8, or 16 bytes in length to be aligned to at least their size to allow them to be
+ // 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
- static constexpr bool __min_alignement = (sizeof(_Tp) & (sizeof(_Tp) - 1)) || (sizeof(_Tp) > 16) ? 0 : sizeof(_Tp);
+ static constexpr bool __min_alignment = (sizeof(_Tp) & (sizeof(_Tp) - 1)) || (sizeof(_Tp) > 16) ? 0 : sizeof(_Tp);
public:
using value_type = _Tp;
- static constexpr size_t required_alignment = alignof(_Tp) > __min_alignement ? alignof(_Tp) : __min_alignement;
+ static constexpr size_t required_alignment = alignof(_Tp) > __min_alignment ? alignof(_Tp) : __min_alignment;
// The __atomic_always_lock_free builtin takes into account the alignment of the pointer if provided,
// so we create a fake pointer with a suitable alignment when querying it. Note that we are guaranteed
>From 056d4afb074e469aaf9c2411b3c32836159bf0a9 Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24+github at gmail.com>
Date: Tue, 23 Jul 2024 10:31:43 -0500
Subject: [PATCH 4/6] Fixup bool -> size_t
Co-authored-by: Louis Dionne <ldionne.2 at gmail.com>
---
libcxx/include/__atomic/atomic_ref.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/include/__atomic/atomic_ref.h b/libcxx/include/__atomic/atomic_ref.h
index 7093895506d24..d34713aa26530 100644
--- a/libcxx/include/__atomic/atomic_ref.h
+++ b/libcxx/include/__atomic/atomic_ref.h
@@ -97,7 +97,7 @@ struct __atomic_ref_base {
// 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
- static constexpr bool __min_alignment = (sizeof(_Tp) & (sizeof(_Tp) - 1)) || (sizeof(_Tp) > 16) ? 0 : sizeof(_Tp);
+ static constexpr size_t __min_alignment = (sizeof(_Tp) & (sizeof(_Tp) - 1)) || (sizeof(_Tp) > 16) ? 0 : sizeof(_Tp);
public:
using value_type = _Tp;
>From edeef46f44950cffa4a221c14016662578bac005 Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Fri, 26 Jul 2024 14:29:36 -0400
Subject: [PATCH 5/6] Follow James' suggestion and add aligned attribute to the
pointer data member
---
libcxx/include/__atomic/atomic_ref.h | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/libcxx/include/__atomic/atomic_ref.h b/libcxx/include/__atomic/atomic_ref.h
index db93a189d883f..b0180a37ab500 100644
--- a/libcxx/include/__atomic/atomic_ref.h
+++ b/libcxx/include/__atomic/atomic_ref.h
@@ -57,11 +57,6 @@ struct __get_aligner_instance {
template <class _Tp>
struct __atomic_ref_base {
-protected:
- _Tp* __ptr_;
-
- _LIBCPP_HIDE_FROM_ABI __atomic_ref_base(_Tp& __obj) : __ptr_(std::addressof(__obj)) {}
-
private:
_LIBCPP_HIDE_FROM_ABI static _Tp* __clear_padding(_Tp& __val) noexcept {
_Tp* __ptr = std::addressof(__val);
@@ -222,6 +217,12 @@ struct __atomic_ref_base {
}
_LIBCPP_HIDE_FROM_ABI void notify_one() const noexcept { std::__atomic_notify_one(*this); }
_LIBCPP_HIDE_FROM_ABI void notify_all() const noexcept { std::__atomic_notify_all(*this); }
+
+protected:
+ typedef _Tp _Aligned_Tp __attribute__((aligned(required_alignment)));
+ _Aligned_Tp* __ptr_;
+
+ _LIBCPP_HIDE_FROM_ABI __atomic_ref_base(_Tp& __obj) : __ptr_(std::addressof(__obj)) {}
};
template <class _Tp>
>From 976dbd9af7d73cf0d205e299c5c14654656437ea Mon Sep 17 00:00:00 2001
From: Damien L-G <dalg24 at gmail.com>
Date: Thu, 1 Aug 2024 07:57:13 -0400
Subject: [PATCH 6/6] Use alignas to make objects satisfy atomic_ref's
alignement requirement
---
.../test/std/atomics/atomics.ref/is_always_lock_free.pass.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/test/std/atomics/atomics.ref/is_always_lock_free.pass.cpp b/libcxx/test/std/atomics/atomics.ref/is_always_lock_free.pass.cpp
index acdbf63a24d85..78e46c0397951 100644
--- a/libcxx/test/std/atomics/atomics.ref/is_always_lock_free.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.ref/is_always_lock_free.pass.cpp
@@ -54,7 +54,7 @@ void check_always_lock_free(std::atomic_ref<T> const& a) {
#define CHECK_ALWAYS_LOCK_FREE(T) \
do { \
typedef T type; \
- type obj{}; \
+ alignas(std::atomic_ref<type>::required_alignment) type obj{}; \
std::atomic_ref<type> a(obj); \
check_always_lock_free(a); \
} while (0)
More information about the libcxx-commits
mailing list