[PATCH] D90639: Add parallelTransformReduce and parallelForEachError

[River Riddle via Phabricator via llvm-commits]

rriddle accepted this revision.
rriddle added a comment.
This revision is now accepted and ready to land.

Nice, this looks like a good addition. There's already at least one place in MLIR that I could see this used as well.



================
Comment at: llvm/include/llvm/Support/Parallel.h:169
+  // improving to take the number of available cores into account.)
+  size_t NumInputs = std::distance(Begin, End);
+  size_t NumTasks = std::min(static_cast<size_t>(1024), NumInputs);
----------------
Can you check for the case where `NumInputs == 0` and return Init? Any reason that we should require that there is always at least one task?


================
Comment at: llvm/include/llvm/Support/Parallel.h:193
+  // reductions are cheaper than the transformation.
+  for (auto &R : Results)
+    Init = Reduce(Init, std::move(R));
----------------
We could strip the first reduce given that we know that there will be at least one task, but I suppose the reduce function won't be costly in most cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90639