Skip to content

Conversation

@frobtech
Copy link
Contributor

@frobtech frobtech commented Mar 29, 2025

The basic futex operations have always been available on Fuchsia.
Wire them up to properly support C++20 atomic notify_*/wait.

@frobtech frobtech requested a review from petrhosek March 29, 2025 07:59
@frobtech frobtech marked this pull request as ready for review March 29, 2025 08:03
@frobtech frobtech requested a review from a team as a code owner March 29, 2025 08:03
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 29, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 29, 2025

@llvm/pr-subscribers-libcxx

Author: Roland McGrath (frobtech)

Changes

The basic futex operations have always been available on Fuchsia.
Wire them up to properly support C++20 atomic notify_*/wait.


Full diff: https://github.com/llvm/llvm-project/pull/133571.diff

3 Files Affected:

  • (modified) libcxx/include/__atomic/contention_t.h (+1-1)
  • (modified) libcxx/include/__cxx03/__atomic/contention_t.h (+1-1)
  • (modified) libcxx/src/atomic.cpp (+22)
diff --git a/libcxx/include/__atomic/contention_t.h b/libcxx/include/__atomic/contention_t.h
index 5b42a0125f875..cd3a8e4b29094 100644
--- a/libcxx/include/__atomic/contention_t.h
+++ b/libcxx/include/__atomic/contention_t.h
@@ -19,7 +19,7 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-#if defined(__linux__) || (defined(_AIX) && !defined(__64BIT__))
+#if defined(__linux__) || defined(__Fuchsia__) || (defined(_AIX) && !defined(__64BIT__))
 using __cxx_contention_t _LIBCPP_NODEBUG = int32_t;
 #else
 using __cxx_contention_t _LIBCPP_NODEBUG = int64_t;
diff --git a/libcxx/include/__cxx03/__atomic/contention_t.h b/libcxx/include/__cxx03/__atomic/contention_t.h
index a97f0668da2fe..3fc3ccef78df5 100644
--- a/libcxx/include/__cxx03/__atomic/contention_t.h
+++ b/libcxx/include/__cxx03/__atomic/contention_t.h
@@ -19,7 +19,7 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-#if defined(__linux__) || (defined(_AIX) && !defined(__64BIT__))
+#if defined(__linux__) || defined(__Fuchsia__) || (defined(_AIX) && !defined(__64BIT__))
 using __cxx_contention_t = int32_t;
 #else
 using __cxx_contention_t = int64_t;
diff --git a/libcxx/src/atomic.cpp b/libcxx/src/atomic.cpp
index c1af8d6f95aae..6de355b534b08 100644
--- a/libcxx/src/atomic.cpp
+++ b/libcxx/src/atomic.cpp
@@ -41,6 +41,10 @@
 // OpenBSD has no indirect syscalls
 #  define _LIBCPP_FUTEX(...) futex(__VA_ARGS__)
 
+#elif defined(__Fuchsia__)
+
+#  include <zircon/syscalls.h>
+
 #else // <- Add other operating systems here
 
 // Baseline needs no new headers
@@ -101,6 +105,24 @@ static void __libcpp_platform_wake_by_address(__cxx_atomic_contention_t const vo
   _umtx_op(const_cast<__cxx_atomic_contention_t*>(__ptr), UMTX_OP_WAKE, __notify_one ? 1 : INT_MAX, nullptr, nullptr);
 }
 
+#elif defined(__Fuchsia__)
+
+static inline zx_futex_t const* __libcpp_zx_futex(__cxx_atomic_contention_t const volatile* ptr) {
+  // Implicitly link against the vDSO system call ABI without requiring the
+  // final link to specify -lzircon explicitly when statically linking libc++.
+#  pragma comment(lib, "zircon")
+  return const_cast<zx_futex_t const*>(reinterpret_cast<zx_futex_t const volatile*>(&ptr->__a_value));
+}
+
+static void
+__libcpp_platform_wait_on_address(__cxx_atomic_contention_t const volatile* __ptr, __cxx_contention_t __val) {
+  _zx_futex_wait(__libcpp_zx_futex(__ptr), __val, ZX_HANDLE_INVALID, ZX_TIME_INFINITE);
+}
+
+static void __libcpp_platform_wake_by_address(__cxx_atomic_contention_t const volatile* __ptr, bool __notify_one) {
+  _zx_futex_wake(__libcpp_zx_futex(__ptr), __notify_one ? 1 : UINT32_MAX);
+}
+
 #else // <- Add other operating systems here
 
 // Baseline is just a timed backoff

_LIBCPP_BEGIN_NAMESPACE_STD

#if defined(__linux__) || (defined(_AIX) && !defined(__64BIT__))
#if defined(__linux__) || defined(__Fuchsia__) || (defined(_AIX) && !defined(__64BIT__))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't modify the C++03 headers for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philnik777 Is this still an issue?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine now. The large refactorings have been done and I don't think I'll touch contention_t.h.

@huixie90 huixie90 self-assigned this Jun 28, 2025
_LIBCPP_BEGIN_NAMESPACE_STD

#if defined(__linux__) || (defined(_AIX) && !defined(__64BIT__))
#if defined(__linux__) || defined(__Fuchsia__) || (defined(_AIX) && !defined(__64BIT__))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be bit unfortunate, but isn’t this an ABI break for your platform ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not really an issue on Fuchsia.

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM in principle but we will need to coordinate with #161086. I'll let @huixie90 chime in, please wait for his approval before merging. He'll decide whether he wants to land #161086 first and have this rebased, or the other way around.


#elif defined(__Fuchsia__)

static inline zx_futex_t const* __libcpp_zx_futex(__cxx_atomic_contention_t const volatile* ptr) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static inline zx_futex_t const* __libcpp_zx_futex(__cxx_atomic_contention_t const volatile* ptr) {
static inline zx_futex_t const* __get_zx_futex(__cxx_atomic_contention_t const volatile* ptr) {

Let's move away from __libcpp_ prefixes everywhere. I know it's confusing because we have a lot of them, but they shouldn't exist in most cases.

_LIBCPP_BEGIN_NAMESPACE_STD

#if defined(__linux__) || (defined(_AIX) && !defined(__64BIT__))
#if defined(__linux__) || defined(__Fuchsia__) || (defined(_AIX) && !defined(__64BIT__))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@huixie90
Copy link
Member

This LGTM in principle but we will need to coordinate with #161086. I'll let @huixie90 chime in, please wait for his approval before merging. He'll decide whether he wants to land #161086 first and have this rebased, or the other way around.

@

This LGTM in principle but we will need to coordinate with #161086. I'll let @huixie90 chime in, please wait for his approval before merging. He'll decide whether he wants to land #161086 first and have this rebased, or the other way around.

@petrhosek let me know your plan. There will be some merge conflicts. My patch basically allow more types to going directly to the platform api under the unstable ABI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants