[libcxx-commits] [PATCH] D110349: [libcxx][SystemZ][z/OS] Added is_threading_api_enabled and might_have_multiple_threads to __threading_support

[Daniel McIntosh via Phabricator via libcxx-commits]

DanielMcIntosh-IBM added a comment.

In D110349#3126010 <https://reviews.llvm.org/D110349#3126010>, @ldionne wrote:

> In D110349#3116828 <https://reviews.llvm.org/D110349#3116828>, @DanielMcIntosh-IBM wrote:
>
>> In D110349#3116616 <https://reviews.llvm.org/D110349#3116616>, @ldionne wrote:
>>
>>> Is there a reason why the underlying C library can't implement e.g. `mtx_lock` as a no-op if threading is disabled at runtime?
>>
>> I don't know if we could have the underlying `pthread_mutex_lock` and friends act as a no-op like you've described (it's managed by a separate team), but I strongly suspect it's not an option, as that could affect the behaviour of existing applications on z/OS.
>
> I don't understand -- if it's correct to do it for libc++, it should be correct to do it for the C library too, no? If a program is single-threaded, I don't understand why the C library couldn't apply the same optimizations that you're trying to add to libc++, i.e. make mutexes no-ops and so on.

As far as I can tell, the only parts of the C library that need to perform any kind of thread synchronization are the ones that the user would use when they themselves are performing thread synchronization (e.g. `mtx_lock`).
The places we're making changes are places where the user could run into thread synchronization code, even if the didn't call any functions related to threading. Each place we're making a change either doesn't have a C equivalent, or their C equivalent doesn't need to worry about thread safety.

- Exception storage (D113054 <https://reviews.llvm.org/D113054>): C doesn't have exceptions
- `__call_once` (D113058 <https://reviews.llvm.org/D113058>): This is needed because `locale::id::__get()` uses `call_once`. C makes concurrent use/modification of the current locale undefined behaviour.
- LIBCPP debug (D113065 <https://reviews.llvm.org/D113065>): No C equivalent
- Shared ptr std::atomic... overloads (D113066 <https://reviews.llvm.org/D113066>): C doesn't have shared pointers
- random_shuffle (D113069 <https://reviews.llvm.org/D113069>): C doesn't provide any functions/algorithms as complex as random_shuffle
- fallback malloc (D112567 <https://reviews.llvm.org/D112567>): This is used for exception handling so they still function in the out-of-memory case. C doesn't have exceptions.
- static local variables (D110351 <https://reviews.llvm.org/D110351>): In C, static local variables are initialized before main(), so a) we don't need to keep track of whether the initialization has been performed yet and b) the initialization all happens in a single-threaded environment anyways

The goal isn't performance optimization, but to address the places in libc++ where threading code has leaked into code unrelated to the threading library. The issue is that z/OS users still need to be able to use those parts of the library when posix is disabled, but we can't completely strip out the synchronization code at compile time like _LIBCPP_HAS_NO_THREADS does because that synchronization code is needed when for when posix ISN'T disabled. Thus, we need to gate the synchronization code behind a run-time check of the availability the threading api (which is what `__libcpp_is_threading_api_enabled` is for).

`__libcpp_is_threading_api_enabled` is sufficient on its own to fix this problem, and we could stop there. 
However, in most cases, if we have to gate the synchronization code behind a runtime check anyways, we might as well go one step further and use `__libcpp_might_have_multiple_threads` instead. The notable exception where I couldn't use `__libcpp_might_have_multiple_threads` is D113054 <https://reviews.llvm.org/D113054>.

On a related note, AIX will have the same issue with the availability of the threading library being unknown at compile time, and unlike z/OS, they do not have any equivalent of `__libcpp_is_threading_api_enabled`, so we've been talking internally about how to address the places where I used `__libcpp_is_threading_api_enabled`. This may result in updates that make it even less obvious that these changes are not about optimizations to libc++.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110349/new/

https://reviews.llvm.org/D110349