[PATCH] D79390: [Support] Sink LLD's parallel algorithm wrappers to support

[Reid Kleckner via Phabricator via llvm-commits]

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


================
Comment at: lld/COFF/LLDMapFile.cpp:77
   std::vector<std::string> str(syms.size());
-  parallelForEachN((size_t)0, syms.size(), [&](size_t i) {
+  parallel::for_each_n(parallel::par, (size_t)0, syms.size(), [&](size_t i) {
     raw_string_ostream os(str[i]);
----------------
aganea wrote:
> MaskRay wrote:
> > Isn't `parallel::par` redundant? Can we just use
> > 
> > `parallel::for_each_n((size_t)0, syms.size(), [&](size_t i) { ... })` ?
> +1
IMO yes, but I think these APIs are designed to be drop-in replaceable by the parallel STL APIs standardized in C++17, and those take a policy. I think the idea is that there could be other policies for GPUs, accelerators, SIMD variants, etc, although I'm not super familiar with them.

I peaked at llvm-project/pstl/include/internal/glue_algorithm_impl.h, which declares the C++17 standard APIs, and they are already slightly different from what LLVM has here. (No range wrappers, for_each_n does not take size_t.) If we are going to be different, we might as well simplify the API. I will remove the policy parameter and reupload this change. I believe there are only two other in-tree clients of this API in MLIR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79390