[PATCH] D33024: [Support] Move LLD parallel algorithms to LLVM.

Zachary Turner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 9 19:33:12 PDT 2017


zturner added inline comments.


================
Comment at: lld/COFF/Writer.cpp:748
       memset(SecBuf, 0xCC, Sec->getRawSize());
-    for_each(parallel::par, Sec->getChunks().begin(), Sec->getChunks().end(),
-             [&](Chunk *C) { C->writeTo(SecBuf); });
+    for_each(llvm::parallel::par, Sec->getChunks().begin(),
+             Sec->getChunks().end(), [&](Chunk *C) { C->writeTo(SecBuf); });
----------------
chandlerc wrote:
> I find the fact that `for_each` is now getting looked up via ADL on the `llvm::parallel::par` type ... disturbing.
I can certainly qualify the function call, but I don't know of a way to disable ADL if that's what you're suggesting.


================
Comment at: llvm/include/llvm/Support/TaskGroup.h:51-53
 /// \brief Allows launching a number of tasks and waiting for them to finish
 ///   either explicitly via sync() or implicitly on destruction.
 class TaskGroup {
----------------
chandlerc wrote:
> I thought all of this was being moved to be implementation details of the algorithms?
> 
> Also, all the code appears to still be LLD style?
The executor stuff was all moved to an anonymous namespace inside of `TaskGroup.cpp`.  `TaskGroup` cannot enjoy the same level of hiding because it needs to be usable from within a header file.  Namely, `Parallel.h`.  I can, however, move it to a detail namespace inside of `Parallel.h` if you prefer that.

also: Yes rui pointed out the names in a previous patch and I forgot to update them.  All of them have been fixed except for the few instances in this file.


https://reviews.llvm.org/D33024





More information about the llvm-commits mailing list