[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

aganea updated this revision to Diff 230143. aganea marked 10 inline comments as done. aganea edited the summary of this revision. aganea added a comment. Addressed @hans' comments. I've also added a script in the summary, if you'd like to reproduce the results on your end, and to ensure we all

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

aganea added inline comments. Comment at: clang/lib/Driver/Job.cpp:347 +StringRef DriverExe = llvm::sys::path::stem(D.ClangExecutable); +if (CommandExe.equals_lower(DriverExe)) +CC1Main = Driver::CC1Main; hans wrote: > Now that we're not calling

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

hans added a comment. In D69825#1747539 , @aganea wrote: > @hans : Simply because `ExecuteCC1Tool()` lives in the clang tool > (`clang/tools/driver/driver.cpp`) and we're calling it from the > clangDriver.lib (`clang/lib/Driver/Job.cpp`). The

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

aganea added a comment. @hans : Simply because `ExecuteCC1Tool()` lives in the clang tool (`clang/tools/driver/driver.cpp`) and we're calling it from the clangDriver.lib (`clang/lib/Driver/Job.cpp`). The clangDriver.lib is linked into many other tools (clang-refactor, clang-rename, clang-diff,

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

hans added a comment. Thanks for the update! I think calling ExecuteCC1Tool instead of main is an improvement (there's no need to think about "re-entering" the process, it's really just a function call). I still don't see why it needs to go through a function pointer. I was hoping the code

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

aganea updated this revision to Diff 229333. aganea marked an inline comment as done. aganea added a comment. Call into `ExecuteCC1Tool()` directly, as suggested by @hans Add more comments. Remove `pThis` in `CrashRecoveryContext.cpp:RunSafely()` as suggested by @zturner CHANGES SINCE LAST

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

aganea marked an inline comment as done. aganea added a comment. In D69825#1742276 , @hans wrote: > Instead of invoking main() (or a similar function like ClangDriverMain) as an > alternative to spinning up the cc1 process, would it be possible to call

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

zturner added a comment. Does this change crash recovery semantics in any meaningful way? Will we still be able to get stack traces on all platforms when the compiler crashes? Comment at: llvm/lib/Support/CrashRecoveryContext.cpp:207 + // FIXME error: cannot compile this

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

Meinersbur added inline comments. Comment at: clang/lib/Driver/Job.cpp:347 +StringRef DriverExe = llvm::sys::path::stem(D.ClangExecutable); +if (CommandExe.equals_lower(DriverExe)) + ClangDriverMain = Driver::Main; Why is this check necessary? Why

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

hans added a comment. Thanks for sharing this! I still need to take a proper look, but my first thought is this: Instead of invoking main() (or a similar function like ClangDriverMain) as an alternative to spinning up the cc1 process, would it be possible to call ExecuteCC1Tool() directly? I

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

aganea updated this revision to Diff 228729. aganea marked 2 inline comments as done. aganea added a comment. Addressed comments & finished the Linux part. All tests pass. @Meinersbur : Just to show the difference between Windows & Linux, here's some timings for running the tests over the same

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

aganea added a comment. In D69825#1733958 , @thakis wrote: > Thanks for sending this out! > > Instead of the dynamic lookup of that symbol, what do you think about passing > in the function via a normal api? That way, the type checker and linker help >

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

aganea added a comment. Thanks for the feedback @Meinersbur! This patch is mainly geared towards Windows users. I'm not expecting anything on Linux, you already have all the bells & whistles there :-) Although I definitely see improvements on my Linux box. Would the distro make a difference?

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

thakis added a comment. Thanks for sending this out! Instead of the dynamic lookup of that symbol, what do you think about passing in the function via a normal api? That way, the type checker and linker help us keep things working; dynamic lookup is always a bit subtle and hard to grep for.

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

Meinersbur added inline comments. Comment at: clang/lib/Driver/Job.cpp:340-355 + typedef int (*ClangDriverMainFunc)(SmallVectorImpl &); + ClangDriverMainFunc ClangDriverMain = nullptr; + + if (ReenterTool) { +StringRef F = llvm::sys::path::filename(Executable); +if

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

Meinersbur added a comment. This patch reduced the build time from 12 to 7 minutes? Sounds too good to be true. Thanks for the work! I would primarily interested in this to avoid determining the `-cc1` command line when debugging. However, the implementation of calling it's own main function

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

aganea marked 3 inline comments as done. aganea added a comment. A few remarks: Comment at: clang/lib/Driver/Job.cpp:319 +LLVM_THREAD_LOCAL bool Command::ReenterTool = true; + See my other comment about `LLVM_THREAD_LOCAL` Comment at:

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

aganea updated this revision to Diff 227779. aganea added a comment. Herald added a subscriber: dexonsmith. Fix missing `llvm::InitializeAllTargets()` in driver.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69825/new/

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

aganea created this revision. aganea added reviewers: thakis, hans, rnk. aganea added a project: clang. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added a reviewer: jfb. Herald added a project: LLVM. This patch is an optimization for speed: it will bypass the cc1