[PATCH] D148916: [Support][Parallel] Initialize threadIndex and add assertion checking its usage.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

In D148916#4295882 <https://reviews.llvm.org/D148916#4295882>, @lattner wrote:

> I haven't looked in detail at this patch, but I can describe the problem that I was hitting.  The situation was happening in MLIR and other clients that have an expose nested parallelism, e.g. in its parallel pass manager.  The thread pool at the time refused to do nested cases in parallel, so if you did:
>
>   parallel for x in ...
>     parallel for y in ...
>        work
>
> it would do the outer loop in parallel and then refuse to do the inner loop in parallel.  This was sort of ok, but was really really bad when the outer loop actually had only a single entry.  That would end up running the inner loop in series even though there was no parallelism in play.
>
> The change to do 0/1 without "parallelism" led to a 50x speedup on a CIRCT workload.  It also eliminates a bit of fixed overhead in those small cases, but that is a more minor issue.
>
> -Chris

I suggest to use another approach to have more parallelism:

  std::function<void()> Fn = [&]() {
    parallel for y in ...
  };
  
  ThreadPool Pool;
  
  for x in ...
    Pool.async(Fn);
  
  Pool.wait();

What do you think of it? it could gave more benefits than  0/1 approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148916