[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

aganea added a comment. Hi @tycho ! Sorry for not getting back earlier. I implemented an alternate approach last year, which proved to be better (in terms of build times) than what I proposed in this demo patch. That is, using a thread pool instead of the process pool as implemented here. This

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

tycho added a comment. I did some local work to make this build and pass almost all tests on Linux as well, not to make use of the multi-process features but just to avoid having a separate "for Windows only" branch, and to ensure tests pass across platforms. Unfortunately my change is not at

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

tycho added a comment. OK, those two problems were actually easy enough to fix. diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp index 2f98cf9e04..93e974842f 100644 --- a/lib/Driver/Driver.cpp +++ b/lib/Driver/Driver.cpp @@ -3241,8 +3241,8 @@ void

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

tycho added a comment. I rebased this myself on the release_90 branch and I was pleasantly surprised that I got the merge right on the first try (?!), and it actually works well without any significant changes (other than merge conflict resolutions). I've run into two problems with it though:

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

angushewlett added a comment. In D52193#1589657 , @aganea wrote: > > In D52193#1589150 , @angushewlett > wrote: > >> They're issued in the right order, but the second doesn't wait for the first >> to

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

aganea added a comment. In D52193#1589096 , @yurybura wrote: > Do you have any plans to make this PR compatible with trunk? Now MSVC with > /MP builds much faster than clang-cl (at least 2x faster for our project)... I'll get back to this after the

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

angushewlett added a comment. In D52193#1457612 , @aganea wrote: > That is strange. As you can’t mix /Yc and /Yu on the command line, MSBuild > should issue two different calls to clang-cl, one with /Yc and one /Yu /MP. > Can you check with ProcMon that

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

yurybura added a comment. Dear @aganea, Do you have any plans to make this PR compatible with trunk? Now MSVC with /MP builds much faster than clang-cl (at least 2x faster for our project)... Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52193/new/

Re: [PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

Even if I just issue one file with /Yc and none with /Yu, it still fails. Looks like it possibly generates two processes, one to generate the pch and the other to compile and link. A temp file is generated but no final pch; an error is emitted "Unable to read pch file foo.pch". On Sun, 7 Apr

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

aganea added a comment. That is strange. As you can’t mix /Yc and /Yu on the command line, MSBuild should issue two different calls to clang-cl, one with /Yc and one /Yu /MP. Can you check with ProcMon that commands are issued in the right order? Repository: rC Clang CHANGES SINCE LAST

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

angushewlett added a comment. Having applied this patch to the clang latest (which took a bit of doing, and it's possible I messed up along the way) - all working in general, and a roughly 6x speed-up on my i7-7820X. However, PCH support does not seem to work correctly. Specifically, it looks

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

rnk added a comment. In https://reviews.llvm.org/D52193#1260933, @aganea wrote: > I wrote a fully fledged crash reporting system which does that, so I know > that's possible. The tricky thing is to ensure > `Driver::generateCompilationDiagnostics()` doesn't touch potentially invalid >

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

aganea added a comment. In https://reviews.llvm.org/D52193#1260862, @zturner wrote: > I can try to get some timings from my machine. How do we handle crash > recovery in the case where we don't spawn a child process? I thought the > whole reason for spawning the cc1 driver as a separate

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

zturner added a comment. I can try to get some timings from my machine. How do we handle crash recovery in the case where we don't spawn a child process? I thought the whole reason for spawning the cc1 driver as a separate process was so that we could collect and report crash information in a

Re: [PATCH] D52193: RFC: [clang] Multithreaded compilation support

I can try to get some timings from my machine. How do we handle crash recovery in the case where we don't spawn a child process? I thought the whole reason for spawning the cc1 driver as a separate process was so that we could collect and report crash information in a nice way. Not having that

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

aganea added a comment. Updated tests again. This time I've closed all applications, all unused services, and tried to have a "clean" machine. New timings without the child `clang-cl.exe` being invoked (hacked from https://reviews.llvm.org/D52411). The test consists in building Clang+LLVM+LLD

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

aganea added a comment. In https://reviews.llvm.org/D52193#1243456, @thakis wrote: > ...and to reword this a bit: Clang taking a long time to start up in some > configurations is a bug we should profile and fix :-) This is time spent in `ntdll.dll` loading various low-level libraries like

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

thakis added a comment. ...and to reword this a bit: Clang taking a long time to start up in some configurations is a bug we should profile and fix :-) Repository: rC Clang https://reviews.llvm.org/D52193 ___ cfe-commits mailing list

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

thakis added a comment. In https://reviews.llvm.org/D52193#1241067, @aganea wrote: > @thakis > clang-cl isn't supposed to do (explicit) registry accesses when you > hold it right (pass in -fms-compatibility-version etc). Have you seen > registry access costs, or is that speculation? > > Please

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

aganea added a comment. > clang-cl isn't supposed to do (explicit) registry accesses when you hold it > right (pass in -fms-compatibility-version etc). Have you seen registry access > costs, or is that speculation? Please see this log: F7268226: clang-cl-log.zip

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

aganea added a comment. In https://reviews.llvm.org/D52193#1241056, @zturner wrote: > The process stuff looks cool and useful independently of `/MP`. Would it be > possible to break that into a separate patch, and add a unit test for the > behavior of the `WaitAll` flag? Of course, will do.

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

zturner added a comment. The process stuff looks cool and useful independently of `/MP`. Would it be possible to break that into a separate patch, and add a unit test for the behavior of the `WaitAll` flag? Repository: rC Clang https://reviews.llvm.org/D52193

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

aganea marked an inline comment as done. aganea added a comment. It seems Reid's change has done good: `cmake` is not as significant as before in the build process. See below the improved timings: The tests consist in a full cleanup (delete build folder), cmake to regenerate, then a full

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

aganea added a comment. > clang-cl isn't supposed to do (explicit) registry accesses when you hold it > right (pass in -fms-compatibility-version etc). Have you seen registry access > costs, or is that speculation? No speculation, I will share the FileMon logs shortly. It is indirectly

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

thakis added a comment. Since it helps existing msbuild configs, adding this seems like a good thing to me. clang-cl isn't supposed to do (explicit) registry accesses when you hold it right (pass in -fms-compatibility-version etc). Have you seen registry access costs, or is that speculation?

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

aganea added a comment. In https://reviews.llvm.org/D52193#1239171, @hans wrote: > Thanks for adding the Ninja numbers. It confirms that Ninja is significantly > faster than MSBuild + /MP. It is true for the 6-cores Xeon, but not for the dual-18-cores. I think there are two issues there,

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

hans added a comment. Thanks for adding the Ninja numbers. It confirms that Ninja is significantly faster than MSBuild + /MP. Since that's the case, maybe we should be poking MS about making MSBuild faster instead of adding /MP support to Clang? Or making it easier to use Ninja in VS

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

rnk added a comment. This is pretty cool. The process launching APIs in LLVM were pretty basic, left a lot to be desired, returned ints, etc etc. This addresses a lot of that. Comment at: clang/trunk/lib/Driver/Driver.cpp:3030 +if (Arg *A =

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

zturner added a comment. In https://reviews.llvm.org/D52193#1238978, @aganea wrote: > In https://reviews.llvm.org/D52193#1238944, @zturner wrote: > > > In https://reviews.llvm.org/D52193#1238923, @aganea wrote: > > > > > The goal of this change is frictionless compilation into VS2017 when > > >

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

aganea added a comment. In https://reviews.llvm.org/D52193#1238944, @zturner wrote: > In https://reviews.llvm.org/D52193#1238923, @aganea wrote: > > > The goal of this change is frictionless compilation into VS2017 when using > > `clang-cl` as a compiler. We've realized that compiling Clang+LLD

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

zturner added a reviewer: thakis. zturner added a comment. That said, the numbers are pretty convincing These two rows alone: MSBuild, Clang + LLD(29min 12sec) 32 parallel msbuild MSBuild, Clang /MP + LLD(9min 22sec)32 parallel msbuild Are enough to make this patch worthy of

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

zturner added a comment. In https://reviews.llvm.org/D52193#1238923, @aganea wrote: > The goal of this change is frictionless compilation into VS2017 when using > `clang-cl` as a compiler. We've realized that compiling Clang+LLD with Clang > generates a faster executable that with MSVC (even

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

aganea added a comment. The goal of this change is frictionless compilation into VS2017 when using `clang-cl` as a compiler. We've realized that compiling Clang+LLD with Clang generates a faster executable that with MSVC (even latest one). I currently can't see a good way of generating the

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

zturner added a subscriber: aganea. zturner added a comment. In an ideal world yes, but the reality is that many people still use MSBuild, and in that world /MP presumably helps quite a bit. And given that many people already depend on this functionality of cl, it’s a potential showstopper for

Re: [PATCH] D52193: RFC: [clang] Multithreaded compilation support

In an ideal world yes, but the reality is that many people still use MSBuild, and in that world /MP presumably helps quite a bit. And given that many people already depend on this functionality of cl, it’s a potential showstopper for migrating if we don’t support it. That said, if the benefit

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

hans added a comment. In https://reviews.llvm.org/D52193#1237468, @zturner wrote: > What about the timings of clang-cl without /MP? And one using Ninja rather than msbuild. I think the real question is whether we want clang and clang-cl to do this. I'm not sure we do as it adds complexity

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

zturner added a reviewer: rsmith. zturner added a comment. What about the timings of clang-cl without /MP? Repository: rC Clang https://reviews.llvm.org/D52193 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52193: RFC: [clang] Multithreaded compilation support

aganea created this revision. aganea added reviewers: rnk, hans, zturner. Herald added a reviewer: JDevlieghere. Herald added a subscriber: cfe-commits. This is very preliminary change which adds support for (`clang-cl`) `/MP` (Build with multiple processes). Doc for `/MP` is here