Page MenuHomePhabricator

[libcxx] Use C11 thread API on Fuchsia
Needs ReviewPublic

Authored by phosek on Jul 8 2019, 4:12 PM.

Details

Reviewers
EricWF
Summary

On Fuchsia, pthread API is emulated on top of C11 thread API. Using C11
thread API directly is more efficient.

While this implementation is only used by Fuchsia at the moment, it's
not Fuchsia specific (except for the MTX_INIT and CND_INIT macros which
are Fuchsia extensions), and could be used by other platforms that use
C11 threads rather than pthreads in the future.

Diff Detail

Repository
rCXX libc++

Event Timeline

phosek created this revision.Jul 8 2019, 4:12 PM
jfb added inline comments.Jul 8 2019, 4:30 PM
libcxx/include/__threading_support
31

What does this do? Docs say "Enable extensions for AIX 3, and for Interix."

None of these calls (__libcpp_tls_create, for instance) are templates, or marked as inline candidates.
Should they be in a source file, and baked into the dylib instead of a header file?
(like we do in support/win32/thread_win32.cpp, for example)

EricWF added a comment.Jul 8 2019, 7:52 PM

If this is using the C11 threading API and isn't Fuchsia specific, then would the macro name reflect that?
i.e. _LIBCPP_HAS_THREAD_API_C11?

libcxx/include/__threading_support
31

What happens when threads.h is included before any libc++ headers?
We might have to lift this define into the compiler, like we've had to do with _GNU_SOURCE.

EricWF added inline comments.Jul 8 2019, 7:55 PM
libcxx/include/__config
1077

Should this not also check for __has_include(<threads.h>)?

EricWF added inline comments.Jul 8 2019, 7:59 PM
libcxx/include/__config
1068

Switching the implementation used by Fuchsia is ABI breaking. Are we 100% certain that all of our Fuchsia users don't expect ABI stability?

1106

Can you point towards the C11 source for mtx_destroy and cnd_destroy?

EricWF added inline comments.Jul 8 2019, 8:24 PM
libcxx/include/__config
1115–1116

After switching to the _LIBCPP_HAS_THREAD_API_C11 this check should include _LIBCPP_HAS_MUSL_LIBC, because AFAIK, the trivial destruction property is an implementation detail of MUSL.

libcxx/include/__threading_support
31

After some searching, I'm assuming the libc Fucshia uses is MUSL?

If so, then MTX_INIT is defined as simply {}. Writing that in source is going to be less problematic than ensuring _ALL_SOURCE is always defined.

564

Just use const here, it has the same effect but it works in C++03.

phosek updated this revision to Diff 208862.Jul 9 2019, 6:33 PM
phosek marked 11 inline comments as done.
phosek added inline comments.
libcxx/include/__config
1068

Yes, that's fundamental part of Fuchsia's packaging story.

1106

I've added guard for Fuchsia since this is implementation of details our libc.

1115–1116

I've instead added a check for defined(__Fuchsia__), Fuchsia's libc started as a fork of musl but it has sufficiently diverged at this point that I don't think it's safe to consider it being musl-compatible.

Ping? Anything else you'd like me to change?

@EricWF friendly ping?