[libc-commits] [libc] [libc][arc4random][patch 2/n] add ABA protected MPMCStack (PR #151361)
Schrodinger ZHU Yifan via libc-commits
libc-commits at lists.llvm.org
Fri Aug 8 07:25:41 PDT 2025
https://github.com/SchrodingerZhu updated https://github.com/llvm/llvm-project/pull/151361
>From 6c46017d9b0b5eb829adfd646f45efed2ba3367e Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Wed, 30 Jul 2025 12:47:24 -0400
Subject: [PATCH 1/6] wip
---
libc/src/__support/aba_ptr.h | 70 ++++++++++++++++++++++++++++++
libc/src/__support/mpmc_stack.h | 75 +++++++++++++++++++++++++++++++++
2 files changed, 145 insertions(+)
create mode 100644 libc/src/__support/aba_ptr.h
create mode 100644 libc/src/__support/mpmc_stack.h
diff --git a/libc/src/__support/aba_ptr.h b/libc/src/__support/aba_ptr.h
new file mode 100644
index 0000000000000..c702aae017502
--- /dev/null
+++ b/libc/src/__support/aba_ptr.h
@@ -0,0 +1,70 @@
+//===-- Transactional Ptr for ABA prevention --------------------*- 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_TAGGED_POINTER_H
+#define LLVM_LIBC_SRC___SUPPORT_TAGGED_POINTER_H
+
+#include "src/__support/common.h"
+#include "src/__support/threads/sleep.h"
+
+#ifdef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16
+#define LIBC_ABA_PTR_IS_ATOMIC true
+#else
+#define LIBC_ABA_PTR_IS_ATOMIC false
+#endif
+
+namespace LIBC_NAMESPACE_DECL {
+
+template <class T, bool IsAtomic> struct AbaPtrImpl {
+ union Impl {
+ struct alignas(2 * alignof(void *)) Atomic {
+ T *ptr;
+ __SIZE_TYPE__ tag;
+ } atomic;
+ struct Mutex {
+ T *ptr;
+ bool locked;
+ } mutex;
+ } impl;
+
+ LIBC_INLINE constexpr AbaPtrImpl(T *ptr)
+ : impl(IsAtomic ? Impl{.atomic{ptr, 0}} : Impl{.mutex{ptr, false}}) {}
+
+ /// User must guarantee that operation is redoable.
+ template <class Op> LIBC_INLINE void transaction(Op &&op) {
+ if constexpr (IsAtomic) {
+ for (;;) {
+ typename Impl::Atomic snapshot, next;
+ __atomic_load(&impl.atomic, &snapshot, __ATOMIC_RELAXED);
+ next.ptr = op(snapshot.ptr);
+ // Wrapping add for unsigned integers.
+ next.tag = snapshot.tag + 1;
+ if (__atomic_compare_exchange(&impl.atomic, &snapshot, &next, true,
+ __ATOMIC_ACQ_REL, __ATOMIC_RELAXED)) {
+ return;
+ }
+ }
+ } else {
+ // Acquire the lock.
+ while (__atomic_exchange_n(&impl.mutex.locked, true, __ATOMIC_ACQUIRE)) {
+ while (__atomic_load_n(&impl.mutex.locked, __ATOMIC_RELAXED)) {
+ LIBC_NAMESPACE::sleep_briefly();
+ }
+ }
+ impl.mutex.ptr = op(impl.mutex.ptr);
+ // Release the lock.
+ __atomic_store_n(&impl.mutex.locked, false, __ATOMIC_RELEASE);
+ }
+ }
+};
+
+template <class T> using AbaPtr = AbaPtrImpl<T, LIBC_ABA_PTR_IS_ATOMIC>;
+} // namespace LIBC_NAMESPACE_DECL
+
+#undef LIBC_ABA_PTR_IS_ATOMIC
+#endif
diff --git a/libc/src/__support/mpmc_stack.h b/libc/src/__support/mpmc_stack.h
new file mode 100644
index 0000000000000..4892c3926b4b0
--- /dev/null
+++ b/libc/src/__support/mpmc_stack.h
@@ -0,0 +1,75 @@
+//===-- Simple Lock-free MPMC Stack -----------------------------*- 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_MPMC_STACK_H
+#define LLVM_LIBC_SRC___SUPPORT_MPMC_STACK_H
+
+#include "src/__support/CPP/atomic.h"
+#include "src/__support/CPP/new.h"
+#include "src/__support/CPP/optional.h"
+#include "src/__support/aba_ptr.h"
+
+namespace LIBC_NAMESPACE_DECL {
+template <class T> class MPMCStack {
+ struct Node {
+ cpp::Atomic<size_t> visitor;
+ AbaPtr<Node> next;
+ T value;
+
+ LIBC_INLINE Node(T val) : visitor(0), next(nullptr), value(val) {}
+ };
+ AbaPtr<Node> head;
+
+public:
+ static_assert(cpp::is_copy_constructible<T>::value,
+ "T must be copy constructible");
+ LIBC_INLINE MPMCStack() : head(nullptr) {}
+ LIBC_INLINE bool push(T value) {
+ AllocChecker ac;
+ Node *new_node = new Node(value, ac);
+ if (!ac) {
+ return false;
+ }
+ head.transaction([new_node](Node *old_head) {
+ new_node->next = old_head;
+ return new_node;
+ });
+ return true;
+ }
+ LIBC_INLINE cpp::optional<T> pop() {
+ cpp::optional<T> res;
+ Node *node;
+ head.transaction([&](Node *current_head) {
+ if (!current_head) {
+ res = cpp::nullopt;
+ return nullptr;
+ }
+ node = current_head;
+ node->visitor.fetch_add(1);
+ res = node->value;
+ auto next = node->next;
+ node->visitor.fetch_sub(1);
+ return next;
+ });
+ // On a successful transaction, a node is popped by us. So we must delete
+ // it. When we are at here, no one else can acquire
+ // new reference to the node, but we still need to wait until other threads
+ // inside the transaction who may potentially be holding a reference to the
+ // node.
+ if (res) {
+ // Spin until the node is no longer in use.
+ while (node->visitor.load() != 0)
+ LIBC_NAMESPACE::sleep_briefly();
+ delete node;
+ }
+ return res;
+ }
+};
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif
>From 80b6c2445ec60853a7f081c42db53ca3e39c635a Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Wed, 30 Jul 2025 12:52:35 -0400
Subject: [PATCH 2/6] remove some extra braces
---
libc/src/__support/aba_ptr.h | 10 ++++------
libc/src/__support/mpmc_stack.h | 3 +--
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/libc/src/__support/aba_ptr.h b/libc/src/__support/aba_ptr.h
index c702aae017502..f7ed601dfbe70 100644
--- a/libc/src/__support/aba_ptr.h
+++ b/libc/src/__support/aba_ptr.h
@@ -45,17 +45,15 @@ template <class T, bool IsAtomic> struct AbaPtrImpl {
// Wrapping add for unsigned integers.
next.tag = snapshot.tag + 1;
if (__atomic_compare_exchange(&impl.atomic, &snapshot, &next, true,
- __ATOMIC_ACQ_REL, __ATOMIC_RELAXED)) {
+ __ATOMIC_ACQ_REL, __ATOMIC_RELAXED))
return;
- }
}
} else {
// Acquire the lock.
- while (__atomic_exchange_n(&impl.mutex.locked, true, __ATOMIC_ACQUIRE)) {
- while (__atomic_load_n(&impl.mutex.locked, __ATOMIC_RELAXED)) {
+ while (__atomic_exchange_n(&impl.mutex.locked, true, __ATOMIC_ACQUIRE))
+ while (__atomic_load_n(&impl.mutex.locked, __ATOMIC_RELAXED))
LIBC_NAMESPACE::sleep_briefly();
- }
- }
+
impl.mutex.ptr = op(impl.mutex.ptr);
// Release the lock.
__atomic_store_n(&impl.mutex.locked, false, __ATOMIC_RELEASE);
diff --git a/libc/src/__support/mpmc_stack.h b/libc/src/__support/mpmc_stack.h
index 4892c3926b4b0..819433f24380c 100644
--- a/libc/src/__support/mpmc_stack.h
+++ b/libc/src/__support/mpmc_stack.h
@@ -32,9 +32,8 @@ template <class T> class MPMCStack {
LIBC_INLINE bool push(T value) {
AllocChecker ac;
Node *new_node = new Node(value, ac);
- if (!ac) {
+ if (!ac)
return false;
- }
head.transaction([new_node](Node *old_head) {
new_node->next = old_head;
return new_node;
>From 79fdf8084e7bbb56d117d4a532453520d1d61fd4 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <i at zhuyi.fan>
Date: Wed, 30 Jul 2025 16:21:43 -0400
Subject: [PATCH 3/6] add tests
---
libc/src/__support/CMakeLists.txt | 24 ++++++++
libc/src/__support/aba_ptr.h | 17 +++++-
libc/src/__support/mpmc_stack.h | 15 ++---
.../integration/src/__support/CMakeLists.txt | 15 +++++
.../src/__support/mpmc_stack_test.cpp | 61 +++++++++++++++++++
5 files changed, 124 insertions(+), 8 deletions(-)
create mode 100644 libc/test/integration/src/__support/mpmc_stack_test.cpp
diff --git a/libc/src/__support/CMakeLists.txt b/libc/src/__support/CMakeLists.txt
index 2196d9e23bba7..c9d89cf6fc286 100644
--- a/libc/src/__support/CMakeLists.txt
+++ b/libc/src/__support/CMakeLists.txt
@@ -398,6 +398,30 @@ add_header_library(
libc.src.__support.macros.attributes
)
+add_header_library(
+ aba_ptr
+ HDRS
+ aba_ptr.h
+ DEPENDS
+ libc.hdr.types.size_t
+ libc.src.__support.common
+ libc.src.__support.threads.sleep
+)
+
+add_header_library(
+ mpmc_stack
+ HDRS
+ mpmc_stack.h
+ DEPENDS
+ libc.src.__support.aba_ptr
+ libc.src.__support.common
+ libc.src.__support.CPP.atomic
+ libc.src.__support.CPP.new
+ libc.src.__support.CPP.optional
+ libc.src.__support.CPP.type_traits
+)
+
+
add_subdirectory(FPUtil)
add_subdirectory(OSUtil)
add_subdirectory(StringUtil)
diff --git a/libc/src/__support/aba_ptr.h b/libc/src/__support/aba_ptr.h
index f7ed601dfbe70..632cc466c295b 100644
--- a/libc/src/__support/aba_ptr.h
+++ b/libc/src/__support/aba_ptr.h
@@ -9,6 +9,7 @@
#ifndef LLVM_LIBC_SRC___SUPPORT_TAGGED_POINTER_H
#define LLVM_LIBC_SRC___SUPPORT_TAGGED_POINTER_H
+#include "hdr/types/size_t.h"
#include "src/__support/common.h"
#include "src/__support/threads/sleep.h"
@@ -24,7 +25,7 @@ template <class T, bool IsAtomic> struct AbaPtrImpl {
union Impl {
struct alignas(2 * alignof(void *)) Atomic {
T *ptr;
- __SIZE_TYPE__ tag;
+ size_t tag;
} atomic;
struct Mutex {
T *ptr;
@@ -59,6 +60,20 @@ template <class T, bool IsAtomic> struct AbaPtrImpl {
__atomic_store_n(&impl.mutex.locked, false, __ATOMIC_RELEASE);
}
}
+
+ LIBC_INLINE T *get() const {
+ if constexpr (IsAtomic) {
+ // Weak micro-architectures typically reguards simultaneous partial word
+ // loading and full word loading as a race condition. While there are
+ // implementations that uses racy read anyway, we still load the whole
+ // word to avoid any complications.
+ typename Impl::Atomic snapshot;
+ __atomic_load(&impl.atomic, &snapshot, __ATOMIC_RELAXED);
+ return snapshot.ptr;
+ } else {
+ return impl.mutex.ptr;
+ }
+ }
};
template <class T> using AbaPtr = AbaPtrImpl<T, LIBC_ABA_PTR_IS_ATOMIC>;
diff --git a/libc/src/__support/mpmc_stack.h b/libc/src/__support/mpmc_stack.h
index 819433f24380c..c6546bac9be9a 100644
--- a/libc/src/__support/mpmc_stack.h
+++ b/libc/src/__support/mpmc_stack.h
@@ -12,13 +12,14 @@
#include "src/__support/CPP/atomic.h"
#include "src/__support/CPP/new.h"
#include "src/__support/CPP/optional.h"
+#include "src/__support/CPP/type_traits.h"
#include "src/__support/aba_ptr.h"
namespace LIBC_NAMESPACE_DECL {
template <class T> class MPMCStack {
struct Node {
cpp::Atomic<size_t> visitor;
- AbaPtr<Node> next;
+ Node *next;
T value;
LIBC_INLINE Node(T val) : visitor(0), next(nullptr), value(val) {}
@@ -31,7 +32,7 @@ template <class T> class MPMCStack {
LIBC_INLINE MPMCStack() : head(nullptr) {}
LIBC_INLINE bool push(T value) {
AllocChecker ac;
- Node *new_node = new Node(value, ac);
+ Node *new_node = new (ac) Node(value);
if (!ac)
return false;
head.transaction([new_node](Node *old_head) {
@@ -41,17 +42,17 @@ template <class T> class MPMCStack {
return true;
}
LIBC_INLINE cpp::optional<T> pop() {
- cpp::optional<T> res;
- Node *node;
- head.transaction([&](Node *current_head) {
+ cpp::optional<T> res = cpp::nullopt;
+ Node *node = nullptr;
+ head.transaction([&](Node *current_head) -> Node * {
if (!current_head) {
res = cpp::nullopt;
return nullptr;
}
node = current_head;
node->visitor.fetch_add(1);
- res = node->value;
- auto next = node->next;
+ res = cpp::optional<T>{node->value};
+ Node *next = node->next;
node->visitor.fetch_sub(1);
return next;
});
diff --git a/libc/test/integration/src/__support/CMakeLists.txt b/libc/test/integration/src/__support/CMakeLists.txt
index b5b6557e8d689..93f54083f3c00 100644
--- a/libc/test/integration/src/__support/CMakeLists.txt
+++ b/libc/test/integration/src/__support/CMakeLists.txt
@@ -2,3 +2,18 @@ add_subdirectory(threads)
if(LIBC_TARGET_OS_IS_GPU)
add_subdirectory(GPU)
endif()
+
+add_libc_integration_test_suite(libc-support-integration-tests)
+
+add_integration_test(
+ mpmc_stack_test
+ SUITE
+ libc-support-integration-tests
+ SRCS
+ mpmc_stack_test.cpp
+ DEPENDS
+ libc.src.__support.mpmc_stack
+ libc.src.__support.threads.thread
+ libc.src.pthread.pthread_create
+ libc.src.pthread.pthread_join
+)
diff --git a/libc/test/integration/src/__support/mpmc_stack_test.cpp b/libc/test/integration/src/__support/mpmc_stack_test.cpp
new file mode 100644
index 0000000000000..3cc8237c9e179
--- /dev/null
+++ b/libc/test/integration/src/__support/mpmc_stack_test.cpp
@@ -0,0 +1,61 @@
+#include "src/__support/CPP/atomic.h"
+#include "src/__support/mpmc_stack.h"
+#include "src/pthread/pthread_create.h"
+#include "src/pthread/pthread_join.h"
+#include "test/IntegrationTest/test.h"
+
+using namespace LIBC_NAMESPACE;
+
+void smoke_test() {
+ MPMCStack<int> stack;
+ for (int i = 0; i <= 100; ++i)
+ if (!stack.push(i))
+ __builtin_trap();
+ for (int i = 100; i >= 0; --i)
+ if (*stack.pop() != i)
+ __builtin_trap();
+ if (stack.pop())
+ __builtin_trap(); // Should be empty now.
+}
+
+void multithread_test() {
+ constexpr static size_t NUM_THREADS = 5;
+ constexpr static size_t NUM_PUSHES = 100;
+ struct State {
+ MPMCStack<size_t> stack;
+ cpp::Atomic<size_t> counter = 0;
+ cpp::Atomic<bool> flags[NUM_PUSHES];
+ } state;
+ pthread_t threads[NUM_THREADS];
+ for (size_t i = 0; i < NUM_THREADS; ++i) {
+ LIBC_NAMESPACE::pthread_create(
+ &threads[i], nullptr,
+ [](void *arg) -> void * {
+ State *state = static_cast<State *>(arg);
+ for (;;) {
+ size_t current = state->counter.fetch_add(1);
+ if (current >= NUM_PUSHES)
+ break;
+ if (!state->stack.push(current))
+ __builtin_trap();
+ }
+ while (auto res = state->stack.pop())
+ state->flags[res.value()].store(true);
+ return nullptr;
+ },
+ &state);
+ }
+ for (pthread_t thread : threads)
+ LIBC_NAMESPACE::pthread_join(thread, nullptr);
+ while (cpp::optional<size_t> res = state.stack.pop())
+ state.flags[res.value()].store(true);
+ for (size_t i = 0; i < NUM_PUSHES; ++i)
+ if (!state.flags[i].load())
+ __builtin_trap();
+}
+
+TEST_MAIN() {
+ smoke_test();
+ multithread_test();
+ return 0;
+}
>From 7bb7c6d3b31a1eccbe30c435a15daf8e0ab37c13 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Fri, 8 Aug 2025 09:06:31 -0400
Subject: [PATCH 4/6] move changes
---
libc/src/__support/mpmc_stack.h | 36 +++++++++++-
.../src/__support/mpmc_stack_test.cpp | 58 +++++++++++++++++++
2 files changed, 92 insertions(+), 2 deletions(-)
diff --git a/libc/src/__support/mpmc_stack.h b/libc/src/__support/mpmc_stack.h
index c6546bac9be9a..df235c2c1dfac 100644
--- a/libc/src/__support/mpmc_stack.h
+++ b/libc/src/__support/mpmc_stack.h
@@ -12,7 +12,6 @@
#include "src/__support/CPP/atomic.h"
#include "src/__support/CPP/new.h"
#include "src/__support/CPP/optional.h"
-#include "src/__support/CPP/type_traits.h"
#include "src/__support/aba_ptr.h"
namespace LIBC_NAMESPACE_DECL {
@@ -29,7 +28,7 @@ template <class T> class MPMCStack {
public:
static_assert(cpp::is_copy_constructible<T>::value,
"T must be copy constructible");
- LIBC_INLINE MPMCStack() : head(nullptr) {}
+ LIBC_INLINE constexpr MPMCStack() : head(nullptr) {}
LIBC_INLINE bool push(T value) {
AllocChecker ac;
Node *new_node = new (ac) Node(value);
@@ -41,6 +40,39 @@ template <class T> class MPMCStack {
});
return true;
}
+ LIBC_INLINE bool push_all(T values[], size_t count) {
+ struct Guard {
+ size_t count;
+ Node **allocated;
+ LIBC_INLINE Guard(Node *allocated[]) : count(0), allocated(allocated) {}
+ LIBC_INLINE ~Guard() {
+ for (size_t i = 0; i < count; ++i)
+ delete allocated[i];
+ }
+ LIBC_INLINE void add(Node *node) { allocated[count++] = node; }
+ LIBC_INLINE void clear() { count = 0; }
+ };
+ // Variable sized array is a GNU extension.
+ __extension__ Node *allocated[count];
+ {
+ Guard guard(allocated);
+ for (size_t i = 0; i < count; ++i) {
+ AllocChecker ac;
+ Node *new_node = new (ac) Node(values[i]);
+ if (!ac)
+ return false;
+ guard.add(new_node);
+ if (i != 0)
+ new_node->next = allocated[i - 1];
+ }
+ guard.clear();
+ }
+ head.transaction([&allocated, count](Node *old_head) {
+ allocated[0]->next = old_head;
+ return allocated[count - 1];
+ });
+ return true;
+ }
LIBC_INLINE cpp::optional<T> pop() {
cpp::optional<T> res = cpp::nullopt;
Node *node = nullptr;
diff --git a/libc/test/integration/src/__support/mpmc_stack_test.cpp b/libc/test/integration/src/__support/mpmc_stack_test.cpp
index 3cc8237c9e179..9166a816a74fe 100644
--- a/libc/test/integration/src/__support/mpmc_stack_test.cpp
+++ b/libc/test/integration/src/__support/mpmc_stack_test.cpp
@@ -54,8 +54,66 @@ void multithread_test() {
__builtin_trap();
}
+void multithread_push_all_test() {
+ constexpr static size_t NUM_THREADS = 4;
+ constexpr static size_t BATCH_SIZE = 10;
+ constexpr static size_t NUM_BATCHES = 20;
+ struct State {
+ MPMCStack<size_t> stack;
+ cpp::Atomic<size_t> counter = 0;
+ cpp::Atomic<bool> flags[NUM_THREADS * BATCH_SIZE * NUM_BATCHES];
+ } state;
+ pthread_t threads[NUM_THREADS];
+
+ for (size_t i = 0; i < NUM_THREADS; ++i) {
+ LIBC_NAMESPACE::pthread_create(
+ &threads[i], nullptr,
+ [](void *arg) -> void * {
+ State *state = static_cast<State *>(arg);
+ size_t values[BATCH_SIZE];
+
+ for (size_t batch = 0; batch < NUM_BATCHES; ++batch) {
+ // Prepare batch of values
+ for (size_t j = 0; j < BATCH_SIZE; ++j) {
+ size_t current = state->counter.fetch_add(1);
+ values[j] = current;
+ }
+
+ // Push all values in batch
+ if (!state->stack.push_all(values, BATCH_SIZE))
+ __builtin_trap();
+ }
+
+ // Pop and mark all values
+ while (auto res = state->stack.pop()) {
+ size_t value = res.value();
+ if (value < NUM_THREADS * BATCH_SIZE * NUM_BATCHES)
+ state->flags[value].store(true);
+ }
+ return nullptr;
+ },
+ &state);
+ }
+
+ for (pthread_t thread : threads)
+ LIBC_NAMESPACE::pthread_join(thread, nullptr);
+
+ // Pop any remaining values
+ while (cpp::optional<size_t> res = state.stack.pop()) {
+ size_t value = res.value();
+ if (value < NUM_THREADS * BATCH_SIZE * NUM_BATCHES)
+ state.flags[value].store(true);
+ }
+
+ // Verify all values were processed
+ for (size_t i = 0; i < NUM_THREADS * BATCH_SIZE * NUM_BATCHES; ++i)
+ if (!state.flags[i].load())
+ __builtin_trap();
+}
+
TEST_MAIN() {
smoke_test();
multithread_test();
+ multithread_push_all_test();
return 0;
}
>From 436947e3ca6000918e183cead706ee75d7f8bcc8 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Fri, 8 Aug 2025 09:22:07 -0400
Subject: [PATCH 5/6] Update libc/src/__support/aba_ptr.h
Co-authored-by: Copilot <175728472+Copilot at users.noreply.github.com>
---
libc/src/__support/aba_ptr.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libc/src/__support/aba_ptr.h b/libc/src/__support/aba_ptr.h
index 632cc466c295b..3e55aea8875a3 100644
--- a/libc/src/__support/aba_ptr.h
+++ b/libc/src/__support/aba_ptr.h
@@ -63,7 +63,7 @@ template <class T, bool IsAtomic> struct AbaPtrImpl {
LIBC_INLINE T *get() const {
if constexpr (IsAtomic) {
- // Weak micro-architectures typically reguards simultaneous partial word
+ // Weak micro-architectures typically regards simultaneous partial word
// loading and full word loading as a race condition. While there are
// implementations that uses racy read anyway, we still load the whole
// word to avoid any complications.
>From ee8e3013c4910a86f6f1feabe88c2126a38a4c06 Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Fri, 8 Aug 2025 10:21:51 -0400
Subject: [PATCH 6/6] address jhuber6's comments
---
libc/src/__support/CMakeLists.txt | 1 +
libc/src/__support/aba_ptr.h | 20 ++++++++-----
libc/src/__support/mpmc_stack.h | 49 ++++++++++++++++++-------------
3 files changed, 43 insertions(+), 27 deletions(-)
diff --git a/libc/src/__support/CMakeLists.txt b/libc/src/__support/CMakeLists.txt
index c9d89cf6fc286..7103bf7f5fc21 100644
--- a/libc/src/__support/CMakeLists.txt
+++ b/libc/src/__support/CMakeLists.txt
@@ -405,6 +405,7 @@ add_header_library(
DEPENDS
libc.hdr.types.size_t
libc.src.__support.common
+ libc.src.__support.CPP.atomic
libc.src.__support.threads.sleep
)
diff --git a/libc/src/__support/aba_ptr.h b/libc/src/__support/aba_ptr.h
index 3e55aea8875a3..b4291f1800dcd 100644
--- a/libc/src/__support/aba_ptr.h
+++ b/libc/src/__support/aba_ptr.h
@@ -10,6 +10,7 @@
#define LLVM_LIBC_SRC___SUPPORT_TAGGED_POINTER_H
#include "hdr/types/size_t.h"
+#include "src/__support/CPP/atomic.h"
#include "src/__support/common.h"
#include "src/__support/threads/sleep.h"
@@ -40,24 +41,28 @@ template <class T, bool IsAtomic> struct AbaPtrImpl {
template <class Op> LIBC_INLINE void transaction(Op &&op) {
if constexpr (IsAtomic) {
for (;;) {
+ cpp::AtomicRef<typename Impl::Atomic> ref(impl.atomic);
typename Impl::Atomic snapshot, next;
- __atomic_load(&impl.atomic, &snapshot, __ATOMIC_RELAXED);
+ snapshot = ref.load(cpp::MemoryOrder::RELAXED);
next.ptr = op(snapshot.ptr);
// Wrapping add for unsigned integers.
next.tag = snapshot.tag + 1;
- if (__atomic_compare_exchange(&impl.atomic, &snapshot, &next, true,
- __ATOMIC_ACQ_REL, __ATOMIC_RELAXED))
+ // Redo transaction can be costly, so we use strong version.
+ if (ref.compare_exchange_strong(snapshot, next,
+ cpp::MemoryOrder::ACQ_REL,
+ cpp::MemoryOrder::RELAXED))
return;
}
} else {
// Acquire the lock.
- while (__atomic_exchange_n(&impl.mutex.locked, true, __ATOMIC_ACQUIRE))
- while (__atomic_load_n(&impl.mutex.locked, __ATOMIC_RELAXED))
+ cpp::AtomicRef<bool> ref(impl.mutex.locked);
+ while (ref.exchange(true, cpp::MemoryOrder::ACQUIRE))
+ while (ref.load(cpp::MemoryOrder::RELAXED))
LIBC_NAMESPACE::sleep_briefly();
impl.mutex.ptr = op(impl.mutex.ptr);
// Release the lock.
- __atomic_store_n(&impl.mutex.locked, false, __ATOMIC_RELEASE);
+ ref.store(false, cpp::MemoryOrder::RELEASE);
}
}
@@ -68,7 +73,8 @@ template <class T, bool IsAtomic> struct AbaPtrImpl {
// implementations that uses racy read anyway, we still load the whole
// word to avoid any complications.
typename Impl::Atomic snapshot;
- __atomic_load(&impl.atomic, &snapshot, __ATOMIC_RELAXED);
+ cpp::AtomicRef<typename Impl::Atomic> ref(impl.atomic);
+ snapshot = ref.load();
return snapshot.ptr;
} else {
return impl.mutex.ptr;
diff --git a/libc/src/__support/mpmc_stack.h b/libc/src/__support/mpmc_stack.h
index df235c2c1dfac..259732ef84568 100644
--- a/libc/src/__support/mpmc_stack.h
+++ b/libc/src/__support/mpmc_stack.h
@@ -42,34 +42,43 @@ template <class T> class MPMCStack {
}
LIBC_INLINE bool push_all(T values[], size_t count) {
struct Guard {
- size_t count;
- Node **allocated;
- LIBC_INLINE Guard(Node *allocated[]) : count(0), allocated(allocated) {}
+ Node *cursor;
+ LIBC_INLINE Guard() : cursor(nullptr) {}
LIBC_INLINE ~Guard() {
- for (size_t i = 0; i < count; ++i)
- delete allocated[i];
+ while (cursor) {
+ Node *next = cursor->next;
+ delete cursor;
+ cursor = next;
+ }
+ }
+ LIBC_INLINE void advance(Node *node) {
+ node->next = cursor;
+ cursor = node;
+ }
+ LIBC_INLINE Node *finish() {
+ Node *ret = cursor;
+ cursor = nullptr;
+ return ret;
}
- LIBC_INLINE void add(Node *node) { allocated[count++] = node; }
- LIBC_INLINE void clear() { count = 0; }
};
- // Variable sized array is a GNU extension.
- __extension__ Node *allocated[count];
+ Node *first = nullptr;
+ Node *last = nullptr;
{
- Guard guard(allocated);
+ Guard guard{};
for (size_t i = 0; i < count; ++i) {
AllocChecker ac;
Node *new_node = new (ac) Node(values[i]);
if (!ac)
return false;
- guard.add(new_node);
- if (i != 0)
- new_node->next = allocated[i - 1];
+ if (i == 0)
+ first = new_node;
+ guard.advance(new_node);
}
- guard.clear();
+ last = guard.finish();
}
- head.transaction([&allocated, count](Node *old_head) {
- allocated[0]->next = old_head;
- return allocated[count - 1];
+ head.transaction([first, last](Node *old_head) {
+ first->next = old_head;
+ return last;
});
return true;
}
@@ -82,10 +91,10 @@ template <class T> class MPMCStack {
return nullptr;
}
node = current_head;
- node->visitor.fetch_add(1);
+ node->visitor.fetch_add(1, cpp::MemoryOrder::ACQUIRE);
res = cpp::optional<T>{node->value};
Node *next = node->next;
- node->visitor.fetch_sub(1);
+ node->visitor.fetch_sub(1, cpp::MemoryOrder::RELEASE);
return next;
});
// On a successful transaction, a node is popped by us. So we must delete
@@ -95,7 +104,7 @@ template <class T> class MPMCStack {
// node.
if (res) {
// Spin until the node is no longer in use.
- while (node->visitor.load() != 0)
+ while (node->visitor.load(cpp::MemoryOrder::RELAXED) != 0)
LIBC_NAMESPACE::sleep_briefly();
delete node;
}
More information about the libc-commits
mailing list