Page MenuHomePhabricator

aganea (Alexandre Ganea)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 6 2017, 1:24 PM (238 w, 5 d)

Recent Activity

May 2 2022

aganea accepted D123225: [ThreadPool] add ability to group tasks into separate groups.

LGTM, thanks for all the changes @llunak!

May 2 2022, 5:22 AM · Restricted Project, Restricted Project

Apr 26 2022

aganea added inline comments to D123225: [ThreadPool] add ability to group tasks into separate groups.
Apr 26 2022, 5:49 AM · Restricted Project, Restricted Project

Apr 22 2022

aganea updated subscribers of D122922: [lld][common][lld-macho][lld-coff] Support per-thread allocators and StringSavers.

@int3 I don't want to block this, I understand your needs, and I value the intention of this patch. I'd be happy to reintroduce llvm::sys::ThreadLocal later. However I wasn't able to reproduce the 1% regression (see D123879 for numbers), although I don't deny it could exist in some configurations (depending on the machine, OS, LLVM build options, allocator). I'm just wondering if 1% regression isn't acceptable for now in the trunk, so that we can commit a more frictionless version of this patch.

Apr 22 2022, 7:48 AM · Restricted Project, Restricted Project
aganea added inline comments to D86351: WIP: llvm-buildozer.
Apr 22 2022, 6:41 AM · Restricted Project, Restricted Project, Restricted Project

Apr 19 2022

aganea added inline comments to D123225: [ThreadPool] add ability to group tasks into separate groups.
Apr 19 2022, 12:13 PM · Restricted Project, Restricted Project

Apr 18 2022

aganea added a comment to D123879: [LLD] Alternate implementation for "Support per-thread allocators and StringSavers".

I see no difference on the ELF side, but @MaskRay maybe you're running a more through test? This is a two-stage LLD, second stage uses -march=native -Xclang -O3 -fstrict-aliasing -fwhole-program-vtables -flto=thin, running on a c5a.24xlarge EC2 instance.

ubuntu@XXX:~/chromium/src/out/Default$ uname -a
Linux XXX 5.13.0-1021-aws #23~20.04.2-Ubuntu SMP Thu Mar 31 11:36:15 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
ubuntu@XXX:~/chromium/src/out/Default$ lscpu
Architecture:                    x86_64
CPU op-mode(s):                  32-bit, 64-bit
Byte Order:                      Little Endian
Address sizes:                   48 bits physical, 48 bits virtual
CPU(s):                          96
On-line CPU(s) list:             0-95
Thread(s) per core:              2
Core(s) per socket:              48
Socket(s):                       1
NUMA node(s):                    1
Vendor ID:                       AuthenticAMD
CPU family:                      23
Model:                           49
Model name:                      AMD EPYC 7R32
Stepping:                        0
CPU MHz:                         2799.992
BogoMIPS:                        5599.98
Hypervisor vendor:               KVM
Virtualization type:             full
L1d cache:                       1.5 MiB
L1i cache:                       1.5 MiB
L2 cache:                        24 MiB
L3 cache:                        192 MiB
NUMA node0 CPU(s):               0-95
Vulnerability Itlb multihit:     Not affected
Vulnerability L1tf:              Not affected
Vulnerability Mds:               Not affected
Vulnerability Meltdown:          Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl and seccomp
Vulnerability Spectre v1:        Mitigation; usercopy/swapgs barriers and __user pointer sanitization
Vulnerability Spectre v2:        Mitigation; LFENCE, IBPB conditional, IBRS_FW, STIBP conditional, RSB filling
Vulnerability Srbds:             Not affected
Vulnerability Tsx async abort:   Not affected
Flags:                           fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext
                                 fxsr_opt pdpe1gb rdtscp lm constant_tsc rep_good nopl nonstop_tsc cpuid extd_apicid aperfmperf tsc_known_freq pni pclmu
                                 lqdq monitor ssse3 fma cx16 sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm cmp_legacy cr8_lega
                                 cy abm sse4a misalignsse 3dnowprefetch topoext perfctr_core ssbd ibrs ibpb stibp vmmcall fsgsbase bmi1 avx2 smep bmi2 r
                                 dseed adx smap clflushopt clwb sha_ni xsaveopt xsavec xgetbv1 clzero xsaveerptr rdpru wbnoinvd arat npt nrip_save rdpid

