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

[Daniel McIntosh via Phabricator via libcxx-commits]

DanielMcIntosh-IBM 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);
----------------
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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113069