[PATCH] D60758: Add an assertion that parallelForEach doesn't nest.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/Common/Threads.cpp:13
 bool lld::ThreadsEnabled = true;
+thread_local bool lld::ParallelForEachRunning;
----------------
MaskRay wrote:
> This doesn't have to be `thread_local`. This is a fundamental problem of `TaskGroup`.
> 
> ```
> template <class IterTy, class FuncTy>
> void parallel_for_each(IterTy Begin, IterTy End, FuncTy Fn) {
>   TaskGroup TG;
> .... // In ~Latch called by ~TaskGroup, all threads of the default executor can block on different `std::mutex`es if there are multiple concurrent `parallel_for_each` invocations.
> }
> ```
> 
> It doesn't need nested `parallel_for_each` to get stuck. It just needs more concurrent `parallel_for_each` invocations than the number of executors.
The code (with `thread_local bool` -> `std::atomic<bool>`) can probably be moved to `include/llvm/Support/Parallel.h` before it gets improved to allow concurrent invocations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60758