[libc-commits] [libc] 5442e15 - [libc][__support] move CndVar to __support (#89329)
via libc-commits
libc-commits at lists.llvm.org
Tue May 21 15:49:46 PDT 2024
Author: Nick Desaulniers (paternity leave)
Date: 2024-05-21T15:49:42-07:00
New Revision: 5442e15a152614628e2e2bef250c91eacf75fc9c
URL: https://github.com/llvm/llvm-project/commit/5442e15a152614628e2e2bef250c91eacf75fc9c
DIFF: https://github.com/llvm/llvm-project/commit/5442e15a152614628e2e2bef250c91eacf75fc9c.diff
LOG: [libc][__support] move CndVar to __support (#89329)
We should be able to reuse this between the implementation of C11 cnd_t
condition variables and POSIX pthread_cond_t condition variables.
The current implementation is hyper linux specific, making use of Futex. That
obviously wont work outside of linux, so split the OS specific functions off
into their own source outside of the header.
Modifies the return values of the to-be-shared impl to return 0 on success and
-1 on error. This pattern was shamelessly stolen from Bionic's
[__bionic_thrd_error](https://android.googlesource.com/platform/bionic/+/refs/heads/main/libc/include/bits/threads_inlines.h#41).
Fixes: #88580
Link: #88583
Added:
libc/src/__support/threads/CndVar.h
libc/src/__support/threads/linux/CndVar.cpp
Modified:
libc/src/__support/threads/CMakeLists.txt
libc/src/__support/threads/linux/CMakeLists.txt
libc/src/threads/linux/CMakeLists.txt
libc/src/threads/linux/cnd_broadcast.cpp
libc/src/threads/linux/cnd_destroy.cpp
libc/src/threads/linux/cnd_init.cpp
libc/src/threads/linux/cnd_signal.cpp
libc/src/threads/linux/cnd_wait.cpp
Removed:
libc/src/threads/linux/CndVar.h
################################################################################
diff --git a/libc/src/__support/threads/CMakeLists.txt b/libc/src/__support/threads/CMakeLists.txt
index 34412be4dfed6..9ea0b59befe7a 100644
--- a/libc/src/__support/threads/CMakeLists.txt
+++ b/libc/src/__support/threads/CMakeLists.txt
@@ -71,3 +71,12 @@ if(TARGET libc.src.__support.threads.${LIBC_TARGET_OS}.callonce)
.${LIBC_TARGET_OS}.callonce
)
endif()
+
+if(TARGET libc.src.__support.threads.${LIBC_TARGET_OS}.CndVar)
+ add_object_library(
+ CndVar
+ ALIAS
+ DEPENDS
+ .${LIBC_TARGET_OS}.CndVar
+ )
+endif()
diff --git a/libc/src/__support/threads/CndVar.h b/libc/src/__support/threads/CndVar.h
new file mode 100644
index 0000000000000..baa2a686c57d4
--- /dev/null
+++ b/libc/src/__support/threads/CndVar.h
@@ -0,0 +1,52 @@
+//===-- A platform independent abstraction layer for cond vars --*- 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___SUPPORT_SRC_THREADS_LINUX_CNDVAR_H
+#define LLVM_LIBC___SUPPORT_SRC_THREADS_LINUX_CNDVAR_H
+
+#include "src/__support/threads/linux/futex_utils.h" // Futex
+#include "src/__support/threads/mutex.h" // Mutex
+
+#include <stdint.h> // uint32_t
+
+namespace LIBC_NAMESPACE {
+
+struct CndVar {
+ enum CndWaiterStatus : uint32_t {
+ WS_Waiting = 0xE,
+ WS_Signalled = 0x5,
+ };
+
+ struct CndWaiter {
+ Futex futex_word = WS_Waiting;
+ CndWaiter *next = nullptr;
+ };
+
+ CndWaiter *waitq_front;
+ CndWaiter *waitq_back;
+ Mutex qmtx;
+
+ static int init(CndVar *cv) {
+ cv->waitq_front = cv->waitq_back = nullptr;
+ auto err = Mutex::init(&cv->qmtx, false, false, false);
+ return err == MutexError::NONE ? 0 : -1;
+ }
+
+ static void destroy(CndVar *cv) {
+ cv->waitq_front = cv->waitq_back = nullptr;
+ }
+
+ // Returns 0 on success, -1 on error.
+ int wait(Mutex *m);
+ void notify_one();
+ void broadcast();
+};
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC___SUPPORT_THREADS_LINUX_CNDVAR_H
diff --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt
index d3353f6b3ff8c..39c4ad20201ca 100644
--- a/libc/src/__support/threads/linux/CMakeLists.txt
+++ b/libc/src/__support/threads/linux/CMakeLists.txt
@@ -63,3 +63,16 @@ add_object_library(
DEPENDS
.futex_utils
)
+
+add_object_library(
+ CndVar
+ SRCS
+ CndVar.cpp
+ HDRS
+ ../CndVar.h
+ DEPENDS
+ libc.include.sys_syscall
+ libc.src.__support.OSUtil.osutil
+ libc.src.__support.threads.linux.futex_word_type
+ libc.src.__support.threads.mutex
+)
diff --git a/libc/src/__support/threads/linux/CndVar.cpp b/libc/src/__support/threads/linux/CndVar.cpp
new file mode 100644
index 0000000000000..daf56bca1ed21
--- /dev/null
+++ b/libc/src/__support/threads/linux/CndVar.cpp
@@ -0,0 +1,103 @@
+//===-- Utility condition variable class ------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/threads/CndVar.h"
+#include "src/__support/OSUtil/syscall.h" // syscall_impl
+#include "src/__support/threads/linux/futex_word.h" // FutexWordType
+#include "src/__support/threads/mutex.h" // Mutex, MutexLock
+
+#include <sys/syscall.h> // For syscall numbers.
+
+namespace LIBC_NAMESPACE {
+
+int CndVar::wait(Mutex *m) {
+ // The goal is to perform "unlock |m| and wait" in an
+ // atomic operation. However, it is not possible to do it
+ // in the true sense so we do it in spirit. Before unlocking
+ // |m|, a new waiter object is added to the waiter queue with
+ // the waiter queue locked. Iff a signalling thread signals
+ // the waiter before the waiter actually starts waiting, the
+ // wait operation will not begin at all and the waiter immediately
+ // returns.
+
+ CndWaiter waiter;
+ {
+ MutexLock ml(&qmtx);
+ CndWaiter *old_back = nullptr;
+ if (waitq_front == nullptr) {
+ waitq_front = waitq_back = &waiter;
+ } else {
+ old_back = waitq_back;
+ waitq_back->next = &waiter;
+ waitq_back = &waiter;
+ }
+
+ if (m->unlock() != MutexError::NONE) {
+ // If we do not remove the queued up waiter before returning,
+ // then another thread can potentially signal a non-existing
+ // waiter. Note also that we do this with |qmtx| locked. This
+ // ensures that another thread will not signal the withdrawing
+ // waiter.
+ waitq_back = old_back;
+ if (waitq_back == nullptr)
+ waitq_front = nullptr;
+ else
+ waitq_back->next = nullptr;
+
+ return -1;
+ }
+ }
+
+ waiter.futex_word.wait(WS_Waiting, cpp::nullopt, true);
+
+ // At this point, if locking |m| fails, we can simply return as the
+ // queued up waiter would have been removed from the queue.
+ auto err = m->lock();
+ return err == MutexError::NONE ? 0 : -1;
+}
+
+void CndVar::notify_one() {
+ // We don't use an RAII locker in this method as we want to unlock
+ // |qmtx| and signal the waiter using a single FUTEX_WAKE_OP signal.
+ qmtx.lock();
+ if (waitq_front == nullptr)
+ qmtx.unlock();
+
+ CndWaiter *first = waitq_front;
+ waitq_front = waitq_front->next;
+ if (waitq_front == nullptr)
+ waitq_back = nullptr;
+
+ qmtx.futex_word = FutexWordType(Mutex::LockState::Free);
+
+ // this is a special WAKE_OP, so we use syscall directly
+ LIBC_NAMESPACE::syscall_impl<long>(
+ FUTEX_SYSCALL_ID, &qmtx.futex_word.val, FUTEX_WAKE_OP, 1, 1,
+ &first->futex_word.val,
+ FUTEX_OP(FUTEX_OP_SET, WS_Signalled, FUTEX_OP_CMP_EQ, WS_Waiting));
+}
+
+void CndVar::broadcast() {
+ MutexLock ml(&qmtx);
+ uint32_t dummy_futex_word;
+ CndWaiter *waiter = waitq_front;
+ waitq_front = waitq_back = nullptr;
+ while (waiter != nullptr) {
+ // FUTEX_WAKE_OP is used instead of just FUTEX_WAKE as it allows us to
+ // atomically update the waiter status to WS_Signalled before waking
+ // up the waiter. A dummy location is used for the other futex of
+ // FUTEX_WAKE_OP.
+ LIBC_NAMESPACE::syscall_impl<long>(
+ FUTEX_SYSCALL_ID, &dummy_futex_word, FUTEX_WAKE_OP, 1, 1,
+ &waiter->futex_word.val,
+ FUTEX_OP(FUTEX_OP_SET, WS_Signalled, FUTEX_OP_CMP_EQ, WS_Waiting));
+ waiter = waiter->next;
+ }
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/threads/linux/CMakeLists.txt b/libc/src/threads/linux/CMakeLists.txt
index 68b7106c2052f..a5a02e47aab38 100644
--- a/libc/src/threads/linux/CMakeLists.txt
+++ b/libc/src/threads/linux/CMakeLists.txt
@@ -1,7 +1,6 @@
add_header_library(
threads_utils
HDRS
- CndVar.h
Futex.h
DEPENDS
libc.include.sys_syscall
@@ -20,8 +19,8 @@ add_entrypoint_object(
HDRS
../cnd_init.h
DEPENDS
- .threads_utils
libc.include.threads
+ libc.src.__support.threads.CndVar
)
add_entrypoint_object(
@@ -31,8 +30,8 @@ add_entrypoint_object(
HDRS
../cnd_destroy.h
DEPENDS
- .threads_utils
libc.include.threads
+ libc.src.__support.threads.CndVar
)
add_entrypoint_object(
@@ -42,9 +41,9 @@ add_entrypoint_object(
HDRS
../cnd_wait.h
DEPENDS
- .threads_utils
libc.include.threads
libc.src.__support.threads.mutex
+ libc.src.__support.threads.CndVar
)
add_entrypoint_object(
@@ -54,8 +53,8 @@ add_entrypoint_object(
HDRS
../cnd_signal.h
DEPENDS
- .threads_utils
libc.include.threads
+ libc.src.__support.threads.CndVar
)
add_entrypoint_object(
@@ -65,6 +64,6 @@ add_entrypoint_object(
HDRS
../cnd_broadcast.h
DEPENDS
- .threads_utils
libc.include.threads
+ libc.src.__support.threads.CndVar
)
diff --git a/libc/src/threads/linux/CndVar.h b/libc/src/threads/linux/CndVar.h
deleted file mode 100644
index c08ffa393856f..0000000000000
--- a/libc/src/threads/linux/CndVar.h
+++ /dev/null
@@ -1,148 +0,0 @@
-//===-- Utility condition variable class ------------------------*- 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_THREADS_LINUX_CNDVAR_H
-#define LLVM_LIBC_SRC_THREADS_LINUX_CNDVAR_H
-
-#include "src/__support/CPP/atomic.h"
-#include "src/__support/CPP/mutex.h" // lock_guard
-#include "src/__support/CPP/optional.h"
-#include "src/__support/OSUtil/syscall.h" // For syscall functions.
-#include "src/__support/threads/linux/futex_utils.h"
-#include "src/__support/threads/mutex.h"
-
-#include <linux/futex.h> // For futex operations.
-#include <stdint.h>
-#include <sys/syscall.h> // For syscall numbers.
-#include <threads.h> // For values like thrd_success etc.
-
-namespace LIBC_NAMESPACE {
-
-struct CndVar {
- enum CndWaiterStatus : uint32_t {
- WS_Waiting = 0xE,
- WS_Signalled = 0x5,
- };
-
- struct CndWaiter {
- Futex futex_word = WS_Waiting;
- CndWaiter *next = nullptr;
- };
-
- CndWaiter *waitq_front;
- CndWaiter *waitq_back;
- Mutex qmtx;
-
- static int init(CndVar *cv) {
- cv->waitq_front = cv->waitq_back = nullptr;
- auto err = Mutex::init(&cv->qmtx, false, false, false);
- return err == MutexError::NONE ? thrd_success : thrd_error;
- }
-
- static void destroy(CndVar *cv) {
- cv->waitq_front = cv->waitq_back = nullptr;
- }
-
- int wait(Mutex *m) {
- // The goal is to perform "unlock |m| and wait" in an
- // atomic operation. However, it is not possible to do it
- // in the true sense so we do it in spirit. Before unlocking
- // |m|, a new waiter object is added to the waiter queue with
- // the waiter queue locked. Iff a signalling thread signals
- // the waiter before the waiter actually starts waiting, the
- // wait operation will not begin at all and the waiter immediately
- // returns.
-
- CndWaiter waiter;
- {
- cpp::lock_guard ml(qmtx);
- CndWaiter *old_back = nullptr;
- if (waitq_front == nullptr) {
- waitq_front = waitq_back = &waiter;
- } else {
- old_back = waitq_back;
- waitq_back->next = &waiter;
- waitq_back = &waiter;
- }
-
- if (m->unlock() != MutexError::NONE) {
- // If we do not remove the queued up waiter before returning,
- // then another thread can potentially signal a non-existing
- // waiter. Note also that we do this with |qmtx| locked. This
- // ensures that another thread will not signal the withdrawing
- // waiter.
- waitq_back = old_back;
- if (waitq_back == nullptr)
- waitq_front = nullptr;
- else
- waitq_back->next = nullptr;
-
- return thrd_error;
- }
- }
-
- waiter.futex_word.wait(WS_Waiting, cpp::nullopt, true);
-
- // At this point, if locking |m| fails, we can simply return as the
- // queued up waiter would have been removed from the queue.
- auto err = m->lock();
- return err == MutexError::NONE ? thrd_success : thrd_error;
- }
-
- int notify_one() {
- // We don't use an RAII locker in this method as we want to unlock
- // |qmtx| and signal the waiter using a single FUTEX_WAKE_OP signal.
- qmtx.lock();
- if (waitq_front == nullptr) {
- qmtx.unlock();
- return thrd_success;
- }
-
- CndWaiter *first = waitq_front;
- waitq_front = waitq_front->next;
- if (waitq_front == nullptr)
- waitq_back = nullptr;
-
- qmtx.futex_word = FutexWordType(Mutex::LockState::Free);
-
- // this is a special WAKE_OP, so we use syscall directly
- LIBC_NAMESPACE::syscall_impl<long>(
- FUTEX_SYSCALL_ID, &qmtx.futex_word.val, FUTEX_WAKE_OP, 1, 1,
- &first->futex_word.val,
- FUTEX_OP(FUTEX_OP_SET, WS_Signalled, FUTEX_OP_CMP_EQ, WS_Waiting));
- return thrd_success;
- }
-
- int broadcast() {
- cpp::lock_guard ml(qmtx);
- uint32_t dummy_futex_word;
- CndWaiter *waiter = waitq_front;
- waitq_front = waitq_back = nullptr;
- while (waiter != nullptr) {
- // FUTEX_WAKE_OP is used instead of just FUTEX_WAKE as it allows us to
- // atomically update the waiter status to WS_Signalled before waking
- // up the waiter. A dummy location is used for the other futex of
- // FUTEX_WAKE_OP.
- LIBC_NAMESPACE::syscall_impl<long>(
- FUTEX_SYSCALL_ID, &dummy_futex_word, FUTEX_WAKE_OP, 1, 1,
- &waiter->futex_word.val,
- FUTEX_OP(FUTEX_OP_SET, WS_Signalled, FUTEX_OP_CMP_EQ, WS_Waiting));
- waiter = waiter->next;
- }
- return thrd_success;
- }
-};
-
-static_assert(sizeof(CndVar) == sizeof(cnd_t),
- "Mismatch in the size of the "
- "internal representation of condition variable and the public "
- "cnd_t type.");
-
-} // namespace LIBC_NAMESPACE
-
-#endif // LLVM_LIBC_SRC_THREADS_LINUX_CNDVAR_H
diff --git a/libc/src/threads/linux/cnd_broadcast.cpp b/libc/src/threads/linux/cnd_broadcast.cpp
index 180ac6d68ee86..a56aaa21ee122 100644
--- a/libc/src/threads/linux/cnd_broadcast.cpp
+++ b/libc/src/threads/linux/cnd_broadcast.cpp
@@ -6,16 +6,21 @@
//
//===----------------------------------------------------------------------===//
-#include "CndVar.h"
-
#include "src/threads/cnd_broadcast.h"
#include "src/__support/common.h"
+#include "src/__support/threads/CndVar.h"
+
+// TODO: https://github.com/llvm/llvm-project/issues/92968
+#include <threads.h> // cnd_t, thrd_error, thrd_success
namespace LIBC_NAMESPACE {
+static_assert(sizeof(CndVar) == sizeof(cnd_t));
+
LLVM_LIBC_FUNCTION(int, cnd_broadcast, (cnd_t * cond)) {
CndVar *cndvar = reinterpret_cast<CndVar *>(cond);
- return cndvar->broadcast();
+ cndvar->broadcast();
+ return thrd_success;
}
} // namespace LIBC_NAMESPACE
diff --git a/libc/src/threads/linux/cnd_destroy.cpp b/libc/src/threads/linux/cnd_destroy.cpp
index 08eb3a1057b11..2b03b18c48e46 100644
--- a/libc/src/threads/linux/cnd_destroy.cpp
+++ b/libc/src/threads/linux/cnd_destroy.cpp
@@ -6,13 +6,16 @@
//
//===----------------------------------------------------------------------===//
-#include "CndVar.h"
-
#include "src/threads/cnd_destroy.h"
#include "src/__support/common.h"
+#include "src/__support/threads/CndVar.h"
+
+#include <threads.h> // cnd_t
namespace LIBC_NAMESPACE {
+static_assert(sizeof(CndVar) == sizeof(cnd_t));
+
LLVM_LIBC_FUNCTION(void, cnd_destroy, (cnd_t * cond)) {
CndVar *cndvar = reinterpret_cast<CndVar *>(cond);
CndVar::destroy(cndvar);
diff --git a/libc/src/threads/linux/cnd_init.cpp b/libc/src/threads/linux/cnd_init.cpp
index 5e3f360b1d2b9..d3d2c8a57d82a 100644
--- a/libc/src/threads/linux/cnd_init.cpp
+++ b/libc/src/threads/linux/cnd_init.cpp
@@ -6,16 +6,19 @@
//
//===----------------------------------------------------------------------===//
-#include "CndVar.h"
-
#include "src/threads/cnd_init.h"
#include "src/__support/common.h"
+#include "src/__support/threads/CndVar.h"
+
+#include <threads.h> // cnd_t, thrd_error, thrd_success
namespace LIBC_NAMESPACE {
+static_assert(sizeof(CndVar) == sizeof(cnd_t));
+
LLVM_LIBC_FUNCTION(int, cnd_init, (cnd_t * cond)) {
CndVar *cndvar = reinterpret_cast<CndVar *>(cond);
- return CndVar::init(cndvar);
+ return CndVar::init(cndvar) ? thrd_error : thrd_success;
}
} // namespace LIBC_NAMESPACE
diff --git a/libc/src/threads/linux/cnd_signal.cpp b/libc/src/threads/linux/cnd_signal.cpp
index dba01abdefbc9..f144013e08821 100644
--- a/libc/src/threads/linux/cnd_signal.cpp
+++ b/libc/src/threads/linux/cnd_signal.cpp
@@ -6,16 +6,20 @@
//
//===----------------------------------------------------------------------===//
-#include "CndVar.h"
-
#include "src/threads/cnd_signal.h"
#include "src/__support/common.h"
+#include "src/__support/threads/CndVar.h"
+
+#include <threads.h> // cnd_t, thrd_error, thrd_success
namespace LIBC_NAMESPACE {
+static_assert(sizeof(CndVar) == sizeof(cnd_t));
+
LLVM_LIBC_FUNCTION(int, cnd_signal, (cnd_t * cond)) {
CndVar *cndvar = reinterpret_cast<CndVar *>(cond);
- return cndvar->notify_one();
+ cndvar->notify_one();
+ return thrd_success;
}
} // namespace LIBC_NAMESPACE
diff --git a/libc/src/threads/linux/cnd_wait.cpp b/libc/src/threads/linux/cnd_wait.cpp
index db3d7f1436eb7..97cade3f231d7 100644
--- a/libc/src/threads/linux/cnd_wait.cpp
+++ b/libc/src/threads/linux/cnd_wait.cpp
@@ -6,18 +6,21 @@
//
//===----------------------------------------------------------------------===//
-#include "CndVar.h"
-
+#include "src/threads/cnd_wait.h"
#include "src/__support/common.h"
+#include "src/__support/threads/CndVar.h"
#include "src/__support/threads/mutex.h"
-#include "src/threads/cnd_wait.h"
+
+#include <threads.h> // cnd_t, mtx_t, thrd_error, thrd_success
namespace LIBC_NAMESPACE {
+static_assert(sizeof(CndVar) == sizeof(cnd_t));
+
LLVM_LIBC_FUNCTION(int, cnd_wait, (cnd_t * cond, mtx_t *mtx)) {
CndVar *cndvar = reinterpret_cast<CndVar *>(cond);
Mutex *mutex = reinterpret_cast<Mutex *>(mtx);
- return cndvar->wait(mutex);
+ return cndvar->wait(mutex) ? thrd_error : thrd_success;
}
} // namespace LIBC_NAMESPACE
More information about the libc-commits
mailing list