I didn't even know that you could pass a blob that way. What do you think about dropping support for this feature since it is evidently unused (except possibly by you? But you can switch to EmitRecordWithBlob, right?)

The MAttrs part is fine but we don't want CGOptLevel to be set based on the LTO opt level but rather on the opt level passed at compile time. See mailing list thread "[RFC] Adding function attributes to represent codegen optimization level" for more information.

Signature changes are fine for this package, the API guarantees are the same as for the C bindings: https://llvm.org/docs/DeveloperPolicy.html#c-api-changes

Can you update this patch please? Then I will commit.

LGTM

LGTM

LGTM

Please update the commit message to reflect what this patch is now doing.

It's part of the key under which executables get uploaded to symbol servers (the key is executable name, timestamp, size: https://cs.chromium.org/chromium/src/tools/clang/scripts/package.py?type=cs&q=img_fingerprint&sq=package:chromium&g=0&l=119).

Admittedly though there isn't a regression and as you point out this output is compliant with the COFF spec so I'd be fine with this too.

I wasn't imagining that we'd write 32 bits of hash to the repro directory but rather the 64-bit xxhash that we currently truncate to 32 bits to get it to fit into the timestamp field. Without that, isn't there a high probability of collisions after 65536 files (which could be small depending on the project)?

I can reproduce your result with the 14.14 toolset:

/mnt/c/src/tmp$ cl.exe /Zi /Brepro main.c
Microsoft (R) C/C++ Optimizing Compiler Version 19.14.26433.3 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

Interesting, maybe it was a bug in 14.14.26428.1 that was fixed in 14.15.26726.0?

Same result.

/mnt/c/src/tmp$ cl.exe /Zi /Brepro main.c
Microsoft (R) C/C++ Optimizing Compiler Version 19.15.26726 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

link.exe creates a zero-length repro debug directory entry with /debug /Brepro

Did you consider changing the type id mapping data structure in ModuleSummaryIndex instead of creating a new one? Since we now use a mapping from guids in several places it seems sensible to do that. I think that it would work to change the type of TypeIdMap to something like:

DenseMap<GlobalValue::GUID, TinyPtrVector<std::pair<std::string, TypeIdSummary>>>

@srhines Ping.

I think this is the way that you are supposed to do it. It's the way that we're doing it elsewhere.
http://llvm-cs.pcc.me.uk/?q=createMCInstrAnalysis

LGTM

D51199 fixes the above breakage as well as crbug.com/877235. Once it lands, I'll reland this change.

I received another report of breakage so I reverted in rC340579.

Thanks, I'll take a look.

• Add error handling

With ELF it was around 0.1% for a self-host of Clang. I also checked the overhead with COFF for Chromium's base_unittests and it was around 0.2% without debug info. I think that's small enough that we can turn this on by default.

LGTM

Hmm, I was aware of those races, but it's a little surprising that they would be the cause of a significant number of files being left around (since the files exist with their temporary names for the duration that the LLVM backend is running). I could buy that the Windows filesystem is slow enough that the sum of the racy windows would make up a significant fraction of that time, though.

The temporary files are deliberately named differently from the cache entries in order to prevent cache pruners from racing with processes that create cache files. If we allow these deletions it would have the effect of causing the call to TempFile::keep to sometimes return no such file or directory, so I'd at least expect to see code handling that case.

LGTM

LGTM

LGTM

I'm not sure, I'm not an expert on the Linux build system. There's probably some link rule somewhere that contains something like -o $@, maybe you can change it to say -o $@ -plugin-opt=obj-path=$@.o?

In rL339799#283354, @yo wrote:

The test file: test/tools/llvm-readobj/Inputs/elf-packed-relocs3.s is missing, why?

LGTM except for a few spelling/grammar nits.

Please make sure that your patch is formatted to match the LLVM style. clang-format can do that for you.

obj-path should produce the same order as save-temps because they use the same code path -- they both work by setting Filename to a non-empty string, which causes us to follow the code path that appends task identifiers and use non-temporary files. If you're seeing something different, maybe that should be fixed instead?

Doesn't the obj-path option already do what you want here?