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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 16 10:38:09 PDT 2020


MaskRay added a comment.

In D85782#2276676 <https://reviews.llvm.org/D85782#2276676>, @bd1976llvm wrote:

> In D85782#2273938 <https://reviews.llvm.org/D85782#2273938>, @hans wrote:
>
>> In D85782#2273402 <https://reviews.llvm.org/D85782#2273402>, @bd1976llvm wrote:
>>
>>> @hans, I think that we should put this change onto the release branch for llvm11.
>>
>> I'm not familiar with the details of this patch, can you explain why we should merge it to llvm11?
>>
>> We're very late in the release process, so unless it's fixing some major problem, I would be hesitant to merge.
>
> Apologies - I failed to consider where LLVM is in the release process!
>
> This isn't needed to fix a major problem. However, it would be great to get this patch into a released LLVM ASAP to prevent users coming to rely on the behaviour prior to this patch. See report by @skan above. The patch should be very safe as it essentially restores the referencing behaviour to what it was prior to https://reviews.llvm.org/D73230 for symbol definitions that do not have default ELF visibility.

I testify that this patch is safe. With this patch the behavior is closer to GCC.

(skan's report is a GNU ld x86 bug and has been always the case in GCC x86 + GNU ld circa 2015)

> If you have to do another RC it would be worth considering merging this.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85782



More information about the llvm-commits mailing list