Skip to content

[libc++] Use Fuchsia futex operations #133571

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+24 −2
Open

[libc++] Use Fuchsia futex operations #133571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8026b86
[libc++] Use Fuchsia futex operations
frobtech Mar 29, 2025
ba63615
Merge branch 'main' into p/libcxx-fuchsia-futex
frobtech Jun 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libcxx/include/__atomic/contention_t.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

_LIBCPP_BEGIN_NAMESPACE_STD

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

@huixie90 huixie90 Jul 12, 2025

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

@petrhosek petrhosek Nov 12, 2025

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.

using __cxx_contention_t _LIBCPP_NODEBUG = int32_t;
#else
using __cxx_contention_t _LIBCPP_NODEBUG = int64_t;
Expand Down
2 changes: 1 addition & 1 deletion libcxx/include/__cxx03/__atomic/contention_t.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

_LIBCPP_BEGIN_NAMESPACE_STD

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

@philnik777 philnik777 Mar 29, 2025

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

@petrhosek petrhosek Nov 12, 2025

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

@ldionne ldionne Nov 14, 2025

Choose a reason for hiding this comment

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

Ping @philnik777

Copy link
Contributor

@philnik777 philnik777 Nov 17, 2025

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.

using __cxx_contention_t = int32_t;
#else
using __cxx_contention_t = int64_t;
Expand Down
22 changes: 22 additions & 0 deletions libcxx/src/atomic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Copy link
Member

@ldionne ldionne Nov 14, 2025

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.

// 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
Expand Down
Loading