(bin_tls_bumpptr is with this patch, bin_no_tls is without, using Chromium checkout 8660f8deda4982de98baddafaffb651898144bac and LLVM checkout b859c39c40a79ff74033f67d807a18130b9afe30)

ubuntu@XXX:~/chromium/src/out/Default$ hyperfine '~/llvm-project/stage2/bin_tls_bumpptr/ld.lld @__link_unit_tests.rsp' '~/llvm-project/stage2/bin_no_tls/ld.lld @__link_unit_tests.rsp'
Benchmark 1: ~/llvm-project/stage2/bin_tls_bumpptr/ld.lld @__link_unit_tests.rsp
  Time (mean ± σ):      9.743 s ±  0.057 s    [User: 21.273 s, System: 78.149 s]
  Range (min … max):    9.661 s …  9.845 s    10 runs
Apr 18 2022, 7:12 AM · Restricted Project, Restricted Project
aganea added inline comments to D123879: [LLD] Alternate implementation for "Support per-thread allocators and StringSavers".
Apr 18 2022, 6:42 AM · Restricted Project, Restricted Project

Apr 15 2022

aganea added a comment to D123879: [LLD] Alternate implementation for "Support per-thread allocators and StringSavers".

This is just for demonstrative purposes, @oontvoo feel free to cherry-pick anything of interest into the other patch.

Apr 15 2022, 3:57 PM · Restricted Project, Restricted Project
aganea added a comment to D122922: [lld][common][lld-macho][lld-coff] Support per-thread allocators and StringSavers.

I've posted an alternate implementation, please see D123879:

Apr 15 2022, 3:41 PM · Restricted Project, Restricted Project
aganea requested review of D123879: [LLD] Alternate implementation for "Support per-thread allocators and StringSavers".
Apr 15 2022, 3:40 PM · Restricted Project, Restricted Project
aganea added a comment to D122922: [lld][common][lld-macho][lld-coff] Support per-thread allocators and StringSavers.

not super familiar with this effort. What is the scope of this "per-session" ? Is is per-process? If so, it doesn't solve the problem (here we have multiple threads using the same bptrAlloc, hence the race condition)

No, it's meant so that you can use lld as a library, so that one process can have multiple threads running in parallel, where more than one such thread can call e.g. lld::coff::link() at the same time. I haven't followed the state of the work on that in very close detail, but as far as I understand, the implementation strategy has been to gather everything that previously was global, into a per-invocation context, that is passed around (and/or stored as a thread local variable somewhere I think). So within each linker invocation, you'd only ever have allocations happening on one thread, the one where the user called lld::*::link() - each such linker job then would run multiple threads for occasional parallelism in the linking process though.
[...]
I think it'd be valuable to align these two thread safety efforts so we don't end up with two mechanisms for doing the same. I think @aganea has been involved in that effort though. @aganea - can you chime in here?

Apr 15 2022, 3:39 PM · Restricted Project, Restricted Project
aganea committed rG64969446bc27: [Support][cmake] Fix snmalloc integration. NFC. (authored by aganea).
[Support][cmake] Fix snmalloc integration. NFC.
Apr 15 2022, 12:20 PM · Restricted Project, Restricted Project

Apr 8 2022

aganea updated subscribers of D122107: [OpenMP] Add support for ompt_callback_dispatch.

+ @gribozavr2

Apr 8 2022, 10:54 AM · Restricted Project, Restricted Project
aganea added a comment to D122107: [OpenMP] Add support for ompt_callback_dispatch.

Hello! It seems the loop_dispatch.c test is occasionally causing errors on one of the builders: https://lab.llvm.org/buildbot/#/builders/84/builds/22238/steps/6/logs/FAIL__libomp__loop_dispatch_c (also see the builder's history)
Anyone would have a chance to look at it please? Thanks in advance!

Apr 8 2022, 8:24 AM · Restricted Project, Restricted Project
aganea committed rGffaf667a435b: [Support][unittests] Silence warning when building with Clang 13 on (authored by aganea).
[Support][unittests] Silence warning when building with Clang 13 on
Apr 8 2022, 8:08 AM · Restricted Project, Restricted Project
aganea added a comment to D122887: [llvm-pdbutil] Move global state (Filters) inside LinePrinter class..

Thanks for your review @aganea

Thank you for your work and the patience! :-)

Apr 8 2022, 7:04 AM · Restricted Project, Restricted Project
aganea accepted D122887: [llvm-pdbutil] Move global state (Filters) inside LinePrinter class..

