[libcxx-commits] [PATCH] D113069: [libcxx][SystemZ][z/OS] Update libcxx/src/random_shuffle.cpp to accommodate POSIX(OFF)

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/src/random_shuffle.cpp:28
 #ifndef _LIBCPP_HAS_NO_THREADS
+  if (__libcpp_are_threads_enabled())
     __libcpp_mutex_lock(&__rs_mut);
----------------
DanielMcIntosh-IBM wrote:
> Quuxplusone wrote:
> > jroelofs wrote:
> > > should this guard go inside `__libcpp_mutex_{,un}lock` instead?
> > (This function comes from parent revision D110349)
> > FWLIW, I think yes it should.
> I had considered doing that, but don't think it's a good idea because 
> 1. it would result in a small performance hit in a lot of places
> 2. since `cxa_guard` (which manages initialization of static local variables) uses `__libcpp_mutex_{,un}lock`, it would cause infinite recursion (`__libcpp_mutex_{,un}lock` -> `__libcpp_is_threading_api_enabled()` -> `cxa_guard` -> `__libcpp_mutex_{,un}lock`). This loop could be broken by getting rid of the static local variable in `__libcpp_is_threading_api_enabled()`, but I think that is a significantly worse option. Also, getting rid of the static local variable makes the performance hit in 1) a lot bigger
Again FWLIW: Makes sense to me. :)
I briefly wondered if `constinit` could help with the initialization in D110349, but no, it can't. Although... maybe you could use `atomic`? or is that a case of "now you have two problems"? ;) https://godbolt.org/z/objbvvdx3 In the thread-safe-static-initialization case, notice that declaring `__gtca()` as `noexcept` improves codegen, because then Clang doesn't have to codegen a "failure" path for the initialization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113069