[libc++] P0718R2: Implementation of std::atomic<shared_ptr<T>> and std::atomic<weak_ptr<T>> - #194215
[libc++] P0718R2: Implementation of std::atomic<shared_ptr<T>> and std::atomic<weak_ptr<T>>#194215ViNN280801 wants to merge 47 commits into
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Vladislav Semykin (ViNN280801) ChangesMotivationPrior to this patch, Related Links
TestingAll the tests are passed: Additional informationThis is not a lock-free implementation, but I'm still on thoughts how to properly implement it, and I need more time to investigate existing imlpementations and think about the best way to do it. Patch is 46.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/194215.diff 12 Files Affected:
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index 69a6590d18f85..d5402b6fc40b4 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -216,6 +216,7 @@ set(files
__atomic/atomic_lock_free.h
__atomic/atomic_ref.h
__atomic/atomic_sync.h
+ __atomic/atomic_sync_lite.h
__atomic/atomic_sync_timed.h
__atomic/atomic_waitable_traits.h
__atomic/check_memory_order.h
@@ -590,6 +591,7 @@ set(files
__memory/allocator_traits.h
__memory/array_cookie.h
__memory/assume_aligned.h
+ __memory/atomic_shared_ptr.h
__memory/auto_ptr.h
__memory/compressed_pair.h
__memory/concepts.h
diff --git a/libcxx/include/__atomic/atomic_sync_lite.h b/libcxx/include/__atomic/atomic_sync_lite.h
new file mode 100644
index 0000000000000..a0433e16b506a
--- /dev/null
+++ b/libcxx/include/__atomic/atomic_sync_lite.h
@@ -0,0 +1,45 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 _LIBCPP___ATOMIC_ATOMIC_SYNC_LITE_H
+#define _LIBCPP___ATOMIC_ATOMIC_SYNC_LITE_H
+
+#include <__atomic/contention_t.h>
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+# pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if _LIBCPP_STD_VER >= 20 && _LIBCPP_HAS_THREADS
+
+# if !_LIBCPP_AVAILABILITY_HAS_NEW_SYNC
+// Old dylib interface kept for backwards compatibility.
+_LIBCPP_EXPORTED_FROM_ABI void __cxx_atomic_notify_one(void const volatile*) _NOEXCEPT;
+_LIBCPP_EXPORTED_FROM_ABI void __cxx_atomic_notify_all(void const volatile*) _NOEXCEPT;
+_LIBCPP_EXPORTED_FROM_ABI __cxx_contention_t __libcpp_atomic_monitor(void const volatile*) _NOEXCEPT;
+_LIBCPP_EXPORTED_FROM_ABI void __libcpp_atomic_wait(void const volatile*, __cxx_contention_t) _NOEXCEPT;
+# endif // !_LIBCPP_AVAILABILITY_HAS_NEW_SYNC
+
+// New dylib interface.
+_LIBCPP_AVAILABILITY_NEW_SYNC _LIBCPP_EXPORTED_FROM_ABI __cxx_contention_t
+__atomic_monitor_global(void const* __address) _NOEXCEPT;
+
+_LIBCPP_AVAILABILITY_NEW_SYNC _LIBCPP_EXPORTED_FROM_ABI void
+__atomic_wait_global_table(void const* __address, __cxx_contention_t __monitor_value) _NOEXCEPT;
+
+_LIBCPP_AVAILABILITY_NEW_SYNC _LIBCPP_EXPORTED_FROM_ABI void __atomic_notify_one_global_table(void const*) _NOEXCEPT;
+_LIBCPP_AVAILABILITY_NEW_SYNC _LIBCPP_EXPORTED_FROM_ABI void __atomic_notify_all_global_table(void const*) _NOEXCEPT;
+
+#endif // _LIBCPP_STD_VER >= 20 && _LIBCPP_HAS_THREADS
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___ATOMIC_ATOMIC_SYNC_LITE_H
diff --git a/libcxx/include/__memory/atomic_shared_ptr.h b/libcxx/include/__memory/atomic_shared_ptr.h
new file mode 100644
index 0000000000000..000b56e08bae6
--- /dev/null
+++ b/libcxx/include/__memory/atomic_shared_ptr.h
@@ -0,0 +1,491 @@
+// -*- 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 _LIBCPP___MEMORY_ATOMIC_SHARED_PTR_H
+#define _LIBCPP___MEMORY_ATOMIC_SHARED_PTR_H
+
+#include <__atomic/atomic_sync_lite.h>
+#include <__atomic/check_memory_order.h>
+#include <__atomic/memory_order.h>
+#include <__atomic/support.h>
+#include <__config>
+#include <__cstddef/nullptr_t.h>
+#include <__memory/shared_count.h>
+#include <__utility/move.h>
+
+#include <cstdint>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+# pragma GCC system_header
+#endif
+
+#if defined(__SANITIZE_THREAD__) || (defined(__has_feature) && __has_feature(thread_sanitizer))
+# if __has_include(<sanitizer/tsan_interface.h>)
+# include <sanitizer/tsan_interface.h>
+# define _LIBCPP_ATOMIC_SHARED_PTR_TSAN 1
+# endif
+#endif
+
+_LIBCPP_PUSH_MACROS
+#include <__undef_macros>
+
+// TSAN annotations model the lock-bit protocol on __ctrl_.
+#if defined(_LIBCPP_ATOMIC_SHARED_PTR_TSAN)
+# define _LIBCPP_ATOMIC_SP_TSAN_PRE_LOCK(addr) \
+ ::__tsan_mutex_pre_lock(reinterpret_cast<void*>(const_cast<__cxx_atomic_impl<uintptr_t>*>(addr)), 0)
+# define _LIBCPP_ATOMIC_SP_TSAN_POST_LOCK(addr) \
+ ::__tsan_mutex_post_lock(reinterpret_cast<void*>(const_cast<__cxx_atomic_impl<uintptr_t>*>(addr)), 0, 0)
+# define _LIBCPP_ATOMIC_SP_TSAN_PRE_UNLOCK(addr) \
+ ::__tsan_mutex_pre_unlock(reinterpret_cast<void*>(const_cast<__cxx_atomic_impl<uintptr_t>*>(addr)), 0)
+# define _LIBCPP_ATOMIC_SP_TSAN_POST_UNLOCK(addr) \
+ ::__tsan_mutex_post_unlock(reinterpret_cast<void*>(const_cast<__cxx_atomic_impl<uintptr_t>*>(addr)), 0)
+#else
+# define _LIBCPP_ATOMIC_SP_TSAN_PRE_LOCK(addr) ((void)(addr))
+# define _LIBCPP_ATOMIC_SP_TSAN_POST_LOCK(addr) ((void)(addr))
+# define _LIBCPP_ATOMIC_SP_TSAN_PRE_UNLOCK(addr) ((void)(addr))
+# define _LIBCPP_ATOMIC_SP_TSAN_POST_UNLOCK(addr) ((void)(addr))
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if _LIBCPP_STD_VER >= 20 && _LIBCPP_HAS_THREADS && _LIBCPP_HAS_ATOMIC_HEADER
+
+template <class _Tp>
+class shared_ptr;
+
+template <class _Tp>
+class weak_ptr;
+
+template <class _Tp>
+struct atomic;
+
+// Split state into pointer word and control word.
+// The control word stores control-block pointer plus lock/notify bits.
+struct __atomic_smart_ptr_storage {
+ static constexpr uintptr_t __lock_bit_ = uintptr_t{1};
+ static constexpr uintptr_t __notify_bit_ = uintptr_t{2};
+ static constexpr uintptr_t __ptr_mask_ = ~(__lock_bit_ | __notify_bit_);
+
+ _LIBCPP_HIDE_FROM_ABI static uintptr_t __encode(__shared_weak_count* __ctrl, uintptr_t __bits) _NOEXCEPT {
+ return (reinterpret_cast<uintptr_t>(__ctrl) & __ptr_mask_) | (__bits & ~__ptr_mask_);
+ }
+
+ _LIBCPP_HIDE_FROM_ABI static __shared_weak_count* __decode(uintptr_t __word) _NOEXCEPT {
+ return reinterpret_cast<__shared_weak_count*>(__word & __ptr_mask_);
+ }
+
+ _LIBCPP_HIDE_FROM_ABI static bool __has_lock(uintptr_t __word) _NOEXCEPT { return (__word & __lock_bit_) != 0; }
+ _LIBCPP_HIDE_FROM_ABI static bool __has_notify(uintptr_t __word) _NOEXCEPT { return (__word & __notify_bit_) != 0; }
+};
+
+_LIBCPP_HIDE_FROM_ABI inline void __atomic_smart_ptr_notify_one(const void* __address) _NOEXCEPT {
+# if _LIBCPP_AVAILABILITY_HAS_NEW_SYNC
+ std::__atomic_notify_one_global_table(__address);
+# else
+ std::__cxx_atomic_notify_one(reinterpret_cast<void const volatile*>(__address));
+# endif
+}
+
+_LIBCPP_HIDE_FROM_ABI inline void __atomic_smart_ptr_notify_all(const void* __address) _NOEXCEPT {
+# if _LIBCPP_AVAILABILITY_HAS_NEW_SYNC
+ std::__atomic_notify_all_global_table(__address);
+# else
+ std::__cxx_atomic_notify_all(reinterpret_cast<void const volatile*>(__address));
+# endif
+}
+
+template <class _Poll>
+_LIBCPP_HIDE_FROM_ABI inline void __atomic_smart_ptr_wait_on_address(const void* __address, _Poll&& __poll) _NOEXCEPT {
+ while (!__poll()) {
+# if _LIBCPP_AVAILABILITY_HAS_NEW_SYNC
+ auto __monitor_value = std::__atomic_monitor_global(__address);
+ if (__poll())
+ return;
+ std::__atomic_wait_global_table(__address, __monitor_value);
+# else
+ void const volatile* __volatile_address = reinterpret_cast<void const volatile*>(__address);
+ auto __monitor_value = std::__libcpp_atomic_monitor(__volatile_address);
+ if (__poll())
+ return;
+ std::__libcpp_atomic_wait(__volatile_address, __monitor_value);
+# endif
+ }
+}
+
+template <class _Element>
+struct __atomic_smart_ptr_fields {
+ mutable __cxx_atomic_impl<_Element*> __ptr_;
+ mutable __cxx_atomic_impl<uintptr_t> __ctrl_;
+
+ _LIBCPP_HIDE_FROM_ABI __atomic_smart_ptr_fields(_Element* __p, __shared_weak_count* __c) _NOEXCEPT
+ : __ptr_(__p),
+ __ctrl_(__atomic_smart_ptr_storage::__encode(__c, 0)) {}
+
+ _LIBCPP_HIDE_FROM_ABI const void* __ctrl_address() const _NOEXCEPT {
+ return static_cast<const void*>(__builtin_addressof(__ctrl_));
+ }
+
+ // Acquire lock bit on __ctrl_. Contended path sets notify bit and waits.
+ _LIBCPP_HIDE_FROM_ABI void __lock() const _NOEXCEPT {
+ _LIBCPP_ATOMIC_SP_TSAN_PRE_LOCK(&__ctrl_);
+ uintptr_t __expected = std::__cxx_atomic_load(__builtin_addressof(__ctrl_), memory_order_relaxed);
+ for (;;) {
+ if (!__atomic_smart_ptr_storage::__has_lock(__expected)) {
+ uintptr_t __desired = __expected | __atomic_smart_ptr_storage::__lock_bit_;
+ if (std::__cxx_atomic_compare_exchange_weak(
+ __builtin_addressof(__ctrl_),
+ __builtin_addressof(__expected),
+ __desired,
+ memory_order_acquire,
+ memory_order_relaxed)) {
+ _LIBCPP_ATOMIC_SP_TSAN_POST_LOCK(&__ctrl_);
+ return;
+ }
+ continue;
+ }
+
+ uintptr_t __with_notify = __expected | __atomic_smart_ptr_storage::__notify_bit_;
+ if (!__atomic_smart_ptr_storage::__has_notify(__expected)) {
+ if (!std::__cxx_atomic_compare_exchange_weak(
+ __builtin_addressof(__ctrl_),
+ __builtin_addressof(__expected),
+ __with_notify,
+ memory_order_relaxed,
+ memory_order_relaxed))
+ continue;
+ __expected = __with_notify;
+ }
+
+ std::__atomic_smart_ptr_wait_on_address(__ctrl_address(), [&] {
+ __expected = std::__cxx_atomic_load(__builtin_addressof(__ctrl_), memory_order_relaxed);
+ return !__atomic_smart_ptr_storage::__has_lock(__expected);
+ });
+ }
+ }
+
+ // Publish new control pointer, clear bits, and notify waiters if needed.
+ _LIBCPP_HIDE_FROM_ABI void __unlock(__shared_weak_count* __ctrl_to_publish) const _NOEXCEPT {
+ _LIBCPP_ATOMIC_SP_TSAN_PRE_UNLOCK(&__ctrl_);
+ uintptr_t __new_word = __atomic_smart_ptr_storage::__encode(__ctrl_to_publish, 0);
+ uintptr_t __previous = std::__cxx_atomic_exchange(__builtin_addressof(__ctrl_), __new_word, memory_order_release);
+ if (__atomic_smart_ptr_storage::__has_notify(__previous))
+ std::__atomic_smart_ptr_notify_all(__ctrl_address());
+ _LIBCPP_ATOMIC_SP_TSAN_POST_UNLOCK(&__ctrl_);
+ }
+};
+
+// [util.smartptr.atomic.shared]: same stored pointer and same ownership, or both empty.
+template <class _Element>
+_LIBCPP_HIDE_FROM_ABI inline bool __atomic_smart_ptr_equivalent(
+ _Element* __ptr,
+ __shared_weak_count* __ctrl,
+ _Element* __expected_ptr,
+ __shared_weak_count* __expected_ctrl) _NOEXCEPT {
+ if (__ctrl == nullptr && __expected_ctrl == nullptr)
+ return true;
+ return __ptr == __expected_ptr && __ctrl == __expected_ctrl;
+}
+
+template <class _Tp>
+struct atomic<shared_ptr<_Tp>> {
+ using value_type = shared_ptr<_Tp>;
+
+ static constexpr bool is_always_lock_free = false;
+
+ _LIBCPP_HIDE_FROM_ABI atomic() _NOEXCEPT : __fields_(nullptr, nullptr) {}
+ _LIBCPP_HIDE_FROM_ABI constexpr atomic(nullptr_t) _NOEXCEPT : __fields_(nullptr, nullptr) {}
+ _LIBCPP_HIDE_FROM_ABI atomic(shared_ptr<_Tp> __desired) _NOEXCEPT : __fields_(__desired.__ptr_, __desired.__cntrl_) {
+ __desired.__ptr_ = nullptr;
+ __desired.__cntrl_ = nullptr;
+ }
+
+ atomic(const atomic&) = delete;
+ atomic& operator=(const atomic&) = delete;
+
+ _LIBCPP_HIDE_FROM_ABI ~atomic() {
+ if (auto* __c = __atomic_smart_ptr_storage::__decode(
+ std::__cxx_atomic_load(__builtin_addressof(__fields_.__ctrl_), memory_order_relaxed)))
+ __c->__release_shared();
+ }
+
+ _LIBCPP_HIDE_FROM_ABI bool is_lock_free() const _NOEXCEPT { return false; }
+
+ _LIBCPP_HIDE_FROM_ABI void operator=(shared_ptr<_Tp> __desired) _NOEXCEPT { store(std::move(__desired)); }
+ _LIBCPP_HIDE_FROM_ABI void operator=(nullptr_t) _NOEXCEPT { store(nullptr); }
+ _LIBCPP_HIDE_FROM_ABI operator shared_ptr<_Tp>() const _NOEXCEPT { return load(); }
+
+ _LIBCPP_HIDE_FROM_ABI void store(shared_ptr<_Tp> __desired, memory_order __m = memory_order_seq_cst) _NOEXCEPT
+ _LIBCPP_CHECK_STORE_MEMORY_ORDER(__m) {
+ (void)__m;
+ _Tp* __desired_ptr = __desired.__ptr_;
+ __shared_weak_count* __desired_c = __desired.__cntrl_;
+ __desired.__ptr_ = nullptr;
+ __desired.__cntrl_ = nullptr;
+
+ __fields_.__lock();
+ __shared_weak_count* __old_c = __atomic_smart_ptr_storage::__decode(
+ std::__cxx_atomic_load(__builtin_addressof(__fields_.__ctrl_), memory_order_relaxed));
+ std::__cxx_atomic_store(__builtin_addressof(__fields_.__ptr_), __desired_ptr, memory_order_relaxed);
+ __fields_.__unlock(__desired_c);
+
+ if (__old_c)
+ __old_c->__release_shared();
+ }
+
+ _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp> load(memory_order __m = memory_order_seq_cst) const _NOEXCEPT
+ _LIBCPP_CHECK_LOAD_MEMORY_ORDER(__m) {
+ (void)__m;
+ __fields_.__lock();
+ _Tp* __ptr = std::__cxx_atomic_load(__builtin_addressof(__fields_.__ptr_), memory_order_relaxed);
+ __shared_weak_count* __c = __atomic_smart_ptr_storage::__decode(
+ std::__cxx_atomic_load(__builtin_addressof(__fields_.__ctrl_), memory_order_relaxed));
+ if (__c)
+ __c->__add_shared();
+ __fields_.__unlock(__c);
+ return shared_ptr<_Tp>::__create_with_control_block(__ptr, __c);
+ }
+
+ _LIBCPP_HIDE_FROM_ABI shared_ptr<_Tp>
+ exchange(shared_ptr<_Tp> __desired, memory_order __m = memory_order_seq_cst) _NOEXCEPT {
+ (void)__m;
+ _Tp* __desired_ptr = __desired.__ptr_;
+ __shared_weak_count* __desired_c = __desired.__cntrl_;
+ __desired.__ptr_ = nullptr;
+ __desired.__cntrl_ = nullptr;
+
+ __fields_.__lock();
+ _Tp* __old_ptr = std::__cxx_atomic_load(__builtin_addressof(__fields_.__ptr_), memory_order_relaxed);
+ __shared_weak_count* __old_c = __atomic_smart_ptr_storage::__decode(
+ std::__cxx_atomic_load(__builtin_addressof(__fields_.__ctrl_), memory_order_relaxed));
+ std::__cxx_atomic_store(__builtin_addressof(__fields_.__ptr_), __desired_ptr, memory_order_relaxed);
+ __fields_.__unlock(__desired_c);
+
+ return shared_ptr<_Tp>::__create_with_control_block(__old_ptr, __old_c);
+ }
+
+ _LIBCPP_HIDE_FROM_ABI bool compare_exchange_strong(
+ shared_ptr<_Tp>& __expected, shared_ptr<_Tp> __desired, memory_order __success, memory_order __failure) _NOEXCEPT
+ _LIBCPP_CHECK_EXCHANGE_MEMORY_ORDER(__success, __failure) {
+ (void)__success;
+ (void)__failure;
+ __fields_.__lock();
+ _Tp* __cur_ptr = std::__cxx_atomic_load(__builtin_addressof(__fields_.__ptr_), memory_order_relaxed);
+ __shared_weak_count* __cur_c = __atomic_smart_ptr_storage::__decode(
+ std::__cxx_atomic_load(__builtin_addressof(__fields_.__ctrl_), memory_order_relaxed));
+
+ if (__atomic_smart_ptr_equivalent(__cur_ptr, __cur_c, __expected.__ptr_, __expected.__cntrl_)) {
+ _Tp* __desired_ptr = __desired.__ptr_;
+ __shared_weak_count* __desired_c = __desired.__cntrl_;
+ __desired.__ptr_ = nullptr;
+ __desired.__cntrl_ = nullptr;
+
+ std::__cxx_atomic_store(__builtin_addressof(__fields_.__ptr_), __desired_ptr, memory_order_relaxed);
+ __fields_.__unlock(__desired_c);
+ if (__cur_c)
+ __cur_c->__release_shared();
+ return true;
+ }
+
+ if (__cur_c)
+ __cur_c->__add_shared();
+ __fields_.__unlock(__cur_c);
+ __expected = shared_ptr<_Tp>::__create_with_control_block(__cur_ptr, __cur_c);
+ return false;
+ }
+
+ _LIBCPP_HIDE_FROM_ABI bool compare_exchange_strong(
+ shared_ptr<_Tp>& __expected, shared_ptr<_Tp> __desired, memory_order __m = memory_order_seq_cst) _NOEXCEPT {
+ return compare_exchange_strong(__expected, std::move(__desired), __m, std::__to_failure_order(__m));
+ }
+
+ _LIBCPP_HIDE_FROM_ABI bool compare_exchange_weak(
+ shared_ptr<_Tp>& __expected, shared_ptr<_Tp> __desired, memory_order __success, memory_order __failure) _NOEXCEPT
+ _LIBCPP_CHECK_EXCHANGE_MEMORY_ORDER(__success, __failure) {
+ return compare_exchange_strong(__expected, std::move(__desired), __success, __failure);
+ }
+
+ _LIBCPP_HIDE_FROM_ABI bool compare_exchange_weak(
+ shared_ptr<_Tp>& __expected, shared_ptr<_Tp> __desired, memory_order __m = memory_order_seq_cst) _NOEXCEPT {
+ return compare_exchange_strong(__expected, std::move(__desired), __m, std::__to_failure_order(__m));
+ }
+
+ // Wait until the stored value is not equivalent to __old.
+ // __ctrl_ is the wait address; pointer changes are published with control updates.
+ _LIBCPP_HIDE_FROM_ABI void wait(shared_ptr<_Tp> __old, memory_order __m = memory_order_seq_cst) const _NOEXCEPT
+ _LIBCPP_CHECK_WAIT_MEMORY_ORDER(__m) {
+ _Tp* __old_ptr = __old.__ptr_;
+ __shared_weak_count* __old_c = __old.__cntrl_;
+
+ std::__atomic_smart_ptr_wait_on_address(__fields_.__ctrl_address(), [&] {
+ uintptr_t __word = std::__cxx_atomic_load(__builtin_addressof(__fields_.__ctrl_), __m);
+ __shared_weak_count* __cur_c = __atomic_smart_ptr_storage::__decode(__word);
+ if (__cur_c != __old_c)
+ return true;
+ _Tp* __cur_ptr = std::__cxx_atomic_load(__builtin_addressof(__fields_.__ptr_), __m);
+ return !__atomic_smart_ptr_equivalent(__cur_ptr, __cur_c, __old_ptr, __old_c);
+ });
+ }
+
+ _LIBCPP_HIDE_FROM_ABI void notify_one() _NOEXCEPT { std::__atomic_smart_ptr_notify_one(__fields_.__ctrl_address()); }
+ _LIBCPP_HIDE_FROM_ABI void notify_all() _NOEXCEPT { std::__atomic_smart_ptr_notify_all(__fields_.__ctrl_address()); }
+
+private:
+ __atomic_smart_ptr_fields<_Tp> __fields_;
+};
+
+template <class _Tp>
+struct atomic<weak_ptr<_Tp>> {
+ using value_type = weak_ptr<_Tp>;
+
+ static constexpr bool is_always_lock_free = false;
+
+ _LIBCPP_HIDE_FROM_ABI atomic() _NOEXCEPT : __fields_(nullptr, nullptr) {}
+ _LIBCPP_HIDE_FROM_ABI atomic(weak_ptr<_Tp> __desired) _NOEXCEPT : __fields_(__desired.__ptr_, __desired.__cntrl_) {
+ __desired.__ptr_ = nullptr;
+ __desired.__cntrl_ = nullptr;
+ }
+
+ atomic(const atomic&) = delete;
+ atomic& operator=(const atomic&) = delete;
+
+ _LIBCPP_HIDE_FROM_ABI ~atomic() {
+ if (auto* __c = __atomic_smart_ptr_storage::__decode(
+ std::__cxx_atomic_load(__builtin_addressof(__fields_.__ctrl_), memory_order_relaxed)))
+ __c->__release_weak();
+ }
+
+ _LIBCPP_HIDE_FROM_ABI bool is_lock_free() const _NOEXCEPT { return false; }
+
+ _LIBCPP_HIDE_FROM_ABI void operator=(weak_ptr<_Tp> __desired) _NOEXCEPT { store(std::move(__desired)); }
+ _LIBCPP_HIDE_FROM_ABI operator weak_ptr<_Tp>() const _NOEXCEPT { return load(); }
+
+ _LIBCPP_HIDE_FROM_ABI void store(weak_ptr<_Tp> __desired, memory_order __m = memory_order_seq_cst) _NOEXCEPT
+ _LIBCPP_CHECK_STORE_MEMORY_ORDER(__m) {
+ (void)__m;
+ _Tp* __desired_ptr = __desired.__ptr_;
+ __shared_weak_count* __desired_c = __desired.__cntrl_;
+ __desired.__ptr_ = nullptr;
+ __desired.__cntrl_ = nullptr;
+
+ __fields_.__lock();
+ __shared_weak_count* __old_c = __atomic_smart_ptr_storage::__decode(
+ std::__cxx_atomic_load(__builtin_addressof(__fields_.__ctrl_), memory_order_relaxed));
+ std::__cxx_atomic_store(__builtin_addressof(__fields_.__ptr_), __desired_ptr, memory_order_relaxed);
+ __fields_.__unlock(__desired_c);
+
+ if (__old_c)
+ __old_c->__release_weak();
+ }
+
+ _LIBCPP_HIDE_FROM_ABI weak_ptr<_Tp> load(memory_order __m = memory_order_seq_cst) const _NOEXCEPT
+ _LIBCPP_CHECK_LOAD_MEMORY_ORDER(__m) {
+ (void)__m;
+ __fields_.__lock();
+ _Tp* __ptr = std::__cxx_atomic_load(__builtin_addressof(__fields_.__ptr_), memory_order_relaxed);
+ __shared_weak_count* __c = __atomic_smart_ptr...
[truncated]
|
H-G-Hristov
left a comment
H-G-Hristov
left a comment
There was a problem hiding this comment.
Thank you for working on this. I had just a quick look and I have a few notes/questions:
Does this implement P0718R2 fully?
- Please make sure to update the status pages accordingly.
- Use GitHub syntax to associate all implemented papers/LWG issues with the corresponding GitHub issue: https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue, e.g.
Closes #99980. - I have submitted a related patch
https://github.com/llvm/llvm-project/pull/87111to remove the depracated atomic_shared_ptr API, which was reverted temporarily until P0718R2 was implemented. I'll reopen as I think it will be good to have both PRs merged closer together.
|
Thank you for the review! All suggestions have been applied:
P0718R2 coverage:
Limitations of this implementation: This is a lock-based implementation ( I'll also rebase on top of #87111 once it's reopened. Regression testing after changes: ninja -C build check-cxxAlso, I added necessary headers for Armv7 and Armv8 in tests, but how to correctly handle |
|
This time CI fails on AIX (32 and 64 bit), both with What is the recommended way to handle this in libc++ test suite? |
|
IIRC, libc++ hasn't implemented |
|
Is a lock-free implementation strictly required, or is a conforming lock-based implementation acceptable as a first step? The standard does not mandate lock-free - This PR is intended as Phase 1: correct, standard-conforming, lock-based. A lock-free follow-up can come once the prerequisite infrastructure is in place. |
I don’t see how we can have a follow up lock free PR without breaking the phase 1 ABI
even though it is not required, 1-2 years ago I’ve spoken to the author of the original paper https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4058.pdf and they suggested to implement as lock free as the motivations of this class is primary for implementing lock free data structures. Even though I agree that we haven’t found a portable way implementing it yet, but at least IMO we could try looking into ways to implement the lock free versions for some platforms. That is my 2c, and I am not blocking PRs doing otherwise |
|
Thank you for the context and the link to N4058. Regarding specific lock-free: I agree this is the right direction. On x86-64 with CMPXCHG16B and AArch64 with LSE, a DWCAS-based fast path can be implemented without portability issues. I am actively researching this. I'm happy to rework the PR in whatever direction the team prefers. |
|
Thank you for the clarifications! Done. |
I'd be very happy to see an implementation based on |
|
I think that bare |
|
Updated the PR message - see Related Links section |
H-G-Hristov
left a comment
H-G-Hristov
left a comment
There was a problem hiding this comment.
Since this PR should complete the paper, let's also add an entry to the Release Notes:
- P0718R2: Revising ``atomic_shared_ptr`` for C++20 (`Github <https://llvm.org/PR99980
>`__)IMO it would be good to mention the paper in the title of the PR too.
|
Thank you so much for your reviewing, @H-G-Hristov. |
|
FYI, we use the PR description as the squashed commit message by default. Do you really want to mention so many things in the commit message? |
Thanks for the clarification, I will reduce the information in PR message. For example will move it here, in comments. |
Related Links
Not so relevant for libc++ because require 48-bit VA (broken on LA57/ARM 52-bit), dependents from libstdc++ internals: MSVC analysis: Konstantin Vladimirov's lectures: TestingAll the tests are passed: Additional informationThis is not a lock-free implementation, but I'm still on thoughts how to properly implement it, and I need more time to investigate existing imlpementations and think about the best way to do it. |
|
@frederick-vs-ja, how does it look for now? is it good enough? |
ViNN280801
changed the title
ViNN280801
changed the title
It's OK to mention the related papers and chapters (with links) from the Standard. It's actually good to have them in the description. I have recreated: #194544 (P2869R4 + P0718R2's deprecations). Maybe mention that too (for the deprecations). I'm not sure if we want that as separate PRs and the order we want to commit them but we can figure that out when this PR is ready and approved. P.S. Sorry for accidentally editing the message above instead of replying. |
It's ok :) |
|
I haven't looked at the actual code at all, but one option would be to land this behind |
|
Hopefully AIX stays green, but if it goes red again and it's the only thing blocking the PR, feel free to mark the test unsupported on AIX. We haven't had time to look deeper into the contention issues yet. |
|
@jakeegan, ok, thank you! |
Zingam
left a comment
Zingam
left a comment
There was a problem hiding this comment.
@H-G-Hristov, @huixie90, @dalg24, almost all the CIs passed, 3 skipped, I think it is because Github erroneously marked "macos (apple-system-hardenedm, 26.5)" as failed. Can you review please, and do not forget about this:
I've marked two spots with
TODOthat I'd like extra attention on: strict aliasing in__dwcas_address()and a potential ABA issue in__load_dwcas.from #194215 (comment).
I would rather comment only on general libc++ practices and leave the implementation details to the expert.
Let's add a Release Note entry to to docs/ReleaseNotes/24.rst with a note that this impelents just the atomic_shared pointer. When we merge this: #194544 I'll update it to state we've completed the paper. I expect that we merge this patch first and then the other one, which was once merge and then reverted on request: #194544
Zingam
left a comment
Zingam
left a comment
There was a problem hiding this comment.
@H-G-Hristov, @huixie90, @dalg24, almost all the CIs passed, 3 skipped, I think it is because Github erroneously marked "macos (apple-system-hardenedm, 26.5)" as failed. Can you review please, and do not forget about this:
I've marked two spots with
TODOthat I'd like extra attention on: strict aliasing in__dwcas_address()and a potential ABA issue in__load_dwcas.from #194215 (comment).
I would rather comment only on general libc++ practices and leave the implementation details to the expert.
Let's add a Release Note entry to to docs/ReleaseNotes/24.rst with a note that this impelents just the atomic_shared pointer. When we merge this: #194544 I'll update it to state we've completed the paper. I expect that we merge this patch first and then the other one, which was once merge and then reverted on request: #194544
Okay, thank you very much for all the suggestions, how I can track the documentation changes as you, for example, I wouldn't even think about updating any .rst or .csv (or any docs) files, so, need I just to keep them in mind?
Sounds good, but I'm working on benchmarks request from RFC from MindSpectre: https://discourse.llvm.org/t/rfc-implementing-std-atomic-shared-ptr-t-and-std-atomic-weak-ptr-t-p0718r2-for-libc/91586 |
…hared.atomic/atomic_shared_ptr_memory_order.verify.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
…hared.atomic/atomic_shared_ptr_refcount.pass.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
…hared.atomic/atomic_shared_ptr_nullptr.pass.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
…free.pass.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
Co-authored-by: Hristo Hristov <zingam@outlook.com>
Co-authored-by: Hristo Hristov <zingam@outlook.com>
Co-authored-by: Hristo Hristov <zingam@outlook.com>
…onvert.pass.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
…ompare_exchange_strong.pass.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
…xchange.pass.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
…s_always_lock_free.pass.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
…oad.pass.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
…pare_exchange_strong.pass.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
…vert.pass.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
…hange.pass.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
…always_lock_free.pass.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
…lock_free.pass.cpp Co-authored-by: Hristo Hristov <zingam@outlook.com>
…mart_ptr_test_types.h Co-authored-by: Hristo Hristov <zingam@outlook.com>
…mart_ptr_test_types.h Co-authored-by: Hristo Hristov <zingam@outlook.com>
…mart_ptr_test_types.h Co-authored-by: Hristo Hristov <zingam@outlook.com>
|
Thank you so much for your thoughtfulness. You and the future reviewers clearly did more work than I did. |
You need to keep that in mind :)))) I guess! I have a few in-progress PRs that have been through several releases. After a release is branched, new release notes file is added, I synch the branch with main and make the appropriate adjustments. Quite often there a few more changes to be made than just release notes, docs, etc. BTW. Not synching a branch with main often enough might make it very hard to update if left untouched in a long time.
IMO. If this is a complete implementation of P0718 (which is going to be merged), regardless of planned future improvements, we should add a release note entry. |
|
@ViNN280801 BTW, If you are merging change proposals from GH. You can use Batch commit from the files tab. |
Yeah, I just recently encountered with it.
Didn't know about it, thanks! Will keep it in mind for future. |
10 participants
Motivation
std::atomic<std::shared_ptr<T>>andstd::atomic<std::weak_ptr<T>>are part of the C++ standard since C++20 (P2017R1) but were not yet implemented in libc++. libstdc++ (GCC 12, 2022) and MSVC STL (2020) have shipped conforming implementations.Closing Issues
Closes #99980
Closes #104962
Closes #105273