[clang] [lld] [llvm] [ThinLTO] Reduce the number of renaming due to promotions (PR #178587)

[Teresa Johnson via llvm-commits]

teresajohnson wrote:

> Just updated a new revision. The major change are
> 
> * The implemented feature will be on by default.

I think you mean "off" by default? Please do bring back the cl::opt so that we can disable it as necessary for debugging (we use '--lto-whole-program-visibility' extensively internally and would want a way to disable the new handling in case of issues in production builds).

> Adding linker flag '--lto-whole-program-visibility' can enable this feature.
> * Add more comments, fix a few minor issues, and rewrite for more cleanness.
> * Remove all previous changes in ThinLTOCodeGenerator.cpp. llvm-lto2 can be used to do testing.

ThinLTOCodeGenerator.cpp is legacy LTO handling but still used by a few linkers (ld64 and afaik Sony's internal linker). It can be tested with llvm-lto. However, it does not currently support  '--lto-whole-program-visibility' so I think it is ok to not support this there for now. I would just add a note to the PR description that this is only supported for the new LTO API for now.

> * Add a few unit test cases.

One request: please avoid force pushing changes when possible. It makes it impossible to use github's support for comparing to my last review or other old commits. See https://llvm.org/docs/GitHub.html#rebasing-pull-requests-and-force-pushes.

Also, I will be traveling and OOO much of the next week, so my response time may be delayed again. I have just a couple of other suggestions on the current version though after a quick skim that I'll send shortly.

https://github.com/llvm/llvm-project/pull/178587