[PATCH] D69959: [C-index] Fix test when using Debug target & MSVC STL

aganea marked an inline comment as done. aganea added inline comments. Comment at: llvm/include/llvm/ADT/SmallVector.h:905 + + const SmallVector &operator=(const std::vector &Vec) { +this->assign(Vec.begin(), Vec.end()); dblaikie wrote: > aganea wrote: > > r

[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 ACTI

[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] D69959: [C-index] Fix test when using Debug target & MSVC STL

This revision was automatically updated to reflect the committed changes. Closed by commit rGf55cd39f1913: [C-index] Fix test when using Debug target & MSVC STL (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69959/new/ https://r

[PATCH] D69586: [profile] Support online merging with continuous sync mode

aganea added inline comments. Comment at: compiler-rt/lib/profile/InstrProfilingFile.c:419 + * the open file object \p File. */ +static int writeProfileWithFileObject(const char *Filename, FILE *File) { + setProfileFile(File); These two functions are not used wh

[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 m

[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] D70467: [Distro] Bypass distro detection on Windows

aganea created this revision. aganea added reviewers: sylvestre.ledru, rnk. Herald added a project: clang. Herald added a subscriber: cfe-commits. This skips distro detection on Windows, thus saving a few `stat` calls for each invocation of `clang.exe`. Repository: rG LLVM Github Monorepo ht

[PATCH] D70149: [C-index] Fix annotate-deep-statements test in Debug target

aganea added a comment. Thanks Reid! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70149/new/ https://reviews.llvm.org/D70149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D70467: [Distro] Bypass distro detection on Windows

aganea added a comment. In D70467#1752611 , @rnk wrote: > Hm, I guess it does happen. I think that condition should be restructured to > only do all that BSD, PS4, Android, Gentoo etc logic if the format is ELF, if > COFF, then always default to -faddrsi

[PATCH] D70467: [Distro] Bypass distro detection on Windows

aganea added a comment. Actually, I'm not sure the `DetectDistro()` does what it intends to do when cross-compiling: if I compile on Ubuntu and I forcibly specify `-target x86_64-linux` on the cmd-line, should it detect which Linux distro I'm on? Shouldn't it use the target triple, not the host

[PATCH] D70467: [Distro] Bypass distro detection on Windows

aganea updated this revision to Diff 230525. aganea added a comment. Removed `#ifdef _WIN32` Use the target triple where possible to detect the distro. The Cuda installation detector uses the host triple, in order to use the host tooling. Skip distro detection when the target or host system is no

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

aganea updated this revision to Diff 230527. aganea marked 3 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69825/new/ https://reviews.llvm.org/D69825 Files: clang/include/clang/Driver/Driver.h clang/include/clang/Driver/Job.h clang/lib/Driver/Job.cpp clan

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

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: > aganea wrote: > > hans wrote

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

aganea added a comment. I've split the `CrashRecoveryContext` changes into another patch to minimize the risks, please see D70568 . That patch will need to go first before this one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69825/new/ https://revie

[PATCH] D70568: [Support] Possibly use exception handler in the Crash Recovery Context in the same way as global exceptions

aganea created this revision. aganea added reviewers: hans, rnk, thakis. Herald added subscribers: llvm-commits, cfe-commits, jfb, arphaman, hiraditya. Herald added a reviewer: jfb. Herald added projects: clang, LLVM. aganea edited the summary of this revision. Herald added a subscriber: dexonsmith

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

aganea added a comment. Thanks for the feedback Russell! Can you possibly try again with `/MT`? (ie. `-DLLVM_USE_CRT_RELEASE=MT`) In the `abba_test.ps1` script, `ninja check-all` is only used for preparing clang.exe with the patch. The A/B loop //does not// use `check-all`. I've modified the `

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

aganea added a comment. In D69825#1760373 , @russell.gallop wrote: > It looks like the git apply didn't work, but the script continued so this was > a duff experiment, please ignore. I'll try to fix this and run it again. Thanks Russell! Can you try ru

[PATCH] D70568: [Support] Possibly use exception handler in the Crash Recovery Context in the same way as global exceptions

aganea added a comment. In D70568#1760185 , @hans wrote: > Do I understand correctly that the main point is to get a stack trace when > CrashRecoveryContext::RunSafely() fails, instead of just returning an error? Correct. Otherwise, when running with D6

[PATCH] D70467: [Distro] Bypass distro detection on non-Linux hosts

This revision was automatically updated to reflect the committed changes. aganea marked an inline comment as done. Closed by commit rG1abd4c94d757: [Clang] Bypass distro detection on non-Linux hosts (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D70467?vs=230525&id=2314

[PATCH] D70568: [Support] Possibly use exception handler in the Crash Recovery Context in the same way as global exceptions

aganea added a comment. Thanks for analyzing this! In D70568#1763769 , @hans wrote: > - About making CrashRecoveryContext::Enable() the default, that seems > somewhat orthogonal to this change. You mention the use case of running > several compilations

[PATCH] D70854: [Clang] In tests, do not always assume others permissions are set

aganea created this revision. aganea added reviewers: dblaikie, rnk. Herald added a project: clang. Herald added a subscriber: cfe-commits. aganea retitled this revision from "[Clang] Do not always assume others permissions are set" to "[Clang] In tests, do not always assume others permissions ar

[PATCH] D70149: [C-index] Fix annotate-deep-statements test in Debug target

This revision was automatically updated to reflect the committed changes. Closed by commit rG471d06020a6a: [CIndex] Fix annotate-deep-statements test when using a Debug build (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70149/n

[PATCH] D67463: [MS] Warn when shadowing template parameters under -fms-compatibility

aganea added a comment. You probably already know this, but the behavior between MSVC & Clang is different as to the value of the constant (testcase for PR43265). See: https://godbolt.org/z/y4uvgp Clang gives 0, while MSVC gives 5. This incompatibility is a potential source of failure, one of ou

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

aganea added a comment. You're right indeed Russell, my bad, this gain is not as important as I initially claimed -- however there's a gain. Several factors came into play: 1. The latest ninja 1.9.0 doesn't use all cpu sockets on some Windows systems (see this

[PATCH] D70854: [Clang] In tests, do not always assume others permissions are set

aganea marked 3 inline comments as done. aganea added inline comments. Comment at: clang/test/Misc/permissions.cpp:8 // RUN: umask 002 // RUN: %clang_cc1 -emit-llvm-bc %s -o %t rnk wrote: > If you change this to `umask 022`, does that result in `rw-r-`? T

[PATCH] D70854: [Clang] In tests, do not always assume others permissions are set

aganea marked 3 inline comments as done. aganea added inline comments. Comment at: clang/test/Misc/permissions.cpp:8 // RUN: umask 002 // RUN: %clang_cc1 -emit-llvm-bc %s -o %t rnk wrote: > aganea wrote: > > rnk wrote: > > > If you change this to `umask 022`,

[PATCH] D70854: [Clang] In tests, do not always assume others permissions are set

aganea updated this revision to Diff 232100. aganea marked an inline comment as done. aganea added subscribers: tstellar, Meinersbur. aganea added a comment. Just after I hit Submit last night, I found a better way, by disabling ACL for that folder. Ping @tstellar @Meinersbur in case they have an

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

aganea added a comment. In D69825#1768111 , @arphaman wrote: > @aganea Please disable the new behavior for the Darwin platform. We rely on > the fact that Clang `-cc1` processes crash to report crashes using system's > crash reporting infrastructure. W

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

aganea added a comment. I did this in driver.cpp: // Whether the cc1 tool should be called inside the current process, or forked // to a new new process. bool UseCC1ProcessFork = CLANG_FORK_CC1; StringRef ForkCC1Str = ::getenv("CLANG_FORK_CC1"); if (!ForkCC1Str.empty()) { UseCC1P

[PATCH] D70568: [Support] Possibly use exception handler in the Crash Recovery Context in the same way as global exceptions

aganea updated this revision to Diff 232854. aganea marked 8 inline comments as done. aganea added a subscriber: vsk. aganea added a comment. Addressed comments, and: - Removed `CrashRecoveryContext::HandleCrash()` and `#pragma clang handle_crash` which wasn't covered by any test and most likely

[PATCH] D70568: [Support] Possibly use exception handler in the Crash Recovery Context in the same way as global exceptions

aganea added inline comments. Comment at: llvm/include/llvm/Support/CrashRecoveryContext.h:26 /// Clients make use of this code by first calling /// CrashRecoveryContext::Enable(), and then executing unsafe operations via a /// CrashRecoveryContext object. For example: ---

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

aganea added inline comments. Comment at: clang/tools/driver/driver.cpp:313 + llvm::cl::ResetAllOptionOccurrences(); + StringRef Tool = argv[1] + 4; void *GetExecutablePathVP = (void *)(intptr_t) GetExecutablePath; hans wrote: > This feels a little risky to

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

aganea updated this revision to Diff 232861. aganea marked 4 inline comments as done. aganea added a comment. Herald added a subscriber: mgorny. Herald added a reviewer: jdoerfert. Addressed all comments. Added `CLANG_SPAWN_CC1` cmake flag & environment variable to control whether we want a cc1 p

[PATCH] D70854: [Clang] In tests, do not always assume others permissions are set

aganea updated this revision to Diff 233402. aganea added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Ensure the access-rights commands (setfacl and chmod) won't fail the tests if the user doesn't have the appropriate rights to change permissions. Added `llvm

[PATCH] D70854: [Clang] In tests, do not always assume others permissions are set

aganea added a comment. Yes this was added in rG18627115f4d2db5dc73207e0b5312f52536be7dd and rGe08b59f81d950bd5c8b8528fcb3ac4230c7b736c . It looks like it

[PATCH] D70568: [Support] Possibly use exception handler in the Crash Recovery Context in the same way as global exceptions

aganea added a comment. In D70568#1789527 , @hans wrote: > I looked over this again, and also studied CrashRecoveryContext some more. > > I don't really understand why this patch needs to modify the code for how the > CRC is enabled and installed, etc. >

[PATCH] D71248: [clangd] Introduce paragraph, the first part of new rendering structs

aganea added inline comments. Comment at: clang-tools-extra/clangd/FormattedString.cpp:110 +// Separate from next block. +*WritePtr++ = ' '; + } There's an issue at this line, the iterator might go past the end of the string (if running the `Document.Se

[PATCH] D71775: [ThreadPool] On Windows, extend usage to all CPU sockets and all NUMA groups

aganea created this revision. aganea added reviewers: mehdi_amini, rnk, tejohnson, russell.gallop, dexonsmith. Herald added subscribers: llvm-commits, cfe-commits, usaxena95, dang, jfb, kadircet, arphaman, steven_wu, jkorous, MaskRay, javed.absar, hiraditya, kristof.beyls, arichardson, emaste. He

[PATCH] D71775: [ThreadPool] On Windows, extend usage to all CPU sockets and all NUMA groups

aganea added a comment. In D71775#1793767 , @mehdi_amini wrote: > > Will it make sense to say "I don't want hyper-threads" ? > > Not sure I remember correctly, but I believe one motivation behind avoiding > "hyper-threads" and other virtual cores was that

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

aganea added a comment. Looking at the issue now, it should be a straightforward fix. Will send a patch ASAP. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69825/new/ https://reviews.llvm.org/D69825 _

[PATCH] D76099: [Clang][Driver] In -fintegrated-cc1 mode, avoid crashing on exit after a compiler crash

aganea created this revision. aganea added reviewers: rnk, hans, hubert.reinterpretcast, daltenty. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. As reported here: https://reviews.llvm.org/D69825#1916865 and in PR45164, in case of a compiler cr

[PATCH] D76099: [Clang][Driver] In -fintegrated-cc1 mode, avoid crashing on exit after a compiler crash

This revision was automatically updated to reflect the committed changes. Closed by commit rG28ad9fc20823: [Clang][Driver] In -fintegrated-cc1 mode, avoid crashing on exit after a… (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7

[PATCH] D76099: [Clang][Driver] In -fintegrated-cc1 mode, avoid crashing on exit after a compiler crash

aganea added a comment. Looks like there's a memory corruption detected by ASAN, but I'm not sure it's related to my change. Maybe someone with a more expert eye could tell? http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/26137/steps/check-asan%20in%20gcc%20build/logs/stdio Rep

[PATCH] D76099: [Clang][Driver] In -fintegrated-cc1 mode, avoid crashing on exit after a compiler crash

aganea added a comment. @thakis: Wasn't that issue fixed by rG18eae3312297cb197a3131f3ad9ca2bebb217415 ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76099/new/ https://reviews

[PATCH] D76099: [Clang][Driver] In -fintegrated-cc1 mode, avoid crashing on exit after a compiler crash

aganea added a comment. Looks like the latest build passed: https://ci.chromium.org/p/chromium/builders/ci/ToTLinux%20%28dbg%29/11585 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76099/new/ https://reviews.llvm.org/D76099 _

[PATCH] D76295: Use 64 bit integers for bit offsets inside AST file

aganea added a comment. Hello Dmitry! Could we use varints instead of uint64_t if size on disk is a concern? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76295/new/ https://r

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

aganea added a comment. In D73742#1854773 , @hans wrote: > > Evidently I can cut the patch in smaller pieces, let me know. > > Yes, I think this would be good. There's a lot going on in this patch, and it > would be good to separate the simple stuff from

[PATCH] D69582: Let clang driver support parallel jobs

aganea added inline comments. Comment at: clang/lib/Driver/Compilation.cpp:332 +if (!Next) { + std::this_thread::yield(); continue; yaxunl wrote: > aganea wrote: > > In addition to what @thakis said above, yielding here is maybe not a good > > id

[PATCH] D74063: [Clang] Remove #pragma clang __debug handle_crash

aganea created this revision. aganea added a reviewer: hans. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. As discussed in D70568 , remove this because it isn't used anywhere, and I think it's better to go thro

[PATCH] D74070: [Clang] Don't let gen crash diagnostics fail when '#pragma clang __debug crash' is used

aganea created this revision. aganea added reviewers: hans, rnk. Herald added a project: clang. Herald added a subscriber: cfe-commits. When `#pragma clang __debug crash` is used, currently `Driver::generateCompilationDiagnostics()` doesn't work. The `clang -E` created for diagnostics would cras

[PATCH] D74076: [Clang][Driver] Remove -M group options before generating crash diagnostics

aganea created this revision. aganea added reviewers: hans, rnk. Herald added a project: clang. Herald added a subscriber: cfe-commits. aganea edited the summary of this revision. When using `-MF file.d` on the command line, `file.d` would not be deleted after a compiler crash. Previously, the c

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

aganea added a comment. In D73742#1854810 , @hans wrote: > I think so, but I need to look closer at the CrashRecoveryContext changes, > and it would help to do that separately. I split this into smaller pieces. Please take a look at D74063

[PATCH] D74063: [Clang] Remove #pragma clang __debug handle_crash

This revision was automatically updated to reflect the committed changes. Closed by commit rG8ecde3ac34bb: [Clang] Remove unused #pragma clang __debug handle_crash (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D74063?vs=242644&id=242980#toc Repository: rG LLVM Githu

[PATCH] D74070: [Clang] Don't let gen crash diagnostics fail when '#pragma clang __debug crash' is used

This revision was automatically updated to reflect the committed changes. aganea marked 2 inline comments as done. Closed by commit rG5fedc2b41085: [Clang] Avoid crashing when generating crash diagnostics when '#pragma clang… (authored by aganea). Changed prior to commit: https://reviews.llvm.o

[PATCH] D74076: [Clang][Driver] Remove -M group options before generating crash diagnostics

This revision was automatically updated to reflect the committed changes. Closed by commit rGf41ec709d9d3: [Clang][Driver] Remove -M group options before generating crash diagnostics (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D74076?vs=242676&id=242992#toc Reposito

[PATCH] D74070: [Clang] Don't let gen crash diagnostics fail when '#pragma clang __debug crash' is used

aganea added a comment. Relanded as rG75f09b54429bee17a96e2ba7a2ac0f0a8a7f7e74 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74070/new/ https://reviews.llvm.org/D74070 _

[PATCH] D74076: [Clang][Driver] Remove -M group options before generating crash diagnostics

aganea added a comment. Relanded as rG75f09b54429bee17a96e2ba7a2ac0f0a8a7f7e74 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74076/new/ https://reviews.llvm.org/D74076 __

[PATCH] D74229: [Clang] Cover '#pragma clang __debug overflow_stack' in tests

aganea marked an inline comment as done. aganea added inline comments. Comment at: clang/lib/Lex/Pragma.cpp:1148 +#ifdef __clang__ + __attribute__((optnone)) Would it be better if we had something like `LLVM_[DISABLE|ENABLE]_OPT` in `Compiler.h` instead of th

[PATCH] D74229: [Clang] Cover '#pragma clang __debug overflow_stack' in tests

aganea created this revision. aganea added reviewers: rnk, hans. Herald added a project: clang. Herald added a subscriber: cfe-commits. aganea marked an inline comment as done. aganea added inline comments. Comment at: clang/lib/Lex/Pragma.cpp:1148 +#ifdef __clang__ + __attrib

[PATCH] D74229: [Clang] Cover '#pragma clang __debug overflow_stack' in tests

aganea planned changes to this revision. aganea added a comment. Withdrawing this, I found an crash in Debug build. I think this can't work without doing all the crash processing in a separate, pre-allocated thread. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

aganea updated this revision to Diff 243241. aganea marked 2 inline comments as done. aganea edited the summary of this revision. aganea added a comment. Simplified patch after landing previous incremental changes mentioned above . @arichardson If you hav

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

aganea updated this revision to Diff 243380. aganea added a comment. Minor update -- added support & tested the VEH codepath as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73742/new/ https://reviews.llvm.org/D73742 Files: clang/test/Driver/crash-report.c clang/tools/driver/

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

aganea added inline comments. Comment at: clang/tools/driver/cc1_main.cpp:73 // defined as an internal software error. Otherwise, exit with status 1. - exit(GenCrashDiag ? 70 : 1); + llvm::sys::Process::Exit(GenCrashDiag ? 70 : 1); } arichardson wrote: > I

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

This revision was automatically updated to reflect the committed changes. Closed by commit rGfaace365088a: [Clang][Driver] After default -fintegrated-cc1, make llvm::report_fatal_error()… (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D73742?vs=243380&id=243863#toc Rep

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

aganea marked 2 inline comments as done. aganea added a comment. Side-note: Perhaps we would need to do the same as this patch, for calls to `::abort()`? Comment at: llvm/include/llvm/Support/Process.h:205 + + /// When integrated-cc1 is disabled, terminate the current program

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

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,

[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

aganea marked an inline comment as done. aganea added a comment. In D73742#1870961 , @smeenai wrote: > I'm assuming this needs to be picked to 10.0? Yes! Is it up to the authors to integrate their patches to the release branch? I'm seeing @hans is mergi

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

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

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

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

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

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] D74490: [Clang] Limit -fintegrated-cc1 to only one TU

aganea created this revision. aganea added reviewers: thakis, rnk, hans, erichkeane. Herald added a project: clang. Herald added a subscriber: cfe-commits. aganea marked an inline comment as done. aganea added inline comments. Comment at: clang/test/Driver/cc1-spawnprocess.c:9

[PATCH] D74490: [Clang] Limit -fintegrated-cc1 to only one TU

aganea marked an inline comment as done. aganea added inline comments. Comment at: clang/test/Driver/cc1-spawnprocess.c:9 -// RUN: %clang_cl -fintegrated-cc1 -### -- %s 2>&1 \ +// RUN: %clang_cl -fintegrated-cc1 -c -### -- %s 2>&1 \ // RUN: | FileCheck %s --check-prefix=YE

[PATCH] D74490: [Clang] Limit -fintegrated-cc1 to only one TU

aganea marked 2 inline comments as done. aganea added inline comments. Comment at: clang/include/clang/Driver/Job.h:87 + /// Whether to print the input filenames when executing. + bool PrintInputFilenames; + I forgot to put back the default value(s) here, I'll

[PATCH] D74498: [clang-tidy] Fix performance-noexcept-move-constructor-fix.cpp on non-English locale

aganea created this revision. aganea added reviewers: rnk, alexfh. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. When running on Windows under the following locale: D:\llvm-project>python Python 3.8.0 (tags/v3.8.0:fa919fd, Oct 14 2019, 19:37:50) [MSC v.1916

[PATCH] D74498: [clang-tidy] Fix performance-noexcept-move-constructor-fix.cpp on non-English locale

aganea updated this revision to Diff 244246. aganea added a comment. Better fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74498/new/ https://reviews.llvm.org/D74498 Files: clang-tools-extra/test/clang-tidy/check_clang_tidy.py Index: clang-tools-extra/test/clang-tidy/check_clan

[PATCH] D74490: [Clang] Limit -fintegrated-cc1 to only one TU

This revision was automatically updated to reflect the committed changes. Closed by commit rG20f1abe306d0: [Clang] Limit -fintegrated-cc1 to only one TU (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D74490?vs=244197&id=244276#toc Repository: rG LLVM Github Monorepo

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

aganea added a comment. Hello @plotfi and @hctim -- rG20f1abe306d0 should solve the initial issues you were seeing, please let me know if it doesn't. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D74569: [clang-scan-deps] Switch to using a ThreadPool

aganea updated this revision to Diff 244475. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74569/new/ https://reviews.llvm.org/D74569 Files: clang/tools/clang-scan-deps/ClangScanDeps.cpp Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp

[PATCH] D74569: [clang-scan-deps] Switch to using a ThreadPool

aganea created this revision. aganea added reviewers: arphaman, dexonsmith, Bigcheese. Herald added subscribers: llvm-commits, cfe-commits, tschuett, hiraditya. Herald added projects: clang, LLVM. aganea updated this revision to Diff 244475. This was already reviewed as part of D71775

[PATCH] D74498: [clang-tidy] Fix performance-noexcept-move-constructor-fix.cpp on non-English locale

This revision was automatically updated to reflect the committed changes. Closed by commit rG47abb43fc364: [clang-tidy] Fix performance-noexcept-move-constructor-fix test on non-English… (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D74569: [clang-scan-deps] Switch to using a ThreadPool

aganea added a comment. Thanks for the quick response! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74569/new/ https://reviews.llvm.org/D74569 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D74569: [clang-scan-deps] Switch to using a ThreadPool

This revision was automatically updated to reflect the committed changes. Closed by commit rGd9049e871f30: [clang-scan-deps] Switch to using a ThreadPool (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74569/new/ https://reviews.

[PATCH] D71775: [ThreadPool] On Windows, extend usage to all CPU sockets and all NUMA groups

This revision was automatically updated to reflect the committed changes. Closed by commit rG8404aeb56a73: [Support] On Windows, ensure hardware_concurrency() extends to all CPU sockets… (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D71775?vs=237197&id=244664#toc Repo

[PATCH] D71775: [ThreadPool] On Windows, extend usage to all CPU sockets and all NUMA groups

aganea added a comment. Herald added a reviewer: nicolasvasilache. Herald added a subscriber: liufengdb. Ping! Any further comments? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71775/new/ https://reviews.llvm.org/D71775 ___ cfe-commits ma

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

aganea added a comment. @thakis landed a patch to control this behavior on the cmd-line, adding `-fno-integrated-cc1` would solve your issue (ie. revert to old behavior). @NoQ @xazax.hun is that an acceptable solution? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

aganea added inline comments. Comment at: clang/lib/Sema/Sema.cpp:984 + +PerformPendingInstantiations(); } All tests pass if you add `if (LangOpts.BuildingPCHWithObjectFile)` here. But if a specialization occurs inside a .CPP which includes the .PCH, not

[PATCH] D72982: [Clang] Un-break scan-build after integrated-cc1 change

aganea created this revision. aganea added reviewers: xazax.hun, NoQ, thakis, hans. Herald added subscribers: cfe-commits, Charusso, rnkovacs. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D72982 Files: clang/lib/Driver/Job.cpp clang/test/Drive

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

aganea added a comment. @wenlei Taking a look now. @hans I remember now why I was re-entering main() in the first iteration of this path. Response files were one reason. And handling /MP was another, which triggers the re-entrance twice (once for CL driver, and once for cc1). Repository: rG

[PATCH] D73060: [Clang] Fix expansion of response files in -Wp after integrated-cc1 change

aganea created this revision. aganea added reviewers: hans, thakis, wenlei. aganea added a project: clang. Herald added a subscriber: cfe-commits. aganea edited the summary of this revision. After rGb4a99a061f517e60985667e39519f60186cbb469

[PATCH] D72982: [Clang] Un-break scan-build after integrated-cc1 change

This revision was automatically updated to reflect the committed changes. Closed by commit rGa6883017ea9a: [Clang] Un-break scan-build after integrated-cc1 change (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72982/new/ https:/

[PATCH] D73060: [Clang] Fix expansion of response files in -Wp after integrated-cc1 change

aganea marked an inline comment as done. aganea added inline comments. Comment at: clang/tools/driver/driver.cpp:338 + static unsigned ReenteranceCount; + if (ReenteranceCount++) +llvm::cl::ResetAllOptionOccurrences(); hans wrote: > This looks pretty hacky.

[PATCH] D72982: [Clang] Un-break scan-build after integrated-cc1 change

aganea added a comment. In D72982#1831595 , @hans wrote: > Wait, do we really want the "(in-process)" marker to be written to a separate > line? I'm not sure that we do. Do you see value in keeping "(in-process)" at all? I added it in the first place f

[PATCH] D72982: [Clang] Un-break scan-build after integrated-cc1 change

aganea added a comment. In D72982#1832000 , @xazax.hun wrote: > Thanks! Alternatively we could try to push the changes to all three versions > and revert this patch once all of them are accepted and a new pip package is > published. @xazax.hun In that

[PATCH] D72982: [Clang] Un-break scan-build after integrated-cc1 change

aganea added a comment. In D72982#1832209 , @phosek wrote: > We're seeing `Driver/cc-print-options.c` test failures after this change: Yes, seeing it too, I will commit a fix in a minute! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D73060: [Clang] Fix expansion of response files in -Wp after integrated-cc1 change

aganea abandoned this revision. aganea added a comment. Abandoning this in favor of D73120 . Please take a look at the other patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73060/new/ https://reviews.llvm.org/

[PATCH] D73120: [Clang] Alternate fix for "expansion of response files in -Wp after integrated-cc1 change"

aganea updated this revision to Diff 239420. aganea added a comment. Remove `Reenter` flag as requested by @hans here . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73120/new/ https://reviews.llvm.org/D73120 Files: clang/include/clang/Driver

[PATCH] D72982: [Clang] Un-break scan-build after integrated-cc1 change

aganea added a comment. In D72982#1832233 , @aganea wrote: > In D72982#1832209 , @phosek wrote: > > > We're seeing `Driver/cc-print-options.c` test failures after this change: > > > Yes, seeing it too, I will commit

<    1   2   3   4   5   >