LGTM

Thanks for the prompt reviews

LGTM

LGTM

LGTM

If this can be fixed without affecting any potential clients, then I am all for it. In hindsight it might have been better to make a use case specific calling convention instead and not a generic one that everyone could depend on. Oen possible solution would be to make a Chromium specific calling convention and also name it as such. This would provide clear expectations what it is intended for and how is supposed to use it. Furthermore this would give you the option of changing it in the future if the need should arise.

LGTM

Test case?

LGTM

We could invoke clang with the -stats option and compare the result against the expected number of stat calls.

LGTM

This doesn't affect any tests?

This makes sense. We shouldn't re-validate system headers when -fmodules-validate-once-per-build-session is passed too.

Seems straightforward. LGTM.

RISCV is not my expertise, but LGTM with respect to constant hoisting.

OpenMP pre-merge check issues seem unrelated to this change.

Thank you Pete

In D123831#3458774, @dang wrote:

In D123831#3455048, @cishida wrote:

we might not always want to transform an absolute path because the resulting relative include name might get remapped in a headermap, for example in test known_files_only_hmap.c. But how does it work with modules where we need relative includes? Is the setup in known_files_only_hmap even valid?

I think, in most cases, this shouldn't matter because if the header path input doesn't match the location stored in the header map, they should still have the same source content. The same should be true with header search resolution with modules & vfsoverlay

Agreed, I think it would be classified as a user error to remap to different source content via headermap.

LGTM

Nice! Thank you for adding support for multiple headers. Is this a step towards the json input file list?

LGTM. Just a few minor nits.

LGTM

The unit test failure on Windows seems unrelated, because I have seen it in a previous pre-merge check too. Furthermore, this should be a NFC.

LGTM

LGTM

LGTM

LGTM

LGTM

SpareSet is deterministic given the same input order - similar to SetVector. Printing the result is not the common use case, so changing the print function to sort the output first might be more useful for testing purposes.

Thanks Cyndy. LGTM

Does nm print now all symbols - including the inlined ones? That would be an unexpected change, because we don't do the same for MachOs that have re-exported frameworks. I think this feature should be guarded by an option.

xxhash64 is apparently faster than MD5 and SHA1, and produces good quality hashes. I am not sure about the hash quality of hash_code for this purpose.

LGTM

LGTM

Did you try xxHash64?

LGTM

The motivation to add this to llvm-nm, is to get the same functionally back that we lost with adding TBD files. Using it for testing is a nice side benefit.

LGTM

LGTM, but we should also wait for Lang's feedback.

New version is tracked here: D66149

Sure, np

Hi Cyndy, thanks for catching this copy-past-o bug. LGTM

If I recall correctly, then getIntImmCost(const APInt &Imm) predates the ConstantHoisting pass and is also used by other optimizations. The purpose of that hook is to calculate the materialization cost of a constant and is context free.

Thanks for catching this. LGTM.