[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