[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