LGTM, thanks again @CarlosAlbertoEnciso !

Apr 8 2022, 5:25 AM · Restricted Project, Restricted Project

Apr 7 2022

aganea added inline comments to D122922: [lld][common][lld-macho][lld-coff] Support per-thread allocators and StringSavers.
Apr 7 2022, 10:12 AM · Restricted Project, Restricted Project
aganea added inline comments to D122922: [lld][common][lld-macho][lld-coff] Support per-thread allocators and StringSavers.
Apr 7 2022, 10:03 AM · Restricted Project, Restricted Project
aganea updated subscribers of D122922: [lld][common][lld-macho][lld-coff] Support per-thread allocators and StringSavers.

Hello! Thanks for adding me :-) Interesting challenge!
It's a bit sad that we have to do all this high-level/application-level memory management. Most modern allocators already support (lock-free) per-thread memory pools out-of-the-box, along with migration between threads and to the global pool/arena. This patch seems to be needed solely because we use BumpPtrAllocator/SpecificBumpPtrAllocator. I wonder how things would perform with just using malloc() instead + a modern underlying allocator (rpmalloc, mimalloc, ...). Memory locality brought by the bumpalloc is important, but it'd be interesting to compare benchmarks. FWIW there were discussions with @lattner at some point about integrating rpmalloc into the LLVM codebase, but I never got to post a RFC.

Apr 7 2022, 6:48 AM · Restricted Project, Restricted Project
aganea added a comment to D122887: [llvm-pdbutil] Move global state (Filters) inside LinePrinter class..

Do you think it would be possible to omit Optional and just pass PrintScope &HeaderScope everywhere? It feels like without the printer, these functions don't have much purpose?

Apr 7 2022, 6:24 AM · Restricted Project, Restricted Project
aganea added a comment to D123225: [ThreadPool] add ability to group tasks into separate groups.

In essence the call stack would be: the thread loop in ThreadPool::grow() calls Task() which would call the TaskGroup logic lambda, which would call the user lambda. Regular non-TaskGroup would not go through that logic.

I don't quite get what you mean? Can you elaborate a bit?
I agree that the complexity increase is worrisome, so any idea to manage it is welcome :)

Apr 7 2022, 5:42 AM · Restricted Project, Restricted Project

Apr 6 2022

aganea added inline comments to D123226: [lldb] use one shared ThreadPool and task groups.
Apr 6 2022, 2:20 PM · Restricted Project, Restricted Project
aganea added inline comments to D123226: [lldb] use one shared ThreadPool and task groups.
Apr 6 2022, 1:25 PM · Restricted Project, Restricted Project
aganea added reviewers for D123225: [ThreadPool] add ability to group tasks into separate groups: mehdi_amini, MaskRay, aganea.

Not sure @chandlerc will be able to review your patch, adding some people that have been working in this space recently.

Apr 6 2022, 1:05 PM · Restricted Project, Restricted Project
aganea accepted D123185: [LLD][COFF] Fix TypeServerSource matcher with more than one collision.

LGTM.

Apr 6 2022, 8:23 AM · Restricted Project, Restricted Project
aganea added a comment to D123185: [LLD][COFF] Fix TypeServerSource matcher with more than one collision.

@aganea Added more testing - adding a third collision and also probe all the types and in different orders, since the order fooled the testing the first time since the order they where processed in LLD made a difference for the output.

I had to drop the log message since when we know set the entry to nullptr there is nothing informative to log, but I am not convinced the log message was that useful and I really don't think it's worth adding some more complexity to store the paths of all collisions.

WDYT?

Apr 6 2022, 8:21 AM · Restricted Project, Restricted Project
aganea updated subscribers of D122372: [lld][COFF] Fix TypeServerSource lookup on GUID collisions.
Apr 6 2022, 6:38 AM · Restricted Project, Restricted Project
aganea accepted D123185: [LLD][COFF] Fix TypeServerSource matcher with more than one collision.

This looks good, but please see my other comment first! https://reviews.llvm.org/D122372#3432711

Apr 6 2022, 6:18 AM · Restricted Project, Restricted Project
aganea added inline comments to D122372: [lld][COFF] Fix TypeServerSource lookup on GUID collisions.
Apr 6 2022, 6:17 AM · Restricted Project, Restricted Project

Apr 5 2022

