[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

aganea added subscribers: sammccall, JDevlieghere. aganea added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:1 +//===- DependencyScanningFilesystem.h - clang-scan-deps fs ===---*- C++ -*-===// +//

[PATCH] D65906: [clang-scan-deps] Fix edge cases in the minimizer

aganea created this revision. aganea added reviewers: arphaman, dexonsmith, Bigcheese. aganea added a project: clang. Herald added a subscriber: tschuett. This patch fixes: 1. Invisible characters that come just before #include, such as #ifndef. ( were hidden, depending on the display

[PATCH] D65511: Delay emitting dllexport explicitly defaulted members until the class is fully parsed (PR40006)

aganea added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11545 +for (CXXMethodDecl *M : WorkList) { + DefineImplicitSpecialMember(*this, M, M->getLocation()); + ActOnFinishInlineFunctionDef(M); rnk wrote: > hans wrote: > > rnk

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

aganea added a comment. Thanks for the update Alex! Just a few more comments and we should be good to go: Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:117 +std::mutex CacheLock; +llvm::StringMap> Cache; + };

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. LGTM, thank you! In D63907#1617417 , @arphaman wrote: > Just for reference, this patch still doesn't reuse the FileManager across > invocations in a

[PATCH] D65906: [clang-scan-deps] Fix edge cases in the minimizer

aganea added a comment. In D65906#1620034 , @arphaman wrote: > Regarding the invisible characters before `#ifdef`, are you sure that's the > right behavior? Should we then copy these invisible characters to the minimized output? Believe it or not,

[PATCH] D65906: [clang-scan-deps] Fix edge cases in the minimizer

aganea added a comment. In D65906#1622209 , @arphaman wrote: > @aganea These are not just any invisible characters that you have, this is > the UTF8 BOM. Clang's Lexer skips over them if they're in the beginning of > the file (`Lexer::InitLexer`). The

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

aganea added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:84 +DependencyScanningFilesystemSharedCache() { + NumShards = 8; + CacheShards = llvm::make_unique(NumShards); arphaman wrote: > aganea wrote:

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

aganea added a comment. Thanks for getting back Hans! In D63648#1589119 , @hans wrote: > Are you saying the diagnostics were not using absolute paths in those cases > and this patch fixes that? Yes. >> , or --show-includes, or -E > > Those aren't

[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] D63648: [Preprocessor] Honor absolute paths in diagnostics

aganea added a comment. It totally makes sense, thanks for the explanation Nico! Let's forget about `cl` compatibility, that wasn't my initial intent. We always pass //relative// paths on the cmd-line, for all the reasons you've mentioned. We also pass `-fdiagnostics-absolute-paths` hoping to

[PATCH] D63648: [Preprocessor] Honor absolute paths in diagnostics

aganea added a comment. The problem is that it is not `cl.exe`'s behavior - it always makes paths absolute: F:\svn\test>cl /c test.cc /showIncludes Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27031.1 for x64 Copyright (C) Microsoft Corporation. All rights reserved. test.cc

[PATCH] D63907: [clang-scan-deps] Implementation of dependency scanner over minimized sources

aganea added a comment. I will take a look next week when I get back! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63907/new/ https://reviews.llvm.org/D63907 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D64301: Use `ln -n` to prevent forming a symlink cycle, instead of rm'ing the source

aganea added a comment. In D64301#1574304 , @rnk wrote: > I think @zturner wanted to teach lit to completely remove the Output/ > directory for every test so we don't have to deal with stale files from > previous tests hanging around. I remember

[PATCH] D64301: Use `ln -n` to prevent forming a symlink cycle, instead of rm'ing the source

aganea added a comment. In D64301#1576615 , @aganea wrote: > In D64301#1574304 , @rnk wrote: > > > I think @zturner wanted to teach lit to completely remove the Output/ > > directory for every test so we don't

[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

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

aganea created this revision. aganea added reviewers: STL_MSFT, rnk, dexonsmith. aganea added a project: clang. Herald added subscribers: cfe-commits, arphaman. This is an attempt to fix `clang/test/Index/crash-recovery-modules.m` when compiling a build with the MSVC STL & _ITERATOR_DEBUG_LEVEL

[PATCH] D69582: Let clang driver support parallel jobs

aganea added a comment. This is somehow similar to what I was proposing in D52193 . Would you possibly provide tests and/or an example of your usage please? Comment at: clang/lib/Driver/Compilation.cpp:303 +} +std::thread Th(Work); +

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

aganea updated this revision to Diff 229136. aganea added a comment. Revert everything, but the change to `FrontendOptions::Inputs`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69959/new/ https://reviews.llvm.org/D69959 Files: clang/include/clang/Frontend/FrontendOptions.h

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

aganea marked 3 inline comments as done. aganea added inline comments. Comment at: clang-tools-extra/clang-move/tool/ClangMove.cpp:113 move::MoveDefinitionSpec Spec; - Spec.Names = {Names.begin(), Names.end()}; + Spec.Names = (std::vector &)Names; Spec.OldHeader =

[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 =(const std::vector ) { +this->assign(Vec.begin(), Vec.end()); dblaikie wrote: > aganea wrote: > > rnk wrote: >

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

aganea updated this revision to Diff 228971. aganea edited the summary of this revision. aganea added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Using `SmallVector` as requested. The change in SmallVector.h is to support things such as

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

aganea created this revision. aganea added reviewers: rsmith, aaron.ballman, rnk. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. As per title: previously, running the test `clang/test/Index/annotate-deep-statements.cpp` used to fail because of stack exhaustion

[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

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] 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

[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

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

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

[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

[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

[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

[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

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

[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

[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

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

[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] 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-`?

[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

[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()) {

[PATCH] D69043: [RFC] Adding time-trace to LLD?

aganea added inline comments. Comment at: lld/ELF/Driver.cpp:515 + + if (llvm::timeTraceProfilerEnabled()) { +SmallString<128> path(config->outputFile); Could you possibly move this block (and the init block above) into a new file in `lld/Common/` so we

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

[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

[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

[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

[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] 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] 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:

[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

[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

[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

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

[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

[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

[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

[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] 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.

[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

[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

[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

[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

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

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

[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

[PATCH] D75153: [ThinLTO] Allow usage of all SMT threads in the system

aganea created this revision. aganea added reviewers: tejohnson, thakis, rnk, RobRich999. Herald added subscribers: cfe-commits, dang, dexonsmith, mikhail.ramalho, steven_wu, MaskRay, aheejin, hiraditya, arichardson, inglorion, sbc100, emaste. Herald added a reviewer: espindola. Herald added

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

aganea added a comment. @thakis I think this is a side-effect of implementing `computeHostNumPhysicalCores()` for Windows, which previously returned -1, which in turn made `llvm::heavyweight_hardware_concurrency()` previously behave like `llvm::hardware_concurrency()` (on Windows only). At

[PATCH] D75373: [Clang] Fix Hurd toolchain class hierarchy

aganea updated this revision to Diff 247400. aganea added a comment. Fix clang-formatting. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75373/new/ https://reviews.llvm.org/D75373 Files: clang/lib/Driver/ToolChains/Hurd.cpp clang/lib/Driver/ToolChains/Hurd.h Index:

[PATCH] D75153: [ThinLTO] Allow usage of all SMT threads in the system

aganea added a comment. @mehdi_amini Agreed. In that case, I just need to slightly change this function to ensure threads are equally dispatched on all CPU sockets regardless of the thread strategy.

[PATCH] D75373: [Clang] Fix Hurd toolchain class hierarchy

aganea updated this revision to Diff 247542. aganea edited the summary of this revision. aganea added a comment. The patch did not make sense conceptually. Hurd is not Linux. I think now it makes more sense. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75373/new/

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

[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

[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

[PATCH] D75153: [ThinLTO] Allow usage of all SMT threads in the system

aganea added a comment. Thanks Reid! I will leave this open for a few days, in case there are any other feedbacks. As for `-fthinlto-index`, we have someone looking at distributing it remotely with Fastbuild. I think it is reasonable on the short-term to let the build system handle that (I

[PATCH] D75153: [ThinLTO] Allow usage of all SMT threads in the system

aganea updated this revision to Diff 248254. aganea edited the summary of this revision. aganea edited reviewers, added: dexonsmith, ruiu; removed: RobRich999, espindola. aganea added subscribers: ruiu, RobRich999. aganea added a comment. Herald added a reviewer: espindola. Simplify. Revert to

[PATCH] D75153: [ThinLTO] Allow usage of all SMT threads in the system

aganea added a comment. If a simpler (more yucky?) patch is needed to fix what @thakis was suggesting in https://reviews.llvm.org/D71775#1891709, and if we don't want this extra new flag, we can also check the CPU brand for "AMD Opteron", and keep the old behavior in that case. Repository:

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

aganea added a comment. Thanks @hans! Nevertheless I think this is a good change and we should push it for 11.0. I was indeed seeing better timings on our end with what @llunak was proposing, we are heavy users of PCH headers (and migrating to modules is not trivial in our case, lots of code

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

aganea marked an inline comment as done. aganea added inline comments. Comment at: llvm/lib/Support/Threading.cpp:94 + // uselessly induce more context-switching and cache eviction. + if (!ThreadsRequested || ThreadsRequested > (unsigned)MaxThreadCount) +return

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

aganea added a comment. @llunak Do you have time to address this patch this week? If not, I can finish it and land it. Please let me know. We'd like to see it merged into 10.0. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74846/new/

[PATCH] D75373: [Clang] Fix Hurd toolchain class hierarchy

This revision was automatically updated to reflect the committed changes. Closed by commit rG7e77cf473ac9: [Clang] Fix Hurd toolchain test on a two-stage build with ThinLTO (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D75373?vs=247542=247715#toc Repository: rG

[PATCH] D75373: [Clang] Fix Hurd toolchain class hierarchy

aganea created this revision. aganea added reviewers: hans, sthibaul, kristina. Herald added subscribers: cfe-commits, dexonsmith, Prazek. Herald added a project: clang. aganea added a comment. @hans, do you think this could be included in 10.0? As noted in PR45061, a two-stage ThinLTO build

[PATCH] D75373: [Clang] Fix Hurd toolchain class hierarchy

aganea added a comment. @hans, do you think this could be included in 10.0? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75373/new/ https://reviews.llvm.org/D75373 ___ cfe-commits mailing list

[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

aganea added a comment. In D73242#1893506 , @tejohnson wrote: > In D73242#1888295 , @tejohnson wrote: > > > FYI I have reproduced the failure, and am starting to debug. > > > I see what is going on and have a

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

<    1   2   3   4   5   >