[PATCH] D87901: [Driver] Filter out /gcc and /gcc-cross if they do not exists

aganea added a comment. Thanks for the patch! I'll let others review it, but I was just being curious: what tool do you use for intercepting & printing `stat(` and `openat(`? How can I repro locally? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D8

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

aganea added a comment. Could you please explain a little bit more what motivated this change? You can achieve almost the same with `env VCToolsInstallDir=F:\Your_Path\ clang-cl ...`. You would also want to pass `-fmsc-version=...` to avoid extracting the version from `cl.exe`. If you're seeing

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

aganea added subscribers: amccarth, dblaikie. aganea added a comment. In D85998#2231001 , @zahen wrote: > The build system strives to be deterministic When you say build system, you mean MSBuild? Other? For example, Fastbuild purposely calls `CreateProce

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

aganea added inline comments. Comment at: clang/test/Driver/cl-options.c:685 +// vctoolsdir is handled by the driver; just check that we don't error. Pass -c because fakedir isn't a real toolchain path +// RUN: %clang_cl -c -vctoolsdir fakedir -- %s 2>&1 One m

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

aganea added a comment. Thanks, one more minor thing and it should be good to go. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:764 // Check the environment first, since that's probably the user telling us // what they want to use. Please update thes

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

aganea added inline comments. Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:73 + MSVCToolChain::ToolsetLayout &VSLayout) { + // Don't validate the input; trust the value supplied by the user. + // The primary motivation is to prevent unnecessary

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

aganea updated this revision to Diff 287747. aganea marked an inline comment as done. aganea added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Rebase. Address @MaskRay's suggestions. Made `safeLldLink` a public API and made it possible to provide user-defined

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

aganea updated this revision to Diff 287952. aganea marked 2 inline comments as done. aganea added a comment. Address comments. Added a CrashRecoveryTest. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 Fil

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

aganea added inline comments. Comment at: clang/tools/driver/driver.cpp:537 + // When running in integrated-cc1 mode, the CrashRecoveryContext returns + // the same code as if the program crashed. On Unix, that is codes >128. + IsCrash |= CommandRes > 128;

[PATCH] D86622: Fix failing tests after VCTOOLSDIR change

aganea added a comment. I've pushed one more fix: rG33ce275fc156c8b015acfad918937028b2cc235c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86622/new/ https://reviews.llvm.org/D8

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

aganea added a comment. Ping! @MaskRay any further comments? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

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

[PATCH] D43002: Emit S_OBJNAME symbol in CodeView

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

[PATCH] D93772: [Clang][Driver] Fix read-after-free when using /clang:

aganea created this revision. aganea added reviewers: thakis, hans, rnk, neerajksingh. aganea requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. As described in https://bugs.llvm.org/show_bug.cgi?id=42501 Fixes PR42501 Repository: rG LLVM

[PATCH] D43002: Emit S_OBJNAME symbol in CodeView

aganea updated this revision to Diff 304364. aganea edited the summary of this revision. aganea edited reviewers, added: mstorsjo; removed: reames, espindola. aganea added a comment. Simplify. Added coverage when using `clang-cl` since this is a MSVC-specific feature. When `-fdebug-compilation-dir

[PATCH] D83025: [clang] Include type specifiers in typo correction when checking isCXXDeclarationSpecifiers.

aganea added a comment. We're seeing the same issue mentionned in PR45498 , but with the repro below. `git bisect` points to this patch. Could anyone please possibly confirm? Simply build clang with `-DLLVM_ENABLE_ASSERTIONS=ON`. // compile with:

[PATCH] D83025: [clang] Include type specifiers in typo correction when checking isCXXDeclarationSpecifiers.

aganea added a comment. In D83025#2389486 , @hokein wrote: > In D83025#2389140 , @aganea wrote: > >> We're seeing the same issue mentionned in PR45498 >> , but with the

[PATCH] D43002: Emit S_OBJNAME symbol in CodeView

aganea added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43002/new/ https://reviews.llvm.org/D43002 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-b

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

aganea added inline comments. Comment at: lld/ELF/Driver.cpp:895 const char *argv[] = {config->progName.data(), opt.data()}; + cl::ResetAllOptionOccurrences(); if (cl::ParseCommandLineOptions(2, argv, "", &os)) ayrivera wrote: > MaskRay wrote: > > ayrivera

[PATCH] D89309: [ThinLTO] In documentation, mention possible value for concurrency flags

aganea created this revision. aganea added reviewers: thakis, hans. Herald added subscribers: cfe-commits, dexonsmith, steven_wu, hiraditya, inglorion. Herald added a project: clang. aganea requested review of this revision. As suggested here: https://reviews.llvm.org/D75153#2323939 Repository:

[PATCH] D89309: [ThinLTO] In documentation, mention possible values for concurrency flags

aganea updated this revision to Diff 297845. aganea added a comment. As suggested by @hans. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89309/new/ https://reviews.llvm.org/D89309 Files: clang/docs/ThinLTO.rst Index: clang/docs/ThinLTO.rst ==

[PATCH] D89309: [ThinLTO] In documentation, mention possible values for concurrency flags

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1dbf05f5b44d: [ThinLTO][Documentation] Mention possible values for concurrency flags (authored by aganea). Repository: rG LLVM Github Monorepo CH

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

aganea marked an inline comment as done. aganea added inline comments. Comment at: lld/Common/ErrorHandler.cpp:78 } - _exit(val); + llvm::sys::Process::Exit(val); } rnk wrote: > This appears to have regressed shutdown. `sys::Process::Exit` calls `exit`, >

[PATCH] D78899: [Driver] Add callback to Command execution

aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78899/new/ https://reviews.llvm.org/D78899

[PATCH] D102339: [clang] On Windows, ignore case and separators when discarding duplicate dependency file paths.

aganea added inline comments. Comment at: clang/lib/Frontend/DependencyFile.cpp:145 + StringRef SearchPath; +#ifdef _WIN32 + // Make the search insensitive to case and separators. amccarth wrote: > rnk wrote: > > I feel like this should somehow be a property of

[PATCH] D102339: [clang] On Windows, ignore case and separators when discarding duplicate dependency file paths.

aganea added a comment. In D102339#2755867 , @thakis wrote: > We share obj files built on linux and on windows. So that's a goal for us. Hello @thakis, I believe you don't use this codepath to generate the cache key, do you? I thought you had a custom p

[PATCH] D102736: Fix tmp files being left on Windows builds.

aganea added a comment. Do you think the existing crash tests can be modified to validate that .tmp files are deleted indeed? Comment at: clang/lib/Frontend/CompilerInstance.cpp:829 +Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_Text; +// Use OF_Delete on Win

[PATCH] D102736: Fix tmp files being left on Windows builds.

aganea added a comment. In D102736#2767516 , @amccarth wrote: > I seem to recall some thrashing on this topic a few months ago. If I'm > remembering correctly, setting the disposition to delete temporary files on > Windows was causing problems with Rus

[PATCH] D102736: Fix tmp files being left on Windows builds.

aganea added inline comments. Comment at: clang/include/clang/Frontend/CompilerInstance.h:170 std::string TempFilename; +Optional TempFile; Can we always use `TempFile`? And remove `TempFilename` in that case? Comment at: clang/lib/F

[PATCH] D102633: [clang-scan-deps] Improvements to thread usage

aganea added a subscriber: mehdi_amini. aganea added a comment. In D102633#2769762 , @arphaman wrote: > It might be good for @aganea to take a look as well. Thanks! I actually work with @saudi, I already took a look at the patch before uploading. Howeve

[PATCH] D102633: [clang-scan-deps] Improvements to thread usage

aganea added a comment. @dexonsmith Yes, using the Clang Tooling API seems like a good option, however some logic could/would be needed from `clang/tools/clang-scan-deps/ClangScanDeps.cpp`. Using clang-scan-deps as-a-DLL seemed like the best option on the short term, since we're using its `main

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

aganea added a comment. Hello, this patch introduces a regression in one of our codebases. The compilation of several TUs fails with the following callstack: PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

aganea added a comment. A few questions: Does this work on x86 targets? Are the scudo tests below being built with /MD or with /MT? Would this change compile/work on MinGW as well? Comment at: compiler-rt/lib/scudo/scudo_platform.h:72 +#elif SANITIZER_WINDOWS +const uptr Alloc

[PATCH] D109977: LLVM Driver Multicall tool

aganea added a comment. This is really a great change, thanks @beanz! Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:404 -int main(int argc, char **argv) { +int llvm_objcopy_main(int argc, char **argv) { InitLLVM X(argc, argv); Shouldn't we say: ```

[PATCH] D43002: [CodeView] Emit S_OBJNAME record

aganea updated this revision to Diff 374884. aganea marked 5 inline comments as done. aganea edited reviewers, added: akhuang; removed: zturner, gratianlup. aganea edited subscribers, added: zturner, gratianlup; removed: aganea. aganea added a comment. Herald added subscribers: dang, mgorny. As su

[PATCH] D43002: [CodeView] Emit S_OBJNAME record

aganea added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:752 +static void unescapeSlashes(SmallVectorImpl &Str) { + auto Read = Str.begin(); rnk wrote: > aganea wrote: > > rnk wrote: > > > This isn't unescaping them, it's just ca

[PATCH] D95534: clang-cl: Invent a /winsysroot concept

aganea added a comment. In D95534#2534510 , @smeenai wrote: > Now if only Windows could case-correct their SDKs so we didn't need VFS > overlays and symlinks for the linker... I opened a ticket

[PATCH] D79043: [Driver] Skip validation of system sanitizer blacklists files if -fno-sanitizer-blacklist was specified

aganea added a comment. In D79043#2560710 , @vitalybuka wrote: > This patch is relatively recent. I'd like to revert this for D96203 > > Can we user just pointo to /dev/null if file is not needed? You mean just use `-fn

[PATCH] D95534: clang-cl: Invent a /winsysroot concept

aganea added a comment. In D95534#2550998 , @thakis wrote: > In D95534#2534510 , @smeenai wrote: > >> I saw this on LLVM Weekly, and I like it a lot :) >> >> Now if only Windows could case-correct their SDKs so we d

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

aganea added inline comments. Comment at: clang/lib/Driver/Driver.cpp:1107 +CCPrintProcessStats = true; + Args.ClaimAllArgs(options::OPT_fproc_stat_report); + Args.ClaimAllArgs(options::OPT_fproc_stat_report_EQ); Please remove, `getLastArg` already claims a

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

aganea added a comment. Thanks for the update @vvereschaka ! Just a few minor things and it should be good to go. Also please use `git diff -U9` next time to add context to the patch. Comment at: clang/docs/UsersManual.rst:783 It is possible to specify this option witho

[PATCH] D97138: [clang][flang] Improve the consistency of the code-base

aganea added inline comments. Comment at: clang/tools/driver/driver.cpp:349 "source, and associated run script.\n"); - SmallVector argv(argv_, argv_ + argc_); + SmallVector ArgValues(Argv, Argv + Argc); What about just `Args`? "Values

[PATCH] D97138: [clang][flang] Improve the consistency of the code-base

aganea accepted this revision. aganea added a comment. In D97138#2586988 , @achieveartificialintelligence wrote: > Thanks @aganea. And I have a question that, should I use `argc` & `argv` or > `argC` & `argV` in the flang part now ? Looks good as it is

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

aganea added a comment. In D80344#2792355 , @tentzen wrote: > Thank you for reporting this . From the callstack it does not seem related > to this patch. It was actually `git bisect` which pointed to me to this patch, it could be a side-effect of it.

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

aganea added a comment. In D80344#2797317 , @tentzen wrote: > @aganea , this patch should be zero-impact without explicit option > -fasync-exceptions. Are you also seeing a crash without this option? I'm using `/EHa` which I expect translates to `-fasyn

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

aganea added a comment. Thanks for the quick fix! Would you mind fixing the two failing tests please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103664/new/ https://reviews.llvm.org/D103664 ___ cfe-co

[PATCH] D103773: [clang-cl] Add /permissive and /permissive-

aganea added inline comments. Comment at: clang/test/Driver/cl-permissive.c:15 +// PERMISSIVE-OVERWRITE-NOT: "-fdelayed-template-parsing" +// RUN: %clang_cl /permissive- /Zc:twoPhase- -### -- %s 2>&1 | FileCheck -check-prefix=PERMISSIVE-MINUS-OVERWRITE %s +// PERMISSIVE-MINUS-OV

[PATCH] D112498: [Lex] Remove timer from #pragma clang __debug crash handler

aganea added a comment. This Timer is actually the test coverage for that commit. If we don’t want it here, I guess you could move it to the Support unit tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112498/new/ https://reviews.llvm.org/D11

[PATCH] D112498: [Lex] Remove timer from #pragma clang __debug crash handler

aganea added a comment. That sounds right. You can test that scenario in a debugger, by putting a breakpoint before BuryPointer in clang/tools/driver/driver.cpp and verifying that the TimerGroup still has a pointer to the deleted stack frames, after the CrashRecoveryContext has successfully cat

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. LGTM, just a few minor things: Comment at: clang/docs/UsersManual.rst:798 + Setting ``CC_PRINT_PROC_STAT`` to ``1`` enables the feature, the report goes to + the stdout in

[PATCH] D136474: [CodeView][clang] Add flag to disable emitting command line into CodeView

aganea added a comment. Thanks for the patch @aeubanks and apologies for this oversight. I am wondering however if it wouldn’t make more sense to just strip this flag (-fmessage-length) from the emitted cmd-line? The goal is to provide a reproducer, this flag does not matter for that purpose.

[PATCH] D136474: [CodeView][clang] Add flag to disable emitting command line into CodeView

aganea added a subscriber: stefan_reinalter. aganea added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4354 + +// Emit codeview command line if requested. +if (Args.hasFlag(options::OPT_gcodeview_command_line, aeubanks wrote: > MaskR

[PATCH] D136474: [CodeView][clang] Disable emitting command line into CodeView by default and add flag to enable

aganea added subscribers: thieta, saudi. aganea added a comment. +@saudi @thieta Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136474/new/ https://reviews.llvm.org/D136474 ___ cfe-commits mailing list cf

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

aganea added a comment. Hello @gchatelet! This patches introduces a regression in a two-stage build using rpmalloc on Windows. Bisection lead me here. Would you have a chance to take a look please? Thanks in advance. To reproduce, I used the following script (rename to make_llvm.bat) F25053901:

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

aganea added a comment. In D126903#3885114 , @gchatelet wrote: > It should be fixed now @aganea . Can you check? Works now, thanks again for the quick response. Should D136752 be cherry-picked into the 15.0 branch? In

[PATCH] D136474: [CodeView][clang] Disable emitting command line into CodeView by default and add flag to enable

aganea added a comment. In D136474#3888330 , @thakis wrote: >> I read that, and I'm indicating that I don't agree. Chromium's requirements >> are driven by its particular usage of a cross-compiling distributed build >> system, which may not represent th

[PATCH] D115103: Leak Sanitizer port to Windows

aganea added a comment. In D115103#4097460 , @clemenswasser wrote: > Do you have a suggestion how I could fix this on MSVC @clemenswasser The bug is there for years, I'm surprised it hasn't been fixed yet. MSVC doesn't support different types with bitf

[PATCH] D158279: clang: Make rewrite-includes-macros.cpp runnable on non-Win

aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. Sure. This was meant to catch an issue that only happened on Windows but it should work on all platforms. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158279/new/ https://rev

[PATCH] D137101: [CodeView] Replace GHASH hasher by BLAKE3

This revision was automatically updated to reflect the committed changes. aganea marked 2 inline comments as done. Closed by commit rG49e483d3d62f: [CodeView] Replace GHASH hasher by BLAKE3 (authored by aganea). Herald added a reviewer: MaskRay. Herald added a project: clang. Herald added a subscr

[PATCH] D138746: [clang-format] Add .inc extension to git-clang-format

aganea created this revision. aganea added a reviewer: clang-format. Herald added a project: All. aganea requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The .inc extension isn't picked up for formatting when doing `git clang-format` on the

[PATCH] D138746: [clang-format] Add .inc extension to git-clang-format

This revision was automatically updated to reflect the committed changes. Closed by commit rGae918c78b51a: [clang-format] Add .inc extension to git-clang-format (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138746/new/ https://

[PATCH] D139167: [clang][Windows]Ignore Options '/d1nodatetime' and '/d1import_no_registry'

aganea added a comment. In D139167#3967123 , @efriedma wrote: > FYI, there's some discussion of /d1nodatetime floating around, even though it > isn't formally documented: it disables the definition of the `__DATE__`, > `__TIME__`, and `__TIMESTAMP__` ma

[PATCH] D76631: [Clang] Fix HIP tests when running on Windows with the LLVM toolchain in the path

aganea created this revision. aganea added reviewers: yaxunl, scott.linder. Herald added a project: clang. Herald added a subscriber: cfe-commits. On Windows, when the LLVM toolchain is in the current path (`%PATH%`), fusing the linker yields `c:\{path}\lld.exe` whereas the hip tests did not expe

[PATCH] D76631: [Clang] Fix HIP tests when running on Windows with the LLVM toolchain in the path

aganea added a comment. In D76631#1937428 , @yaxunl wrote: > I am curious why opt and llc is not affected In one case (opt, llc, clang-offload-bundler) it finds those programs in the "program paths", ie. the build folder: https://github.com/llvm/llvm-p

[PATCH] D76631: [Clang] Fix HIP tests when running on Windows with the LLVM toolchain in the path

This revision was automatically updated to reflect the committed changes. Closed by commit rG5896e2df45da: [Clang] Fix HIP tests when running on Windows with the LLVM toolchain is in the… (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

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

aganea marked 3 inline comments as done. aganea added a subscriber: respindola. aganea added a comment. In D75153#1906580 , @MaskRay wrote: > Does `taskset -c 0-3 lld -flavor ...` restrict the number of cores? > > cpu_set_t cpu; > sched_getaffinity(0,

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

This revision was automatically updated to reflect the committed changes. aganea marked an inline comment as done. Closed by commit rG09158252f777: [ThinLTO] Allow usage of all hardware threads in the system (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D75153?vs=24825

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

aganea added a comment. In D75153#1947956 , @zero9178 wrote: > This patch seems to break when disabling threading (aka LLVM_ENABLE_THREADS > == 0) as get_threadpool_strategy is undefined therefore creating linker > failures in clang. (Tested on Windows)

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

aganea added a comment. In D75153#1947956 , @zero9178 wrote: > This patch seems to break when disabling threading (aka LLVM_ENABLE_THREADS > == 0) as get_threadpool_strategy is undefined therefore creating linker > failures in clang. (Tested on Windows)

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

aganea added a comment. There seems to be still an issue with this patch, when linking the LLVM unit tests on a two-stage build, it ends with: FAILED: tools/clang/unittests/StaticAnalyzer/StaticAnalysisTests.exe cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E vs_link_exe --int

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

aganea added a reviewer: rsmith. aganea added a comment. I've tested this patch and it solves our original issue. Could anyone please take a look so that it can be landed & merged into 10.0? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74846/new/ https://review

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

aganea added a comment. @llunak Small typo: your patch fixes PR44953, not PR44958. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74846/new/ https://reviews.llvm.org/D74846 ___ cfe-commits mailing list cfe-commits@lis

[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/ https://reviews.llvm.org/D74

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

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

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

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

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

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

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

[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: clang/lib/Driv

[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/ https://reviews.llvm.

[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&id=247715#toc Repository: rG L

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

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

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

[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/ https://reviews.llvm.org/D

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

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

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

[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] 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 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 `Opts.AddPluginActions

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

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

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

<    1   2   3   4   5   >