aganea added inline comments to D122372: [lld][COFF] Fix TypeServerSource lookup on GUID collisions.
Apr 5 2022, 5:02 PM · Restricted Project, Restricted Project

Apr 4 2022

aganea added a comment to D122943: [LLDB][NativePDB] Fix a crash when S_DEFRANGE_SUBFIELD_REGISTER descirbes a simple type.

There's a typo in the title, s/descirbes/describes/

Apr 4 2022, 7:50 PM · Restricted Project, Restricted Project

Apr 1 2022

aganea added a comment to D122887: [llvm-pdbutil] Move global state (Filters) inside LinePrinter class..

Thanks for following up on this @CarlosAlbertoEnciso!

Apr 1 2022, 8:53 AM · Restricted Project, Restricted Project

Mar 30 2022

aganea accepted D122372: [lld][COFF] Fix TypeServerSource lookup on GUID collisions.

LGTM, thanks @thieta !

Mar 30 2022, 5:39 AM · Restricted Project, Restricted Project

Mar 29 2022

aganea added a comment to D122372: [lld][COFF] Fix TypeServerSource lookup on GUID collisions.

Consolidated the test-inputs and used SED instead. I tried using available input but couldn't get it to collide the same way this one did.

Oh yeah to fully repro the bug, you'd probably need to insert and extra record in the second PDB to shift the offsets to the LF_FUNC_ID. Seems good otherwise, just two (and a half) comments.

Mar 29 2022, 7:06 AM · Restricted Project, Restricted Project

Mar 28 2022

aganea added inline comments to D122372: [lld][COFF] Fix TypeServerSource lookup on GUID collisions.
Mar 28 2022, 7:39 PM · Restricted Project, Restricted Project
aganea added a comment to D122372: [lld][COFF] Fix TypeServerSource lookup on GUID collisions.

Reduced further:

> yaml2obj.exe -o=a.obj lld\test\COFF\Inputs\pdb-type-server-simple-a.yaml
> sed -e s/ts.pdb/bs.pdb/ lld\test\COFF\Inputs\pdb-type-server-simple-b.yaml | yaml2obj.exe > b.obj
> llvm-pdbutil.exe yaml2pdb lld\test\COFF\Inputs\pdb-type-server-simple-ts.yaml -pdb ts.pdb
> llvm-pdbutil.exe yaml2pdb lld\test\COFF\Inputs\pdb-type-server-simple-ts.yaml -pdb bs.pdb
> lld-link.exe a.obj b.obj /debug
(crash)
Mar 28 2022, 7:36 PM · Restricted Project, Restricted Project
aganea added a comment to D122372: [lld][COFF] Fix TypeServerSource lookup on GUID collisions.

The inputs are a bit big - but I am unsure on how to trim them down efficiently.

Mar 28 2022, 7:29 PM · Restricted Project, Restricted Project
aganea accepted D122622: [llvm-pdbutil] Fix a crash due to Expected not checked before destruction.

LGTM with a comment:

Mar 28 2022, 6:26 PM · Restricted Project, Restricted Project

Mar 24 2022

aganea added a comment to D122372: [lld][COFF] Fix TypeServerSource lookup on GUID collisions.

Can you please craft a small test that would showcase the problem? The test shall fail before your change, and should succeed after.

Mar 24 2022, 6:10 AM · Restricted Project, Restricted Project

Mar 23 2022

aganea added a comment to D122226: [llvm-pdbutil] Move InputFile/FormatUtil/LinePrinter to the PDB library..

There's a typo in the title:

[llmv-pdbutil] Move InputFile/FormatUtil/LinePrinter to PDB library.

llvm-pdbutil

Mar 23 2022, 4:32 PM · Restricted Project, Restricted Project

Mar 17 2022

aganea accepted D121801: [llmv-pdbutil] Replace ExitOnError with explicit error handling..

Essentially, this looks good to me. I have some minor concerns that as we migrate dumping functionality into library code, it will tend to increase the code size of production tools such as the linker. Maybe archive semantics and section GC paper over those problems. However, in the end I don't think it makes sense to worry about unmeasured, hypothetical code size problems. If there are problems, I guess it would make sense to factor out "dumping" functionanlity from the PDB and CodeView libraries into their own CVDump helper library or something. This is similar to how we pulled YAML writing out of lib/Object.

Mar 17 2022, 7:47 AM · Restricted Project, Restricted Project

Mar 16 2022

