[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea updated this revision to Diff 268359. aganea added a comment. Herald added subscribers: jfb, dexonsmith. - Using `sys::flattenWindowsCommandLine` instead of previous quoting function, as suggested by @hans - Added Clang tests for `-fdebug-compilation-dir .` - Added LLD support for making a

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2073458 , @hans wrote: > a. The compiler path is only absolute because it was absolute when put into > argv[0] right? I don't see anything in the code that makes it absolute? I > imagine most build systems will invoke th

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea updated this revision to Diff 268881. aganea marked 11 inline comments as done. aganea added a comment. Addressed comments (see inline). Ensured the generated .OBJ contains relative paths when passing `-fdebug-compilation-dir . -no-canonical-prefixes`. Repository: rG LLVM Github Monore

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea updated this revision to Diff 268906. aganea added a comment. Fix tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80833/new/ https://reviews.llvm.org/D80833 Files: clang/include/clang/Basic/CodeGenOptions.h clang/include/clang/Driv

[PATCH] D78903: [Driver] Add option -fproc-stat-report

aganea added inline comments. Comment at: clang/lib/Driver/Driver.cpp:3782 + = Cmd.getProcessStatistics(); + if (ProcStat) { +if (PrintProcessStat) { In the case where `!ProcStat`, I am wondering if we shouldn't emit zero values, in the rep

[PATCH] D81672: [Driver] When forcing a crash call abort to get the correct diagnostic

aganea added inline comments. Comment at: clang/tools/driver/driver.cpp:518 + CRC.DumpStackAndCleanupOnFailure = true; + CRC.RunSafely([&]() { abort(); }); } The only concern I have is that a unrelated call stack will be printed. Could you possibly

[PATCH] D78903: [Driver] Add option -fproc-stat-report

aganea added inline comments. Comment at: clang/lib/Driver/Driver.cpp:3782 + = Cmd.getProcessStatistics(); + if (ProcStat) { +if (PrintProcessStat) { sepavloff wrote: > aganea wrote: > > In the case where `!ProcStat`, I am wondering if we sh

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

This revision was automatically updated to reflect the committed changes. Closed by commit rG403f9537924b: [CodeView] Add full repro to LF_BUILDINFO record (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D80833?vs=268906&id=271698#toc Repository: rG LLVM Github Monore

[PATCH] D78903: [Driver] Add option -fproc-stat-report

aganea accepted this revision. aganea added a comment. This revision is now accepted and ready to land. Thanks again for the changes @sepavloff! Looks good to me! You might wait a bit before commiting, if there are further comments. Comment at: clang/lib/Driver/Driver.cpp:3806

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. Thanks for taking the time @hans, @amccarth! I've split this into several smaller patches, to ease bisect if needs be: rGa45409d8855a1e4538990507ef25e9b51c090193 - [Clang] Move clang::Job::printArg to

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2101057 , @thakis wrote: > Looks like this breaks tests on mac: http://45.33.8.238/mac/15751/step_10.txt > > Please take a look. Checking now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D81672: [Driver] When forcing a crash call abort to get the correct diagnostic

aganea added a comment. But that won't work when compiling & crashing with `-fno-integrated-cc1`, would it? (or if building with `cmake ... -DCLANG_SPAWN_CC1=1`). In that case, normal crashes (not -gen-reproducer) won't go through the `CrashRecoveryContext`. Try running: $ CCC_OVERRIDE_OPTION

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2101636 , @thakis wrote: > This is still broken; bots have been red for a few hours now. Can we revert > and analyze async, to keep the bots green please? Yes I will revert, but I don't understand. All the tests pass he

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2101690 , @thakis wrote: > Hm http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/16533 is > happy so it's not broken on all Windows machines. Guess I'll take a look on > what's going on. You can hold off on

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. Reverted in rG2ae0df5be7408a79524743762b6c74953f31b805 . I'll investigate the issue on my end. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80833/new/ h

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea marked 2 inline comments as done. aganea added a subscriber: uweigand. aganea added inline comments. Comment at: clang/test/CodeGen/debug-info-codeview-buildinfo.c:4 +// RUN: %clang_cl /c /Z7 /Fo%t.obj -fdebug-compilation-dir . -- %s +// RUN: llvm-pdbutil dump --types %t.o

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea updated this revision to Diff 272556. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80833/new/ https://reviews.llvm.org/D80833 Files: clang/test/CodeGen/debug-info-codeview-buildinfo.c lld/COFF/PDB.cpp lld/test/COFF/Inputs/pdb_lines_1_r

[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: clang-tools-extra/clangd/index/Background.cpp:154 assert(this->IndexStorageFactory && "Storage factory can not be null!"); - for (unsigned I = 0; I < ThreadPoolSize; ++I) { + for (unsigned I =

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2108423 , @uweigand wrote: > > Line 4 here fails on s390x but not on other Unix flavors, see: > > http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/33346/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Adebug-in

[PATCH] D82352: [clangd] Make background index thread count calculation clearer

aganea added a comment. LGTM. Thanks for reverting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82352/new/ https://reviews.llvm.org/D82352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. When using `clang-cl`, this code should set it to the right format, on any platform: https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/Driver.cpp#L1071 However maybe something else happens on big endian architectures? @uweigand Would you mind running the

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2109172 , @uweigand wrote: > Hmm, with clang-cl it seems the driver is trying to use this: > Target: s390x-pc-windows-msvc > which of course doesn't exist. Not sure what is supposed to be happening > here, but it seems

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

aganea added a comment. In D75153#1987320 , @phosek wrote: > In D75153#1987272 , @phosek wrote: > > > We've started seeing `llvm-cov` on our Linux bots with this error: > > > > terminating with uncaught exception

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

aganea added a comment. In D75153#1987538 , @MaskRay wrote: > I remember I saw a related bugs.llvm.org report yesterday but I can't find it > now... This? https://bugs.llvm.org/show_bug.cgi?id=45556 Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D78903: [Driver] Add option -fproc-stat-report

aganea added inline comments. Comment at: clang/docs/UsersManual.rst:770 + $ cat abc + clang-11,"/tmp/foo-123456.o",92000,84000,87536 + ld,"a.out",900,8000,53568 Please add a header to the output .CSV, specifying the units of measure where relevant.

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

aganea added a comment. If I understand correctly, this patch reverts the behavior to what it was before your changes, to only handle modules, but not PCH? Is that correct? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74846/new/ https://reviews.llvm.org/D74846

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

aganea added a comment. @llunak : It seems @hans already reverted this in rG7ea9a6e0220da36ff2fd1fbc29c2755be23e5166 , could you please ensure this patch is still relevant? Repository: rC Clang CHANGES SINCE LAST ACTION

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

aganea added a comment. My point is that all the changes in this patch are already in the upstream. If you want to re-land D69778 , you should rather re-open and update that patch instead? (and close this current one) Repository: rC Clang CHANGES SINCE LAST

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

aganea added a comment. Once you do that, maybe you could send another message to the cfe-dev mailing list to ask for reviewers who have interest into optimizing PCH? I do, but I don't have the deep knowledge of this code. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llv

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

aganea added a comment. To ease reviewers comprehension you have to propose patches that diff against the master. People sometimes apply the patches locally before accepting them. This current patch is a subtraction after D69778 is applied locally on master. It

[PATCH] D78903: [Driver] Add option -fproc-stat-report

aganea added inline comments. Comment at: clang/docs/UsersManual.rst:786 + + $ clang -fproc-stat-report=- foo.c + clang-11: output=/tmp/foo-123456.o, total=84000, user=76000, mem=87496 MaskRay wrote: > aganea wrote: > > Why not just `-fproc-stat-report` in th

[PATCH] D78903: [Driver] Add option -fproc-stat-report

aganea added inline comments. Comment at: clang/docs/UsersManual.rst:770 + $ cat abc + clang-11,"/tmp/foo-123456.o",92000,84000,87536 + ld,"a.out",900,8000,53568 sepavloff wrote: > aganea wrote: > > Please add a header to the output .CSV, specifying the uni

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

aganea added a comment. In D80833#2221866 , @aeubanks wrote: > This seems to be breaking determinism on Windows builds, see > https://crbug.com/1117026. Can this be reverted and fixed? Will do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D79337: Silence warnings when compiling x86 with latest MSVC

aganea created this revision. aganea added reviewers: bkramer, john.brawn, vsk. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. When using Visual Studio 2019 16.5.4, and targetting 32-bit, before this patch we were seeing: [1378/3007] Buildin

[PATCH] D79337: Silence warnings when compiling x86 with latest MSVC

aganea added a comment. Will do, thank you for your time! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79337/new/ https://reviews.llvm.org/D79337 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D79337: Silence warnings when compiling x86 with latest MSVC

This revision was automatically updated to reflect the committed changes. Closed by commit rG3483cdc8344d: [Sema] Silence warnings when targeting x86 with VS2019 16.5.4 (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D79337?vs=261848&id=262356#toc Repository: rG LLVM

[PATCH] D129152: [Clang][unittests] Silence trucation warning with MSVC

aganea created this revision. aganea added reviewers: ASDenysPetrov, thieta, hans. Herald added subscribers: steakhal, mgorny. Herald added a project: All. aganea requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. I'm seeing the following with

[PATCH] D129152: [Clang][unittests] Silence trucation warning with MSVC

aganea updated this revision to Diff 442366. aganea added a comment. Use `#pragma` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129152/new/ https://reviews.llvm.org/D129152 Files: clang/unittests/StaticAnalyzer/RangeSetTest.cpp Index: clang/u

[PATCH] D129152: [Clang][unittests] Silence trucation warning with MSVC

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

[PATCH] D129152: [Clang][unittests] Silence trucation warning with MSVC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0880b9d52620: [Clang][unittests] Silence trucation warning with MSVC 2022 (authored by aganea). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D119257: Revert "Re-land [LLD] Remove global state in lldCommon"

aganea added a comment. @krzysz00 I will revert and re-land with both fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119257/new/ https://reviews.llvm.org/D119257 ___ cfe-commits mailing list cfe-co

[PATCH] D119257: Revert "Re-land [LLD] Remove global state in lldCommon"

aganea added a comment. All things considered I think in the short term it is better to aim for small patches/fixes that can be transplanted to `release/14.x` if need be. I've posted D119277 for the issue you were seeing @krzysz00, but I am still planning on p

[PATCH] D119257: Revert "Re-land [LLD] Remove global state in lldCommon"

aganea added a comment. @mehdi_amini Just to unblock users, I've posted https://reviews.llvm.org/D119277 which fixes the issue originally raised by @krzysz00 I'm also working on a more long term solution in https://reviews.llvm.org/D119049 Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D86351: WIP: llvm-buildozer

aganea added inline comments. Comment at: llvm/lib/Analysis/MemorySSA.cpp:89 #ifdef EXPENSIVE_CHECKS -bool llvm::VerifyMemorySSA = true; +LLVM_THREAD_LOCAL bool llvm::VerifyMemorySSA = true; #else arsenm wrote: > Do command line flags like this really need to b

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

aganea added inline comments. Comment at: test/CodeGenCXX/debug-info-atexit-stub.cpp:14 + +// CHECK: define internal void @"??__Ff@?1??d@@YAPEAU?$c@UBXZ@YAXXZ"() +// CHECK-SAME: !dbg ![[SUBPROGRAM:[0-9]+]] { probinson wrote: > Do these Windows-mangled names w

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

aganea updated this revision to Diff 216233. aganea marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66328/new/ https://reviews.llvm.org/D66328 Files: lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDeclCXX.cpp test/CodeGenCXX/debug-info-atexit-stub.cpp t

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

aganea added a comment. In D63648#1593619 , @thakis wrote: > - Does fastbuild have something like distcc-pump? Fastbuild works like plain distcc and unfortunately it does not have pump mode. > - IIRC we used to use fdiagnostics-absolute-paths in Chromiu

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

aganea added a comment. Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66328/new/ https://reviews.llvm.org/D66328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

This revision was automatically updated to reflect the committed changes. Closed by commit rL369458: [DebugInfo] Add debug location to dynamic atexit destructor (authored by aganea, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: h

[PATCH] D66511: [clang-scan-deps] Skip UTF-8 BOM in source minimizer

aganea created this revision. aganea added reviewers: arphaman, dexonsmith, Bigcheese. aganea added a project: clang. Herald added a subscriber: tschuett. As per title. Repository: rC Clang https://reviews.llvm.org/D66511 Files: lib/Lex/DependencyDirectivesSourceMinimizer.cpp test/Lexer/

[PATCH] D66550: [clang-scan-deps] Minimizer: Correctly skip over double slashes in angle bracket #include

aganea created this revision. aganea added reviewers: arphaman, dexonsmith, Bigcheese. aganea added a project: clang. Herald added a subscriber: tschuett. aganea retitled this revision from "[clang-scan-deps] Correctly skip over double slashes in angle bracket #include" to "[clang-scan-deps] Minim

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

aganea created this revision. aganea added reviewers: arphaman, dexonsmith, Bigcheese. aganea added a project: clang. Herald added a subscriber: tschuett. Previously, an `#error` directive with quoted, multi-line content, along with CR+LF line endings wasn't handled correctly. Repository: rC

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

aganea updated this revision to Diff 216474. aganea added a comment. Use proper test file. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66556/new/ https://reviews.llvm.org/D66556 Files: lib/Lex/DependencyDirectivesSourceMinimizer.cpp test/Lexer/minimize_sou

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

aganea abandoned this revision. aganea added a comment. Please see the other reviews I've sent. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65906/new/ https://reviews.llvm.org/D65906 ___ cfe-commits mailing list cf

[PATCH] D66511: [clang-scan-deps] Skip UTF-8 BOM in source minimizer

aganea marked an inline comment as done. aganea added inline comments. Comment at: lib/Lex/DependencyDirectivesSourceMinimizer.cpp:822 bool Minimizer::minimizeImpl(const char *First, const char *const End) { + skipUTF8ByteOrderMark(First, End); while (First != End) -

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

aganea updated this revision to Diff 217055. aganea marked 2 inline comments as done. aganea added a comment. As requested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66556/new/ https://reviews.llvm.org/D66556 Files: lib/Lex/DependencyDirectivesSourceMinimizer.cpp test/Lexer/min

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

aganea updated this revision to Diff 217179. aganea added a comment. Fixed unit tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66556/new/ https://reviews.llvm.org/D66556 Files: lib/Lex/DependencyDirectivesSourceMinimizer.cpp test/Lexer/minimize_source_to_dependency_directives

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

This revision was automatically updated to reflect the committed changes. Closed by commit rL369986: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF… (authored by aganea, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pri

[PATCH] D66550: [clang-scan-deps] Minimizer: Correctly skip over double slashes in angle bracket #include

This revision was automatically updated to reflect the committed changes. Closed by commit rL369988: [clang-scan-deps] Minimizer: Correctly skip over double slashes in angle… (authored by aganea, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

[PATCH] D66511: [clang-scan-deps] Skip UTF-8 BOM in source minimizer

This revision was automatically updated to reflect the committed changes. Closed by commit rL369993: [clang-scan-deps] Skip UTF-8 BOM in source minimizer (authored by aganea, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

aganea updated this revision to Diff 217292. aganea added a reviewer: rsmith. aganea added a comment. This failed the build - I've changed `unix2dos` to `svn:eol-style CRLF` instead. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66556/new/ https://reviews.llvm.org/D66556 Files: lib/L

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

aganea updated this revision to Diff 217298. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66328/new/ https://reviews.llvm.org/D66328 Files: lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDeclCXX.cpp test/CodeGenCXX/debug-info-atexit-stub.cpp test/CodeGenCXX/debug-info-destroy-helper.

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

aganea reopened this revision. aganea added a reviewer: hans. aganea added a comment. This revision is now accepted and ready to land. Re-opening this because the previous commit broke Chromium. - Added a new test `debug-info-dest

[PATCH] D66328: [DebugInfo] Add debug location to dynamic atexit destructor

aganea added a comment. In D66328#1647062 , @probinson wrote: > I don't see a test for the __cxx_global_array_dtor case? It is actually the `arraydestroy.*` loop that is covered (generated by `CodeGenFunction::emitArrayDestroy`), please see `debug-info

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

aganea updated this revision to Diff 217527. aganea added subscribers: jyknight, rnk. aganea added a comment. In D66556#1647669 , @arphaman wrote: > Will the git monorepo handle `svn:eol-style` correctly? Other files in the repo already use `svn:eol-styl

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL370129: Re-land [clang-scan-deps] Minimizer: Correctly handle multi-line content with… (authored by aganea, committed by )

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

aganea added a comment. In D66556#1648591 , @hans wrote: > In D66556#1648118 , @dexonsmith > wrote: > > > In D66556#1648109 , @rnk wrote: > > > > > I'm not sure what happens

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

aganea added a comment. rL370219 and rG3c307370c8f8 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66556/new/ https://reviews.llvm.org/D66556 _

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

aganea updated this revision to Diff 217968. aganea retitled this revision from "[DebugInfo] Add debug location to dynamic atexit destructor" to "[DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial". aganea added a comment. More tagging functions as artific

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

aganea updated this revision to Diff 218515. aganea marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66328/new/ https://reviews.llvm.org/D66328 Files: include/clang/AST/GlobalDecl.h lib/CodeGen/CGDebugInfo.cpp lib/CodeGen/CGDeclCXX.cpp test/CodeGenC

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

aganea added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:3581 +llvm::DILocalScope *PrevScope = +!LexicalBlockStack.empty() +? dyn_cast(LexicalBlockStack.back()) rnk wrote: > Is it OK to look up the lexical block stack at this

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

aganea marked an inline comment as done. aganea added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:3581 +llvm::DILocalScope *PrevScope = +!LexicalBlockStack.empty() +? dyn_cast(LexicalBlockStack.back()) aganea wrote: > rnk wrot

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

This revision was automatically updated to reflect the committed changes. Closed by commit rL371080: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as… (authored by aganea, committed by ). Changed prior to commit: https://reviews.llvm.org/D66328?vs=218515&id=218928

[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/Signals.h:115 + /// - create a core/mini dump of the exception context whenever possible + void CleanupOnSignal(uintptr_t ExceptContext); } // End sys namespace hans wrote: > What is ExceptCon

[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 237038. aganea marked 16 inline comments as done. aganea added a comment. Changes as suggested by @hans. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70568/new/ https://reviews.llvm.org/D70568 Files: llvm/include/llvm/Support/CrashRecoveryContext

[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 237043. aganea marked 4 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69825/new/ https://reviews.llvm.org/D69825 Files: clang/CMakeLists.txt clang/include/clang/Config/config.h.cmake clang/include/clang/Driver/Driver.h

[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:394 + + llvm::CrashRecoveryContext::Enable(); + hans wrote: > Do we need to disable afterwards? I moved this line to `clang/tools/driver/driver.cpp` like discussed previously.

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

aganea updated this revision to Diff 237197. aganea marked 13 inline comments as done. aganea added a comment. Herald added subscribers: lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, burmako, jpienaar, rriddle. Updated as suggested by @rnk. I've also removed `ThreadPoo

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

aganea added inline comments. Comment at: llvm/lib/Support/Unix/Threading.inc:273 + +int computeHostNumPhysicalThreads() { +#if defined(HAVE_SCHED_GETAFFINITY) && defined(HAVE_CPU_COUNT) rnk wrote: > I'm not sure it makes sense to say "physical threads". I think

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

aganea marked an inline comment as done. aganea added a comment. Thanks for taking the time @hans! Comment at: llvm/unittests/Support/CrashRecoveryTest.cpp:72 + sys::RemoveFileOnSignal(Filename); + llvm::sys::AddSignalHandler(incrementGlobalWithCookie, nullptr); + GlobalInt

[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 237465. aganea marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69825/new/ https://reviews.llvm.org/D69825 Files: clang/CMakeLists.txt clang/include/clang/Config/config.h.cmake clang/include/clang/Driver/Driver.h

[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:267 + + StringRef SpawnCC1Str = ::getenv("CLANG_SPAWN_CC1"); + if (!SpawnCC1Str.empty()) { hans wrote: > aganea wrote: > > hans wrote: > > > Maybe just do the "!!" thing like for the enviro

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

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGde0a22471157: Remove umask tests (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D70854?vs=23340

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGa1f16998f371: [Support] Optionally call signal handlers when a function wrapped by the the… (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D70568?vs=237038&id=237518#toc Reposi

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

aganea marked 2 inline comments as done. aganea added inline comments. Comment at: llvm/lib/Support/CrashRecoveryContext.cpp:269 + const_cast(CRCI)->HandleCrash( + (int)ExceptionInfo->ExceptionRecord->ExceptionCode, ExceptionInfo); mstorsjo wrote: > This

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

This revision was automatically updated to reflect the committed changes. aganea marked an inline comment as done. Closed by commit rGb4a99a061f51: [Clang][Driver] Re-use the calling process instead of creating a new process… (authored by aganea). Changed prior to commit: https://reviews.llvm.o

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

aganea added a comment. Attention Apple maintainers: after this patch, do not forget to add `-DCLANG_SPAWN_CC1=ON` to your release scripts, to retain your previous crash reporting behavior (see discussion above). @dexonsmith @arphaman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

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

aganea added a comment. Fixed NetBSD as suggested in rG88b8cb7215d4333ab990c99f21c7f92262ef02ef Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69825/new/ https://reviews.llvm.org

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

aganea added a comment. I'm glad you're fixing this, this problem has came up in our profile traces as well. > The only way this breaks things that I've managed to find is if a .cpp file > using the PCH adds another template specialization that's not mentioned in > the PCH What is the error?

[PATCH] D52193: RFC: [clang] Multithreaded compilation support -- NOT FOR SUBMIT

aganea added a comment. Hi @tycho ! Sorry for not getting back earlier. I implemented an alternate approach last year, which proved to be better (in terms of build times) than what I proposed in this demo patch. That is, using a thread pool instead of the process pool as implemented here. This m

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. @russell.gallop Do you think you can upload the patch again by setting the Repository to "Monorepo"? F12921062: image.png Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86694/new/ https:/

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. Thanks for working on this @russell.gallop! I've reproduced your tests, please see below. The only difference is that I've used a ThinLTO build for stage2: -DCMAKE_CXX_FLAGS="/GS- -Xclang -O3 -fstrict-aliasing -march=skylake-avx512 -flto=thin -fwhole-program-vtables" Run

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. @russell.gallop I see a lots of failing tests when running `ninja check-all` on a Scudo-enabled build (stage 2). Do you see the same thing on your end? ==89136==ERROR: Scudo failed to allocate 0x1 (65536) bytes of memory at address 0x4c0090e00 (error code: 1455)

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a subscriber: maniccoder. aganea added a comment. In D86694#2274682 , @cryptoad wrote: > In D86694#2274548 , @russell.gallop > wrote: > >> I guess using scudo as a general purpose allocator that could

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. In D86694#2277456 , @cryptoad wrote: > I didn't try to make the Exclusive version work, mostly because I was using > the Windows TLS API and the Shared fit right in with those, but it would get > rid of a lot of the contention. T

[PATCH] D87187: [Driver] Perform Linux distribution detection just once

aganea added a subscriber: rnk. aganea added a comment. Please install & run `git clang-format` before uploading the patch. Comment at: clang/include/clang/Driver/Distro.h:27 +// Special value means that no detection was performed yet. +UninitializedDistro = -1, //

[PATCH] D86694: [scudo] Allow -fsanitize=scudo on Linux and Windows (WIP, don't land as is)

aganea added a comment. I'm also in favor, I think this is good direction ahead. It'd be nice if following issues were fixed -- in subsequent patches if you wish: - Stage1 `ninja check-scudo` fails many tests for me, see F13037612: errors.txt . - Stage2 `ninj

[PATCH] D87187: [Driver] Perform Linux distribution detection just once

aganea added a comment. In D87187#2291806 , @dmantipov wrote: > IIUC compiler driver is not intended to be multithreaded. But OK, here is the > version with llvm::call_once(...). Thanks! There could be at least two cases I see, where the driver //could//

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf2efb5742cc9: [LLD][COFF] Cover usage of LLD-as-a-library in tests (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D70378?

[PATCH] D87187: [Driver] Perform Linux distribution detection just once

aganea accepted this revision. aganea added a comment. Makes sense, you cache the commonly taken path, but not the (very uncommon) VFS codepath. LGTM, with some minor comments. Thanks again! Comment at: clang/include/clang/Driver/Distro.h:117 - bool IsOpenSUSE() const { -

<    1   2   3   4   5   >