[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-05-22 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment. In D74447#1872211 , @thakis wrote: > I'd think that everyone debugging clang always passes a single TU to it, so > I'm not sure debuggability does much here :) > > The `-disable-free` code has never been used in normal compilations,

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-02-13 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. I forgot another reason why "only do cc1 with a single TU" might be a good idea: In case crash recovery turns out to not work in all cases with in-process cc1, we could add a signal handler that on crash make the driver exec() itself with -fno-integrated-cc1 added, so th

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-02-12 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added inline comments. Comment at: clang/include/clang/Driver/Job.h:90 + /// Whether the command will be executed in this process or not. + bool InProcess : 1; + hans wrote: > I think Reid just meant put the bool fields after each other to minimize > pa

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-02-12 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 244172. aganea marked 7 inline comments as done. aganea added a comment. Address @hans' comments. While it'd be good to have a bot running without `-disable-free`, I concur with @thakis. This patch will probably introduce too much (untested) variability for

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-02-12 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. The performance benefit of disable-free is still valuable, I'd not want to make every TU pay the price for a multiple TU invocation. Additionally, if we're willing to put up with the 3rd form (which i think is worth it), the multiple TU case cleaning up on all but th

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-02-12 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. I'd think that everyone debugging clang always passes a single TU to it, so I'm not sure debuggability does much here :) The `-disable-free` code has never been used in normal compilations, so we didn't have to worry about this path. This patch here brings us to 3 modes

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-02-12 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment. In D74447#1872043 , @thakis wrote: > Isn't a better approach to only do in-process cc1 if there's just one TU? One of the reasons of the in-process cc1 was the debuggability. If we can compile several TUs in-process, why not doin

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-02-12 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. Isn't a better approach to only do in-process cc1 if there's just one TU? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74447/new/ https://reviews.llvm.org/D74447 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-02-12 Thread Hans Wennborg via Phabricator via cfe-commits
hans added inline comments. Comment at: clang/include/clang/Driver/Job.h:90 + /// Whether the command will be executed in this process or not. + bool InProcess : 1; + I think Reid just meant put the bool fields after each other to minimize padding. Using bitfi

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-02-11 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 244055. aganea marked 3 inline comments as done. aganea added a comment. As suggested by Reid. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74447/new/ https://reviews.llvm.org/D74447 Files: clang/include/clang/Driver/Job.h clang/lib/Driver/Driv

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-02-11 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: clang/include/clang/Driver/Job.h:59 /// Whether to print the input filenames when executing. bool PrintInputFilenames = false; Pack a new field after an existing bool for maximum micro optimization. :)

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-02-11 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea updated this revision to Diff 244028. aganea edited the summary of this revision. aganea added a comment. Wording. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74447/new/ https://reviews.llvm.org/D74447 Files: clang/include/clang/Driver/Job.h clang/lib/Driver/Driver.cpp c

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-02-11 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision. aganea added reviewers: hans, thakis, rnk, erichkeane. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. A `CC1Command` was previously always burying pointers (ie. skipping object deletion). This lead to higher memory usage scenarios,