aganea added inline comments to D121801: [llmv-pdbutil] Replace ExitOnError with explicit error handling..
Mar 16 2022, 11:03 AM · Restricted Project, Restricted Project
aganea added reviewers for D121725: [clang][test] Add using-lld feature variable: rnk, mstorsjo.

Seems reasonable having an integration with LLD, but maybe others would like to chime in?

Mar 16 2022, 8:47 AM · Restricted Project
aganea added a comment to D121724: [lld] Force crashes when env var LLD_FORCE_CRASH set.

Personally I am not feeling very comfortable adding such an environment variable just so that it can be tested.
I think it may make sense to add the clang driver change without a test.

Mar 16 2022, 8:38 AM · Restricted Project
aganea added inline comments to D121801: [llmv-pdbutil] Replace ExitOnError with explicit error handling..
Mar 16 2022, 8:32 AM · Restricted Project, Restricted Project

Mar 15 2022

aganea added a comment to D120175: [Driver] Re-run lld with --reproduce when it crashes.

Thanks for making the changes!

Mar 15 2022, 7:55 AM · Restricted Project

Mar 7 2022

aganea added a comment to D120175: [Driver] Re-run lld with --reproduce when it crashes.

There is currently no testing because, although there is testing for generateCompilationDiagnostics, it is harder to force lld to crash, even if we added a way to crash lld on purpose, the test would need to depend on lld. Looking for suggestions if anyone has any.

I think you add test coverage in LLD, through a environment variable that triggers a crash in any of the drivers & --reproduce to ensure the archive was created.

lld has tests for --reproduce already. Even if I added an environment variable to crash lld, the test would depend on lld being built, adding a big dependency to clang's tests. Do we think that's worth it?

It would be a interesting addition to bind LLVM_LINKER_IS_LLD to a LIT test "feature" -- in a follow-up patch if you're willing to? (see config.available_features.add('z3') and LLVM_WITH_Z3 in clang/test/lit.cfg.py for an example)

Mar 7 2022, 11:53 AM · Restricted Project

Feb 21 2022

aganea added a comment to D120175: [Driver] Re-run lld with --reproduce when it crashes.

There is currently no testing because, although there is testing for generateCompilationDiagnostics, it is harder to force lld to crash, even if we added a way to crash lld on purpose, the test would need to depend on lld. Looking for suggestions if anyone has any.

I think you add test coverage in LLD, through a environment variable that triggers a crash in any of the drivers & --reproduce to ensure the archive was created.

Feb 21 2022, 6:58 AM · Restricted Project

Feb 16 2022

aganea accepted D119913: [lld-macho] Don't include CommandFlags.h in CommonLinkerContext.h.

LGTM, thanks!

Feb 16 2022, 7:57 AM · Restricted Project, Restricted Project
aganea added a comment to D119913: [lld-macho] Don't include CommandFlags.h in CommonLinkerContext.h.

Ideally RegisterCodeGenFlags should be a function call not a class, since it only registers static cl::opts. I think you can simply remove cgf from CommonLinkerContext.h and create a short-lived llvm::codegen::RegisterCodeGenFlags object on the stack inside the CommonLinkerContext constructor definition.

Feb 16 2022, 6:18 AM · Restricted Project, Restricted Project

Feb 8 2022

aganea added inline comments to D119278: [LLD] Fix issue in HIP toolchain due to unspecified order of evaluation of the function object.
Feb 8 2022, 4:17 PM · Restricted Project
aganea committed rGbb8be26a7ec3: [LLD] Fix issue in HIP due to unspecified order of evaluation of the function… (authored by aganea).
[LLD] Fix issue in HIP due to unspecified order of evaluation of the function…
Feb 8 2022, 4:12 PM
aganea committed rG1e661e583d84: [MLIR] Temporary workaround for calling the LLD ELF driver as-a-lib (authored by aganea).
[MLIR] Temporary workaround for calling the LLD ELF driver as-a-lib
Feb 8 2022, 4:12 PM
aganea closed D119278: [LLD] Fix issue in HIP toolchain due to unspecified order of evaluation of the function object.
Feb 8 2022, 4:12 PM · Restricted Project
aganea closed D119277: [MLIR] Temporary workaround for calling the LLD ELF driver as-a-lib.
Feb 8 2022, 4:12 PM · Restricted Project
aganea updated the diff for D119277: [MLIR] Temporary workaround for calling the LLD ELF driver as-a-lib.

