[PATCH] D85782: [X86][ELF] Prefer lowering MC_GlobalAddress operands to .Lfoo$local only for STV_DEFAULT globals

Tue Aug 11 14:35:56 PDT 2020

MaskRay added a comment.

The additional test file places too much burden on people maintaining the tests in the future. I think an additional `@hidden_data = hidden global` in `CodeGen/X86/linux-preemption.ll` should be sufficient.

> GNU now recommends -fvisiblity=hidden (https://gcc.gnu.org/wiki/Visibility) so my understanding is that there will be very few STV_DEFAULT symbols.

I am not very sure this is now recommended but is indeed a non-rare configuration we should care about.

> Improves the assembly (as there are fewer .Lfoo$local labels).

There are still plenty of cases with the `.Lfoo$local`. I'd not state this advantage explicitly in this case.

> Restores the effectiveness of the --wrap linker option and makes the behavior of --wrap consistent between LTO and normal builds for references within a translation-unit.

I think we should explicitly call out that this is still brittle. It is just a side benefit. (I know for you it is a major thing but I hope we don't give us false impression that this is reliably supported.) The main thing is the .symtab saving.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85782/new/

https://reviews.llvm.org/D85782