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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu marked an inline comment as done.
ruiu added inline comments.


================
Comment at: lld/Common/Threads.cpp:13
 bool lld::ThreadsEnabled = true;
+thread_local bool lld::ParallelForEachRunning;
----------------
MaskRay wrote:
> 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.
Do you think you can fix the function so that parallel_for_each is reentrant? If so, do you mind if I ask you do that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60758