Use more generic lld::cleanup() instead of lld::elf::cleanup()

Feb 8 2022, 2:13 PM · Restricted Project
aganea added a comment to D119257: Revert "Re-land [LLD] Remove global state in lldCommon".

@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

Feb 8 2022, 1:28 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
aganea added inline comments to D119277: [MLIR] Temporary workaround for calling the LLD ELF driver as-a-lib.
Feb 8 2022, 1:03 PM · Restricted Project
aganea updated the diff for D119277: [MLIR] Temporary workaround for calling the LLD ELF driver as-a-lib.

namespace fix.

Feb 8 2022, 1:00 PM · Restricted Project
aganea added a comment to D119257: Revert "Re-land [LLD] Remove global state in lldCommon".

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 proper fix over the next week or so.

Feb 8 2022, 12:51 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
aganea requested review of D119278: [LLD] Fix issue in HIP toolchain due to unspecified order of evaluation of the function object.
Feb 8 2022, 12:46 PM · Restricted Project
aganea requested review of D119277: [MLIR] Temporary workaround for calling the LLD ELF driver as-a-lib.
Feb 8 2022, 12:42 PM · Restricted Project
aganea added a comment to D119257: Revert "Re-land [LLD] Remove global state in lldCommon".

@krzysz00 I will revert and re-land with both fixes.

Feb 8 2022, 10:54 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
aganea added a comment to D108850: [LLD] Remove global state in lldCommon.

@yaxunl I have a fix, I will send a patch later today.

Feb 8 2022, 8:55 AM · Restricted Project, Restricted Project, lld
aganea accepted D119181: [Support] Don't print stacktrace if DbgHelp.dll hasn't been loaded yet.

LGTM with a comment:

Feb 8 2022, 5:30 AM · Restricted Project

Feb 7 2022

aganea added a comment to D119181: [Support] Don't print stacktrace if DbgHelp.dll hasn't been loaded yet.

Maybe take a look at llvm/unittests/Support/ProgramTest.cpp (for example TestExecuteAndWaitTimeout). You should be able to start the executable again with an extra env. variable which prevents the call to PrintStackTraceOnErrorSignal in llvm/utils/unittest/UnitTestMain/TestMain.cpp.

Feb 7 2022, 1:16 PM · Restricted Project
aganea added a comment to D119181: [Support] Don't print stacktrace if DbgHelp.dll hasn't been loaded yet.

Do you think you can add a unit test (in SupportTests) that demonstrates the issue before your patch?

Feb 7 2022, 1:05 PM · Restricted Project
aganea added a comment to D119009: [Support] Ensure handlers are set up before printing the stacktrace.

In that case @werat can you get the caller application to call llvm::InitLLVM? That plus the snippet in https://reviews.llvm.org/D119009#3297591 should shield from crashes if the handler isn't installed.

Feb 7 2022, 11:44 AM · Restricted Project, Restricted Project
aganea added a comment to D119009: [Support] Ensure handlers are set up before printing the stacktrace.

Could you please apply clang format just for the changed lines? You can do that usually through git clang-format.

Feb 7 2022, 8:51 AM · Restricted Project, Restricted Project

Feb 4 2022

aganea requested review of D119049: WIP: Allow using LLD as a lib.
Feb 4 2022, 6:08 PM · Restricted Project
aganea added a comment to D119009: [Support] Ensure handlers are set up before printing the stacktrace.

I'm not sure it's a good idea to start loading DLLs when we get to this point.
I would rather suggest calling sys::PrintStackTraceOnErrorSignal({}); in SBDebugger::Initialize(). "argv0" will be auto-detected if necessary, otherwise if you can, change the SBDebugger::Initialize() API to pass it? It's only there so that we can find llvm-symbolizer next to the running executable.
You can also add something along those lines to avoid the crash:

