-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[libc] Add Darwin mutex support via os_sync primitives #167722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
c4fdba9 to
e2623e8
Compare
e2623e8 to
f84fbc4
Compare
🐧 Linux x64 Test ResultsThe build failed before running any tests. Click on a failure below to see the details. runtimes/runtimes-stamps/runtimes-configure /home/gha/actions-runner/_work/llvm-project/llvm-project/build/runtimes/runtimes-stamps/runtimes-configureIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
f84fbc4 to
ce329c4
Compare
|
@llvm/pr-subscribers-libc Author: Shreeyash Pandey (bojle) ChangesThis patch implements the generic mutex and raw_mutex interfaces on macOS. A new Futex class is provided that relies on os_sync_wait and os_sync_wake to emulate futex‑like wait and wake semantics. The OS‑specific part is moved into futex_utils, which now contains the Darwin implementation. Patch is 38.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/167722.diff 38 Files Affected:
diff --git a/libc/hdr/errno_macros.h b/libc/hdr/errno_macros.h
index 27ea49977d8c8..ce3453d16bce1 100644
--- a/libc/hdr/errno_macros.h
+++ b/libc/hdr/errno_macros.h
@@ -15,7 +15,9 @@
#include <linux/errno.h>
#include "include/llvm-libc-macros/error-number-macros.h"
-#else // __linux__
+#elif __APPLE__
+#include <sys/errno.h>
+#else // __APPLE__
#include "include/llvm-libc-macros/generic-error-number-macros.h"
#endif
diff --git a/libc/include/errno.h.def b/libc/include/errno.h.def
index aa1f6c9e48444..ba9864439369a 100644
--- a/libc/include/errno.h.def
+++ b/libc/include/errno.h.def
@@ -21,8 +21,14 @@
#include "llvm-libc-macros/linux/error-number-macros.h"
-#else // __linux__
+#elif __APPLE__
+
+#include <sys/errno.h>
+
+#else // __APPLE__
+
#include "llvm-libc-macros/generic-error-number-macros.h"
+
#endif
__BEGIN_C_DECLS
diff --git a/libc/include/llvm-libc-macros/darwin/CMakeLists.txt b/libc/include/llvm-libc-macros/darwin/CMakeLists.txt
new file mode 100644
index 0000000000000..ea08c63c00301
--- /dev/null
+++ b/libc/include/llvm-libc-macros/darwin/CMakeLists.txt
@@ -0,0 +1,5 @@
+add_header(
+ time_macros
+ HDR
+ time-macros.h
+)
diff --git a/libc/include/llvm-libc-macros/darwin/time-macros.h b/libc/include/llvm-libc-macros/darwin/time-macros.h
new file mode 100644
index 0000000000000..477dfa8eda85f
--- /dev/null
+++ b/libc/include/llvm-libc-macros/darwin/time-macros.h
@@ -0,0 +1,14 @@
+//===-- Definition of macros from time.h ---------------------------------===//
+//
+// 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_MACROS_LINUX_TIME_MACROS_H
+#define LLVM_LIBC_MACROS_LINUX_TIME_MACROS_H
+
+#include <_time.h>
+
+#endif // LLVM_LIBC_MACROS_LINUX_TIME_MACROS_H
diff --git a/libc/include/llvm-libc-macros/time-macros.h b/libc/include/llvm-libc-macros/time-macros.h
index 30e0a310a5485..c026df29b1e7f 100644
--- a/libc/include/llvm-libc-macros/time-macros.h
+++ b/libc/include/llvm-libc-macros/time-macros.h
@@ -7,6 +7,8 @@
#include "linux/time-macros.h"
#elif defined(__ELF__)
#include "baremetal/time-macros.h"
+#elif defined(__APPLE__)
+#include "darwin/time-macros.h"
#else
#define CLOCKS_PER_SEC 1000000
#endif
diff --git a/libc/include/llvm-libc-types/clockid_t.h b/libc/include/llvm-libc-types/clockid_t.h
index 4b059599502c4..926948717c664 100644
--- a/libc/include/llvm-libc-types/clockid_t.h
+++ b/libc/include/llvm-libc-types/clockid_t.h
@@ -9,6 +9,12 @@
#ifndef LLVM_LIBC_TYPES_CLOCKID_T_H
#define LLVM_LIBC_TYPES_CLOCKID_T_H
+#if defined(__APPLE__)
+// Darwin provides its own defintion for clockid_t . Use that to prevent
+// redeclaration errors and correctness.
+#include <_time.h>
+#else
typedef int clockid_t;
+#endif // __APPLE__
#endif // LLVM_LIBC_TYPES_CLOCKID_T_H
diff --git a/libc/include/llvm-libc-types/struct_timespec.h b/libc/include/llvm-libc-types/struct_timespec.h
index 28b5a571f6790..8993ecc7db8f0 100644
--- a/libc/include/llvm-libc-types/struct_timespec.h
+++ b/libc/include/llvm-libc-types/struct_timespec.h
@@ -9,6 +9,11 @@
#ifndef LLVM_LIBC_TYPES_STRUCT_TIMESPEC_H
#define LLVM_LIBC_TYPES_STRUCT_TIMESPEC_H
+#if defined(__APPLE__)
+// Darwin provides its own definition for struct timespec. Include it directly
+// to ensure type compatibility and avoid redefinition errors.
+#include <sys/_types/_timespec.h>
+#else
#include "time_t.h"
struct timespec {
@@ -16,5 +21,6 @@ struct timespec {
/* TODO: BIG_ENDIAN may require padding. */
long tv_nsec; /* Nanoseconds. */
};
+#endif // __APPLE__
#endif // LLVM_LIBC_TYPES_STRUCT_TIMESPEC_H
diff --git a/libc/include/llvm-libc-types/struct_timeval.h b/libc/include/llvm-libc-types/struct_timeval.h
index 9595d85a46c8f..41f0b4e92932e 100644
--- a/libc/include/llvm-libc-types/struct_timeval.h
+++ b/libc/include/llvm-libc-types/struct_timeval.h
@@ -12,9 +12,15 @@
#include "suseconds_t.h"
#include "time_t.h"
+#if defined(__APPLE__)
+// Darwin provides its own definition for struct timeval. Include it directly
+// to ensure type compatibility and avoid redefinition errors.
+#include <sys/_types/_timeval.h>
+#else
struct timeval {
time_t tv_sec; // Seconds
suseconds_t tv_usec; // Micro seconds
};
+#endif // __APPLE__
#endif // LLVM_LIBC_TYPES_STRUCT_TIMEVAL_H
diff --git a/libc/include/llvm-libc-types/suseconds_t.h b/libc/include/llvm-libc-types/suseconds_t.h
index 8e926e8401f5c..acc1822cb59e1 100644
--- a/libc/include/llvm-libc-types/suseconds_t.h
+++ b/libc/include/llvm-libc-types/suseconds_t.h
@@ -14,6 +14,12 @@
// types...] and suseconds_t are no greater than the width of type long.
// The kernel expects 64 bit suseconds_t at least on x86_64.
+#if defined(__APPLE__)
+// Darwin provides its own definition for suseconds_t. Include it directly
+// to ensure type compatibility and avoid redefinition errors.
+#include <sys/_types/_suseconds_t.h>
+#else
typedef long suseconds_t;
+#endif // __APPLE__
#endif // LLVM_LIBC_TYPES_SUSECONDS_T_H
diff --git a/libc/include/llvm-libc-types/time_t_32.h b/libc/include/llvm-libc-types/time_t_32.h
index 2c415f6fa9dca..8d7a81e5ce7f7 100644
--- a/libc/include/llvm-libc-types/time_t_32.h
+++ b/libc/include/llvm-libc-types/time_t_32.h
@@ -9,6 +9,12 @@
#ifndef LLVM_LIBC_TYPES_TIME_T_32_H
#define LLVM_LIBC_TYPES_TIME_T_32_H
+#if defined(__APPLE__)
+// Darwin provides its own definition for time_t. Include it directly
+// to ensure type compatibility and avoid redefinition errors.
+#include <sys/_types/_time_t.h>
+#else
typedef __INT32_TYPE__ time_t;
+#endif // __APPLE__
#endif // LLVM_LIBC_TYPES_TIME_T_32_H
diff --git a/libc/include/llvm-libc-types/time_t_64.h b/libc/include/llvm-libc-types/time_t_64.h
index 8f7fd3233646e..c8267abe31289 100644
--- a/libc/include/llvm-libc-types/time_t_64.h
+++ b/libc/include/llvm-libc-types/time_t_64.h
@@ -9,6 +9,12 @@
#ifndef LLVM_LIBC_TYPES_TIME_T_64_H
#define LLVM_LIBC_TYPES_TIME_T_64_H
+#if defined(__APPLE__)
+// Darwin provides its own definition for time_t. Include it directly
+// to ensure type compatibility and avoid redefinition errors.
+#include <sys/_types/_time_t.h>
+#else
typedef __INT64_TYPE__ time_t;
+#endif // __APPLE__
#endif // LLVM_LIBC_TYPES_TIME_T_64_H
diff --git a/libc/src/__support/OSUtil/darwin/exit.cpp b/libc/src/__support/OSUtil/darwin/exit.cpp
index 7439db2ef38b0..a5fa4a7522189 100644
--- a/libc/src/__support/OSUtil/darwin/exit.cpp
+++ b/libc/src/__support/OSUtil/darwin/exit.cpp
@@ -15,9 +15,8 @@ namespace LIBC_NAMESPACE_DECL {
namespace internal {
[[noreturn]] void exit(int status) {
- for (;;) {
+ for (;;)
LIBC_NAMESPACE::syscall_impl<long>(SYS_exit, status);
- }
}
} // namespace internal
diff --git a/libc/src/__support/threads/CMakeLists.txt b/libc/src/__support/threads/CMakeLists.txt
index f8a44937721b4..b2abf19c53a50 100644
--- a/libc/src/__support/threads/CMakeLists.txt
+++ b/libc/src/__support/threads/CMakeLists.txt
@@ -23,13 +23,36 @@ if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
add_subdirectory(${LIBC_TARGET_OS})
endif()
-if(TARGET libc.src.__support.threads.${LIBC_TARGET_OS}.mutex)
+if(TARGET libc.src.__support.threads.${LIBC_TARGET_OS}.futex_utils)
add_header_library(
mutex
HDRS
- mutex.h
+ mutex.h
+ DEPENDS
+ .unix_mutex
+ )
+ add_header_library(
+ unix_mutex
+ HDRS
+ unix_mutex.h
+ DEPENDS
+ .raw_mutex
+ )
+
+ add_header_library(
+ raw_mutex
+ HDRS
+ raw_mutex.h
DEPENDS
- .${LIBC_TARGET_OS}.mutex
+ .${LIBC_TARGET_OS}.futex_utils
+ libc.src.__support.threads.sleep
+ libc.src.__support.time.abs_timeout
+ libc.src.__support.time.monotonicity
+ libc.src.__support.CPP.optional
+ libc.hdr.types.pid_t
+ COMPILE_OPTIONS
+ -DLIBC_COPT_RAW_MUTEX_DEFAULT_SPIN_COUNT=${LIBC_CONF_RAW_MUTEX_DEFAULT_SPIN_COUNT}
+ ${monotonicity_flags}
)
add_object_library(
diff --git a/libc/src/__support/threads/darwin/CMakeLists.txt b/libc/src/__support/threads/darwin/CMakeLists.txt
new file mode 100644
index 0000000000000..9c651d8c3b0f5
--- /dev/null
+++ b/libc/src/__support/threads/darwin/CMakeLists.txt
@@ -0,0 +1,16 @@
+if(NOT TARGET libc.src.__support.OSUtil.osutil)
+ return()
+endif()
+
+add_header_library(
+ futex_utils
+ HDRS
+ futex_utils.h
+ DEPENDS
+ libc.include.sys_syscall
+ libc.src.__support.OSUtil.osutil
+ libc.src.__support.CPP.atomic
+ libc.src.__support.CPP.limits
+ libc.src.__support.CPP.optional
+ libc.src.__support.threads.mutex_common
+)
diff --git a/libc/src/__support/threads/darwin/futex_utils.h b/libc/src/__support/threads/darwin/futex_utils.h
new file mode 100644
index 0000000000000..180083b7b2c68
--- /dev/null
+++ b/libc/src/__support/threads/darwin/futex_utils.h
@@ -0,0 +1,66 @@
+//===--- Futex utils for Darwin -----------------------------------*- 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_THREADS_DARWIN_FUTEX_UTILS_H
+#define LLVM_LIBC_SRC___SUPPORT_THREADS_DARWIN_FUTEX_UTILS_H
+
+#include "src/__support/CPP/atomic.h"
+#include "src/__support/CPP/optional.h"
+#include "src/__support/time/abs_timeout.h"
+#include "src/__support/time/clock_conversion.h"
+
+#include <os/os_sync_wait_on_address.h>
+
+namespace LIBC_NAMESPACE_DECL {
+
+using FutexWordType = uint32_t;
+
+struct Futex : public cpp::Atomic<FutexWordType> {
+ using cpp::Atomic<FutexWordType>::Atomic;
+ using Timeout = internal::AbsTimeout;
+
+ // The Darwin futex API does not return a value on timeout, so we have to
+ // check for it manually. This means we can't use the return value to
+ // distinguish between a timeout and a successful wake-up.
+ int wait(FutexWordType val, cpp::optional<Timeout> timeout, bool) {
+ if (timeout) {
+ struct timespec now;
+ clock_gettime(timeout->is_realtime() ? CLOCK_REALTIME : CLOCK_MONOTONIC,
+ &now);
+ const timespec &target_ts = timeout->get_timespec();
+
+ if (now.tv_sec > target_ts.tv_sec ||
+ (now.tv_sec == target_ts.tv_sec && now.tv_nsec >= target_ts.tv_nsec))
+ return ETIMEDOUT;
+ }
+
+ os_sync_wait_on_address(reinterpret_cast<void *>(this),
+ static_cast<uint64_t>(val), sizeof(FutexWordType),
+ OS_SYNC_WAIT_ON_ADDRESS_NONE);
+ return 0;
+ }
+
+ void notify_one(bool) {
+ os_sync_wake_by_address_any(reinterpret_cast<void *>(this),
+ sizeof(FutexWordType),
+ OS_SYNC_WAKE_BY_ADDRESS_NONE);
+ }
+
+ void notify_all(bool) {
+ // os_sync_wake_by_address_all is not available, so we use notify_one.
+ // This is not ideal, but it's the best we can do with the available API.
+ os_sync_wake_by_address_any(reinterpret_cast<void *>(this),
+ sizeof(FutexWordType),
+ OS_SYNC_WAKE_BY_ADDRESS_NONE);
+ }
+};
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC___SUPPORT_THREADS_DARWIN_FUTEX_UTILS_H
diff --git a/libc/src/__support/threads/darwin/mutex.h b/libc/src/__support/threads/darwin/mutex.h
new file mode 100644
index 0000000000000..44ba14cc986a2
--- /dev/null
+++ b/libc/src/__support/threads/darwin/mutex.h
@@ -0,0 +1,131 @@
+//===--- Implementation of a Darwin mutex 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___SUPPORT_THREADS_DARWIN_MUTEX_H
+#define LLVM_LIBC_SRC___SUPPORT_THREADS_DARWIN_MUTEX_H
+
+#include "src/__support/libc_assert.h"
+#include "src/__support/macros/config.h"
+#include "src/__support/threads/mutex_common.h"
+#include "src/__support/threads/sleep.h" // For sleep_briefly
+#include "src/__support/time/abs_timeout.h"
+
+#include <mach/mach_init.h> // For mach_thread_self
+#include <mach/mach_port.h> // For mach_port_t and MACH_PORT_NULL
+#include <os/lock.h> // For os_unfair_lock
+#include <time.h> // For clock_gettime
+
+namespace LIBC_NAMESPACE_DECL {
+
+// This file is an implementation of `LIBC_NAMESPACE::mutex` for Darwin-based
+// platforms. It is a wrapper around `os_unfair_lock`, which is a low-level,
+// high-performance locking primitive provided by the kernel.
+//
+// `os_unfair_lock` is a non-recursive, thread-owned lock that blocks waiters
+// efficiently in the kernel. As the name implies, it is "unfair," meaning
+// it does not guarantee the order in which waiting threads acquire the lock.
+// This trade-off allows for higher performance in contended scenarios.
+//
+// The lock must be unlocked from the same thread that locked it. Attempting
+// to unlock from a different thread will result in a runtime error.
+//
+// This implementation is suitable for simple critical sections where fairness
+// and reentrancy are not concerns.
+
+class Mutex final {
+ os_unfair_lock_s lock_val = OS_UNFAIR_LOCK_INIT;
+ mach_port_t owner = MACH_PORT_NULL;
+
+ // API compatibility fields.
+ unsigned char timed;
+ unsigned char recursive;
+ unsigned char robust;
+ unsigned char pshared;
+
+public:
+ LIBC_INLINE constexpr Mutex(bool is_timed, bool is_recursive, bool is_robust,
+ bool is_pshared)
+ : owner(MACH_PORT_NULL), timed(is_timed), recursive(is_recursive),
+ robust(is_robust), pshared(is_pshared) {}
+
+ LIBC_INLINE constexpr Mutex()
+ : owner(MACH_PORT_NULL), timed(0), recursive(0), robust(0), pshared(0) {}
+
+ LIBC_INLINE static MutexError init(Mutex *mutex, bool is_timed, bool is_recur,
+ bool is_robust, bool is_pshared) {
+ mutex->lock_val = OS_UNFAIR_LOCK_INIT;
+ mutex->owner = MACH_PORT_NULL;
+ mutex->timed = is_timed;
+ mutex->recursive = is_recur;
+ mutex->robust = is_robust;
+ mutex->pshared = is_pshared;
+ return MutexError::NONE;
+ }
+
+ LIBC_INLINE static MutexError destroy(Mutex *lock) {
+ LIBC_ASSERT(lock->owner == MACH_PORT_NULL &&
+ "Mutex destroyed while locked.");
+ return MutexError::NONE;
+ }
+
+ LIBC_INLINE MutexError lock() {
+ os_unfair_lock_lock(&lock_val);
+ owner = mach_thread_self();
+ return MutexError::NONE;
+ }
+
+ LIBC_INLINE MutexError timed_lock(internal::AbsTimeout abs_time) {
+ while (true) {
+ if (try_lock() == MutexError::NONE) {
+ return MutexError::NONE;
+ }
+
+ // Manually check if the timeout has expired.
+ struct timespec now;
+ // The clock used here must match the clock used to create the
+ // absolute timeout.
+ clock_gettime(abs_time.is_realtime() ? CLOCK_REALTIME : CLOCK_MONOTONIC,
+ &now);
+ const timespec &target_ts = abs_time.get_timespec();
+
+ if (now.tv_sec > target_ts.tv_sec || (now.tv_sec == target_ts.tv_sec &&
+ now.tv_nsec >= target_ts.tv_nsec)) {
+ // We might have acquired the lock between the last try_lock() and now.
+ // To avoid returning TIMEOUT incorrectly, we do one last try_lock().
+ if (try_lock() == MutexError::NONE)
+ return MutexError::NONE;
+ return MutexError::TIMEOUT;
+ }
+
+ sleep_briefly();
+ }
+ }
+
+ LIBC_INLINE MutexError unlock() {
+ // This check is crucial. It prevents both double-unlocks and unlocks
+ // by threads that do not own the mutex.
+ if (owner != mach_thread_self()) {
+ return MutexError::UNLOCK_WITHOUT_LOCK;
+ }
+ owner = MACH_PORT_NULL;
+ os_unfair_lock_unlock(&lock_val);
+ return MutexError::NONE;
+ }
+
+ LIBC_INLINE MutexError try_lock() {
+ if (os_unfair_lock_trylock(&lock_val)) {
+ owner = mach_thread_self();
+ return MutexError::NONE;
+ }
+ return MutexError::BUSY;
+ }
+};
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC___SUPPORT_THREADS_DARWIN_MUTEX_H
diff --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt
index 39d2c6fef0fed..cc596d217d7d2 100644
--- a/libc/src/__support/threads/linux/CMakeLists.txt
+++ b/libc/src/__support/threads/linux/CMakeLists.txt
@@ -21,7 +21,7 @@ add_header_library(
libc.src.__support.CPP.atomic
libc.src.__support.CPP.limits
libc.src.__support.CPP.optional
- libc.src.__support.time.linux.abs_timeout
+ libc.src.__support.time.abs_timeout
)
set(monotonicity_flags)
@@ -38,8 +38,8 @@ add_header_library(
DEPENDS
.futex_utils
libc.src.__support.threads.sleep
- libc.src.__support.time.linux.abs_timeout
- libc.src.__support.time.linux.monotonicity
+ libc.src.__support.time.abs_timeout
+ libc.src.__support.time.monotonicity
libc.src.__support.CPP.optional
libc.hdr.types.pid_t
COMPILE_OPTIONS
diff --git a/libc/src/__support/threads/linux/futex_utils.h b/libc/src/__support/threads/linux/futex_utils.h
index 943a99ab38c8c..e1cfa36566555 100644
--- a/libc/src/__support/threads/linux/futex_utils.h
+++ b/libc/src/__support/threads/linux/futex_utils.h
@@ -16,7 +16,7 @@
#include "src/__support/macros/attributes.h"
#include "src/__support/macros/config.h"
#include "src/__support/threads/linux/futex_word.h"
-#include "src/__support/time/linux/abs_timeout.h"
+#include "src/__support/time/abs_timeout.h"
#include <linux/errno.h>
#include <linux/futex.h>
diff --git a/libc/src/__support/threads/linux/rwlock.h b/libc/src/__support/threads/linux/rwlock.h
index f7aeb5b709aa3..165e17239bbd5 100644
--- a/libc/src/__support/threads/linux/rwlock.h
+++ b/libc/src/__support/threads/linux/rwlock.h
@@ -35,7 +35,7 @@
#endif
#if LIBC_COPT_TIMEOUT_ENSURE_MONOTONICITY
-#include "src/__support/time/linux/monotonicity.h"
+#include "src/__support/time/monotonicity.h"
#endif
namespace LIBC_NAMESPACE_DECL {
diff --git a/libc/src/__support/threads/mutex.h b/libc/src/__support/threads/mutex.h
index f64f7e7b40082..25feea891e429 100644
--- a/libc/src/__support/threads/mutex.h
+++ b/libc/src/__support/threads/mutex.h
@@ -40,9 +40,9 @@
// few global locks. So, to avoid static initialization order fiasco, we
// want the constructors of the Mutex classes to be constexprs.
-#if defined(__linux__)
-#include "src/__support/threads/linux/mutex.h"
-#endif // __linux__
+#if defined(__linux__) || defined(__APPLE__)
+#include "src/__support/threads/unix_mutex.h"
+#endif
#elif LIBC_THREAD_MODE == LIBC_THREAD_MODE_SINGLE
diff --git a/libc/src/__support/threads/linux/raw_mutex.h b/libc/src/__support/threads/raw_mutex.h
similarity index 88%
rename from libc/src/__support/threads/linux/raw_mutex.h
rename to libc/src/__support/threads/raw_mutex.h
index 47f0aa70f1c46..fb3f5c0f70ed6 100644
--- a/libc/src/__support/threads/linux/raw_mutex.h
+++ b/libc/src/__support/threads/raw_mutex.h
@@ -1,12 +1,12 @@
-//===--- Implementation of a Linux RawMutex class ---------------*- C++ -*-===//
+//===--- Implementation of the RawMutex 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___SUPPORT_THREADS_LINUX_RAW_MUTEX_H
-#define LLVM_LIBC_SRC___SUPPORT_THREADS_LINUX_RAW_MUTEX_H
+//===------------------------------------------------------------------===//
+#ifndef LLVM_LIBC_SRC___SUPPORT_THREADS_RAW_MUTEX_H
+#define LLVM_LIBC_SRC___SUPPORT_THREADS_RAW_MUTEX_H
#include "src/__support/CPP/optional.h"
#include "src/__support/common.h"
@@ -14,17 +14,23 @@
#include "src/__support/macros/attributes.h"
#include "src/__support/macros/config.h"
#include "src/__support/macros/optimization.h"
-#inclu...
[truncated]
|
This patch adds a basic implementation/wrapper for mutex using the underlying os_unfair_lock API that macos provides.
clock_conversion.h implements convert_clock which shifts a timestamp from one clock domain to another. It naturally does not depend on any OS specific interface. Making it generic will allow common use.
Adds support for darwin's errno headers/macros by including it from <sys/errno.h> and modifying header guards in hdr/ and include/.
This patch implements the generic mutex and raw_mutex interfaces on macOS. A new Futex class is provided that relies on os_sync_wait and os_sync_wake to emulate futex‑like wait and wake semantics. The OS‑specific part is moved into futex_utils, which now contains the Darwin implementation.
ec57b31 to
74ed72e
Compare
|
@bojle I am curious about the order of the patches again. Could you remind me of the stacking again? |
This is the stack. |
SchrodingerZhu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, as I said in my previous comment, some of my question may be due to confusion of the stacking layer.
Thank you for the patch! Looking forward to get this landed. After your refactoring, I guess it will also make it easier for me to add the windows counter parts (using WaitOnAddress).
|
|
||
| LIBC_INLINE long wait(FutexWordType val, cpp::optional<Timeout> timeout, | ||
| bool /* is_shared */) { | ||
| // TODO(bojle): consider using OS_SYNC_WAIT_ON_ADDRESS_SHARED to sync |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the document says:
Use this flag when you pass an address allocated in shared memory to any futex wait function. Shared memory can be used for waits and wakes within a single process, but incur a performance hit.
This extra cost is expected and it is the reason why POSIX API also distinguish shared memory locks and normal private locks.
| return 0; | ||
| long ret = 0; | ||
| if (timeout) { | ||
| // Assuming, OS_CLOCK_MACH_ABSOLUTE_TIME is equivalent to CLOCK_REALTIME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this assumption due to we have not port time conversion yet in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. I couldn't find a reference stating what "absolute" means (in the macos context). Is it closer to CLOCK_REALTIME or CLOCK_MONOTONIC? The assumption is based on semantic similarity where I thought it could mean realtime more than monotonic.
| long ret = 0; | ||
| if (timeout) { | ||
| // Assuming, OS_CLOCK_MACH_ABSOLUTE_TIME is equivalent to CLOCK_REALTIME | ||
| uint64_t tnsec = timeout->get_timespec().tv_sec * 1000000000 + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try use existing unit conversion utilities if possible
| sizeof(FutexWordType), OS_SYNC_WAIT_ON_ADDRESS_NONE); | ||
| } | ||
| if ((ret < 0) && (errno == ETIMEDOUT)) { | ||
| return -ETIMEDOUT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove extra brace to conform LLVM's guide line.
| @@ -0,0 +1,78 @@ | |||
| //===--- Futex utils for Darwin ------------------------*- C++-*-===// | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The license header line length seems strange. It should be 80-characters long in total.
| @@ -0,0 +1,131 @@ | |||
| //===--- Implementation of a Darwin mutex class -----------*- C++-*-===// | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to lift POSIX/UNIX mutex to a shared design instead of making this platform specific, as most parts of the design should still be the same.
If constants/headers are different, you can add platform macro guards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood. I was uncertain as to the extent of generality we were looking for. I'll take this as a signal to make things as general as they can be (starting with moving mutex.h out of os folders) and into __support/thread/.
|
|
||
| namespace LIBC_NAMESPACE_DECL { | ||
|
|
||
| // This file is an implementation of `LIBC_NAMESPACE::mutex` for Darwin-based |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I guess this implementation can be removed as futex-like lock is now ready to use?
| ) | ||
|
|
||
| add_header_library( | ||
| clock_conversion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, it seems to me that at this stacking layer, the PR has all the clock conversion utilities ready. So there should be no special handling for timeout (e.g. the TODOs in your comments)
| @@ -0,0 +1,45 @@ | |||
| //===-- Unittests for Darwin's Mutex ------------------------------------===// | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, if futex-based mutex is now the shared implementation on both Darwin and Linux, there should be no need for a separate test file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implies tests from linux/ should be moved one dir up, is that correct?
|
|
||
| #include "include/llvm-libc-macros/error-number-macros.h" | ||
| #else // __linux__ | ||
| #elif __APPLE__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defined(__APPLE__)
| #include "llvm-libc-macros/linux/error-number-macros.h" | ||
|
|
||
| #else // __linux__ | ||
| #elif __APPLE__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defined(__APPLE__)
| add_header_library( | ||
| unix_mutex | ||
| HDRS | ||
| unix_mutex.h | ||
| DEPENDS | ||
| .raw_mutex | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this above mutex
| libc.src.__support.CPP.optional | ||
| libc.hdr.types.pid_t | ||
| COMPILE_OPTIONS | ||
| -DLIBC_COPT_RAW_MUTEX_DEFAULT_SPIN_COUNT=${LIBC_CONF_RAW_MUTEX_DEFAULT_SPIN_COUNT} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| .raw_mutex | ||
| ) | ||
|
|
||
| add_header_library( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this above unix_mutex
| libc.src.__support.time.monotonicity | ||
| libc.src.__support.CPP.optional | ||
| libc.hdr.types.pid_t | ||
| COMPILE_OPTIONS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move COMPILE_OPTIONS above DEPENDS.
| namespace LIBC_NAMESPACE_DECL { | ||
| namespace internal { | ||
|
|
||
| /** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this comment use C++'s // to match other files' style?
This patch implements the generic mutex and raw_mutex interfaces on macOS. A new Futex class is provided that relies on os_sync_wait and os_sync_wake to emulate futex‑like wait and wake semantics. The OS‑specific part is moved into futex_utils, which now contains the Darwin implementation.