[PATCH] D15390: [ThinLTO] Launch importing backends in parallel threads from gold plugin

Peter Collingbourne via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 12 14:22:57 PST 2016


pcc added inline comments.

================
Comment at: tools/gold/gold-plugin.cpp:1019
@@ +1018,3 @@
+    raw_svector_ostream BCOS(BCs.back());
+    WriteBitcodeToFile(MPart.get(), BCOS);
+  });
----------------
joker.eph wrote:
> joker.eph wrote:
> > pcc wrote:
> > > I don't think thread pools are necessary for split code gen, as we can already perfectly assign the right amount of work to individual threads. Also, this implementation loses the pipelining feature from the original code (i.e. worker threads can work on codegen'ing while the main thread is still splitting). I would prefer you to use the existing implementation in `llvm/CodeGen/ParallelCG.h`.
> > The thread that is doing the splitting can issue other jobs to the thread pool, providing the desired pipeline.
> > 
> > Just fuse the loop body below within the lambda...
> I'll add that while the pooling is not necessary if you only queue as many jobs as you have threads, it is not a reason by itself not to use it: the paradigm is fairly clear, and it decouples the actual splitting granularity from the number of actual worker threads, allowing to experiment with different numbers for each (providing better pipelining for instance).
Yes, but the current implementation doesn't need any of that. If we experimentally find that decoupling would provide some benefit, then by all means we can start using thread pools here.

In any case, if there is a compelling reason to use thread pools, the right place to make the change is in `lib/CodeGen/ParallelCG.cpp` rather than in a duplicate implementation here. We can defer what the design for that should look like simply by not using thread pools yet.


http://reviews.llvm.org/D15390





More information about the llvm-commits mailing list