[PATCH] D69582: Let clang driver support parallel jobs

[Yaxun Liu via Phabricator via cfe-commits]

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


================
Comment at: clang/lib/Driver/Compilation.cpp:332
+    if (!Next) {
+      std::this_thread::yield();
       continue;
----------------
aganea wrote:
> yaxunl wrote:
> > aganea wrote:
> > > In addition to what @thakis said above, yielding here is maybe not a good idea. This causes the process to spin, and remain in the OS' active process list, which uselessly eats cpu cycles. This can become significant over the course of several minutes of compilation.
> > > 
> > > Here's a //tiny// example of what happens when threads are waiting for something to happen:
> > > (the top parts yields frequently; the bottom part does not yield - see D68820)
> > > {F10592208}
> > > 
> > > You would need here to go through a OS primitive that suspends the process until at least one job in the pool completes. On Windows this can be achieved through `WaitForMultipleObjects()` or I/O completion ports like provided by @thakis. You can take a look at `Compilation::executeJobs()` in D52193 and further down the line, `WaitMany()` which waits for at least one job/process to complete.
> > Sorry for the delay.
> > 
> > If D52193 is commited, I will probably only need some minor change to support parallel compilation for HIP. Therefore I hope D52193 could get committed soon.
> > 
> > I am wondering what is the current status of D52193 and what is blocking it. Is there any chance to get it commited soon?
> > 
> > Thanks.
> Hi @yaxunl! Nothing prevents from finishing D52193 :-) It was meant as a prototype, but I could transform it into a more desirable state.
> I left it aside because we made another (unpublished) prototype, where the process invocations were in fact collapsed into the calling process, ie. ran in a thread pool in the manner of the recent `-fintegrated-cc1` flag. But that requires for `cl::opt` to support different contexts, as opposed to just one global state ([[ http://lists.llvm.org/pipermail/llvm-dev/2018-October/127039.html | an RFC was discussed ]] about a year ago, but there was no consensus).
> Having a thread pool instead of the process pool is faster when compiling .C/.CPP files with `clang-cl /MP`, but perhaps in your case that won't work, you need to call external binaries, do you? Binaries that are not part of LLVM? If so, then landing D52193 first would makes sense.
HIP toolchain needs to launch executables other than clang for a compilation, therefore D52193 is more relevant to us. I believe this is also the case for CUDA, OpenMP and probably more general situations involving linker. I think both parallel by threads and parallel by processes are useful. However parallel by processes is probably more generic. Therefore landing D52193 first would benefit a lot.


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

https://reviews.llvm.org/D69582