static bool isDebugHelpInitialized(void) {
   return fStackWalk64 && fSymInitialize && fSymSetOptions && fMiniDumpWriteDump;
}
...
static void PrintStackTraceForThread(llvm::raw_ostream &OS, HANDLE hProcess,
                                     HANDLE hThread, STACKFRAME64 &StackFrame,
                                     CONTEXT *Context) {
  if (!isDebugHelpInitialized())
    return;
Feb 4 2022, 11:14 AM · Restricted Project, Restricted Project

Feb 2 2022

aganea added a comment to D118070: Make lld-link work in a non-MSVC shell.

I know that you want a place to be accessed by both clang driver and lld-link but I am a bit nervous about the clang-driver style MSVC library sitting inside llvm/lib/Support/.
Is there a better place? @compnerd @aganea

Feb 2 2022, 6:11 AM · Restricted Project, Restricted Project

Jan 31 2022

aganea accepted D118606: Add llvm-pdbutil in LLVM_TOOLCHAIN_TOOLS.

Thanks @hans!

Jan 31 2022, 7:56 AM · Restricted Project
aganea added inline comments to D118428: [clang-cl] Support the /JMC flag.
Jan 31 2022, 6:15 AM · Restricted Project, Restricted Project

Jan 30 2022

aganea committed rGdc3b9365b66e: [mlir] Silence warnings when building with MSVC (authored by aganea).
[mlir] Silence warnings when building with MSVC
Jan 30 2022, 2:32 PM
aganea closed D118536: [mlir] Silence warnings when building with MSVC.
Jan 30 2022, 2:31 PM · Restricted Project
aganea added inline comments to D118536: [mlir] Silence warnings when building with MSVC.
Jan 30 2022, 7:19 AM · Restricted Project

Jan 29 2022

aganea added a reviewer for D118428: [clang-cl] Support the /JMC flag: mstorsjo.

Cool! :) Seems good generally.
Sounds like an expensive flag for the runtime perf :-D But I guess it makes the debugging experience a bit nicer.

Jan 29 2022, 6:30 AM · Restricted Project, Restricted Project
aganea added inline comments to D118536: [mlir] Silence warnings when building with MSVC.
Jan 29 2022, 5:38 AM · Restricted Project
aganea updated the summary of D118536: [mlir] Silence warnings when building with MSVC.
Jan 29 2022, 5:34 AM · Restricted Project
aganea requested review of D118536: [mlir] Silence warnings when building with MSVC.
Jan 29 2022, 5:33 AM · Restricted Project

Jan 28 2022

aganea committed rG1cf9876661a1: [mlir] Fix build after 83d59e05b201 (authored by aganea).
[mlir] Fix build after 83d59e05b201
Jan 28 2022, 2:22 PM
aganea closed D118510: [mlir] Fix build after 83d59e05b201.
Jan 28 2022, 2:21 PM · Restricted Project
aganea requested review of D118510: [mlir] Fix build after 83d59e05b201.
Jan 28 2022, 2:14 PM · Restricted Project
aganea added a comment to D108850: [LLD] Remove global state in lldCommon.

