[libcxx-commits] [PATCH] D120348: [libcxx][SystemZ][ POSIX(OFF) support on z/OS

[Daniel McIntosh via Phabricator via libcxx-commits]

DanielMcIntosh-IBM added inline comments.


================
Comment at: libcxx/src/locale.cpp:730
 {
+#ifndef _LIBCPP_HAS_NO_THREADS
+    if (!__libcpp_is_threading_api_enabled()) {
----------------
EricWF wrote:
> I would prefer we just use `call_once` in all cases. Why can't we do that?
> Presumably `call_once` needs to work when you have runtime enable/disabled threads?
> 
`call_once` doesn't need to work when you threads have been disabled at runtime. As I discuss in D117372 it's actually kind of foolish to use call_once when threads have been disabled.

We could still support it, but we'd have to make changes to `std::__call_once`. That is an option, and it's what I went with in V1 and V2, but as I discuss in D117372, I think the better option is to eventually stop using `call_once` or any other parts of the C++11 Thread Support Library in the rest of libc++.


================
Comment at: libcxxabi/src/cxa_exception_storage.cpp:96
+        // If threads are disabled at runtime, revert to single-threaded implementation.
+        if (!__libcpp_is_threading_api_enabled()) {
+            static __cxa_eh_globals eh_globals;
----------------
EricWF wrote:
> 
> This seems like performance sensitive code. Are we cool with a branch here?
> 
> Also, are we even allowed to use function local statics in this function? 
> 
On platforms without runtime-dependent threading, `__libcpp_is_threading_api_enabled()` is constexpr, so this should almost always be optimized out (and every compiler I can find will do it even on `-O0`).

I don't see why we couldn't use function local statics here? We use them when `HAS_THREAD_LOCAL` is defined (though they are `thread_local` there).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120348