In D94585#2524476, @madhur13490 wrote:

In D94585#2520966, @kzhuravl wrote:

This change is causing infinite loop when compiling rocThrust and hipCUB. Can you take a look? Thanks.

FWIW, after testing rocThrust and hipCUB, it turned out that, it is not an infinite loop but a rise in compile-time which crossed timing threshold of the internal infra. The apps eventually compiled with 1-2% increment in the compile-time. I figured out the cause behind this. This patch makes two additional calls to Function::hasAddressTaken() and hasAddressTaken() is not optimal. Each time Function::hasAddressTaken() is called, it traverses over all uses of the function to deduce the required info. The calls made by this patch are itself in the loop which effectively made the suboptimality of Function::hasAddressTaken() conspicuous. In the new patch I am going to remove one call to Function::hasAddressTaken() which still preserves the intention of this patch. The new patch would basically be Diff3 of this patch.

We should think about optimizing Function::hasAddressTaken() by introducing a bool in Function class which would cache the result. In addition to this, Function::hasAddressTaken() can accept an additional bool parameter and based on its truthness, the function can return the cached value. The default value of parameter bool should be false to preserve today's behavior but client can set it to "true" if they want the cached value. Latter would be useful for this patch as the information is unlikely to change in a module (which is static naturally). This optimization would significantly improve the run-time of hasAddressTaken().

LGTM

This change is causing infinite loop when compiling rocThrust and hipCUB. Can you take a look? Thanks.

LGTM

Any progress on this? Does it need another rebase?

lgtm

lgtm

lgtm

lgtm

lgtm

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

Addressed review feedback

Address review feedback:

• Merge tests into one test file

Address review feedback:

• Regenerate docs in separate change

LGTM

LGTM

LGTM, thank you!

sramecc is off by default for gfx906:
https://github.com/llvm/llvm-project/blob/master/llvm/test/CodeGen/AMDGPU/elf-header-flags-sram-ecc.ll#L5

Might want to mention that it is being corrected for gfx906 in the commit message

LGTM

LGTM

need to handle gfx10

lgtm

Do we also want to remove it from v2 metadata?

LGTM

In D81780#2091213, @kzhuravl wrote:

Changes to AMDGPUUsage are WIP.

In D81780#2091280, @MaskRay wrote:

lib/ObjectYAML/ELFYAML.cpp and tools/llvm-readobj/ELFDumper.cpp changes can be made to a separate patch. They are not tightly coupled with the MC/CodeGen changes. A group of binary format focused people will be interested to review that part.

LGTM. This was reviewed downstream. Additional review feedback should be addressed in the follow up patch to simplify the merge downstream. Thank you.

Changes to AMDGPUUsage are WIP.

lgtm

LGTM