`@aganea This change broke SerializeToHsaco in MLIR, can you help resolve this issue or revert? See https://github.com/llvm/llvm-project/blob/main/mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp#L440

I can't quite tell what the new boolean arguments mean.

Fixing now.

Jan 28 2022, 1:39 PM · Restricted Project, Restricted Project, lld

Jan 26 2022

aganea added a comment to D110450: [LLD] Remove global state in lld/COFF.

thread_local performance is fine on aarch64 but has a higher cost (hidden visibility doesn't help) on other architectures (especially x86-64 which many people are concerned) because llvm-project compiles lld files with -fPIC which uses traditional GD/LD TLS models which are slow:(

It isn't particularly cheap on the Windows ABI either: https://godbolt.org/z/frEe87zcq - however I/O dominates in LLD right now, so using thread_local for ctx doesn't really change the runtime performance of LLD, at least on Windows/x64: https://reviews.llvm.org/D108850?id=370678#2975472

Jan 26 2022, 11:52 AM · Restricted Project
aganea added a comment to D110450: [LLD] Remove global state in lld/COFF.

If other ports want to follow suit, I think storing const COFFLinkerContext &ctx; in classes like *File are fine since the instances are not too many.

Placing the member in *Section is definitely too wasteful. For classes like ImportThunkChunk, I think it'd be better to avoid storing the member as well.

@akhuang mentioned that this patch only increases by 4 MB the peak mem when linking a Chromium .dll with /Gy, see above https://reviews.llvm.org/D110450#3028764
Since the COFF driver would still not be thread-safe after this patch, if you prefer we can also use lld::context<COFFLinkerContext>() (instead of storing ctx in *ImportThunkChunk). And then as a second step, introduce thread_local contexts, for cases like this where passing ctx through the callstack is infeasible/requires too many changes. See https://reviews.llvm.org/D108850?id=370678, lld/Common/Globals.cpp, L44.

Jan 26 2022, 11:17 AM · Restricted Project
aganea accepted D110450: [LLD] Remove global state in lld/COFF.

Still looks good. Perhaps if you can, before commit to ensure there are no regressions, check that the contents of Chromium's out/ dir is the same before and after this patch? (or any other real-world example)

Jan 26 2022, 10:26 AM · Restricted Project

Jan 24 2022

aganea accepted D118072: [X86] [CodeView] Add codeview mappings for registers ST0-ST7.

The feature can still be enabled with -mllvm -experimental-debug-variable-locations=true, so D116821 wasn't completely reverted.
(l
Why not adding your initial bug repro as a test, along with the flags above on the cmd line? It also looks like that prior mappings were added without test coverage.

Ok, I made such a testcase. I'm a bit afraid that it might be a little brittle in the long run though...

I was simply surprised that a such simple testcase wasn't catched elsewhere. Could you please explain 'brittle'? Do you think the outcome of this test isn't stable in the long run?
In any case LGTM.
I'll let the choice (to add a test or not) to your discretion ;-) I felt that it could make you loose time again later down the line...

Jan 24 2022, 3:25 PM · Restricted Project
aganea added a comment to D118072: [X86] [CodeView] Add codeview mappings for registers ST0-ST7.

The feature can still be enabled with -mllvm -experimental-debug-variable-locations=true, so D116821 wasn't completely reverted.

Jan 24 2022, 1:28 PM · Restricted Project
aganea added a comment to D116821: [DebugInfo][InstrRef] Move instr-ref controlling flag out of TargetOptions.

Thanks @aganea! Would you mind posting such a patch for review, or should I? I’m not sure how to create a simple enough and robust testcase for it, but maybe it’s not strictly necessary when it’s just adding more enum entries in a table?

Feel free to go ahead with a patch!

Jan 24 2022, 11:19 AM · Restricted Project, Restricted Project
aganea added inline comments to D110450: [LLD] Remove global state in lld/COFF.
Jan 24 2022, 7:37 AM · Restricted Project

Jan 21 2022

aganea added inline comments to D110450: [LLD] Remove global state in lld/COFF.
Jan 21 2022, 6:52 PM · Restricted Project
aganea added a comment to D116821: [DebugInfo][InstrRef] Move instr-ref controlling flag out of TargetOptions.

I ran into a regression with this patch, which triggers the error "fatal error: error in backend: unknown codeview register ST0" on code that built fine before that.

Here's a reduced reproducer:

$ cat dither.c 
a() {
  long double b;
  asm volatile("" : "=t"(b));
}
$ clang -target x86_64-w64-mingw32 -c -O2 -g -gcodeview dither.c 
fatal error: error in backend: unknown codeview register ST0
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: bin/clang -target x86_64-w64-mingw32 -c -O2 -g -gcodeview dither.c
1.	<eof> parser at end of file
2.	Code generation
3.	Running pass 'Function Pass Manager' on module 'dither.c'.
4.	Running pass 'X86 Assembly Printer' on function '@a'

Do @rnk or @aganea have any clue about this?

Jan 21 2022, 10:29 AM · Restricted Project, Restricted Project
aganea accepted D110450: [LLD] Remove global state in lld/COFF.

Thanks again Amy! LGTM with a note:

Jan 21 2022, 5:17 AM · Restricted Project

Jan 20 2022

aganea committed rG83d59e05b201: Re-land [LLD] Remove global state in lldCommon (authored by aganea).
Re-land [LLD] Remove global state in lldCommon
Jan 20 2022, 11:53 AM
aganea updated subscribers of D110450: [LLD] Remove global state in lld/COFF.

Hello @akhuang! Are you able to work on this before the 14.0 branch out? If no, can I take it over and land it? Barring any comments - @MaskRay would you have a bit of time to take a look please ?

Jan 20 2022, 11:48 AM · Restricted Project
aganea committed rG5fa4cf82dfa0: [Clang] Separate the 'debug-info-hotpatch' test in two parts: one for ARM and… (authored by aganea).
[Clang] Separate the 'debug-info-hotpatch' test in two parts: one for ARM and…
Jan 20 2022, 11:11 AM
aganea committed rG5af2433e1794: [clang-cl] Support the /HOTPATCH flag (authored by aganea).
[clang-cl] Support the /HOTPATCH flag
Jan 20 2022, 9:57 AM