[PATCH] D157020: [WIP][lld/ELF] Don't relax R_X86_64_(REX_)GOTPCRELX when offset is too far

Wed Aug 9 13:13:16 PDT 2023

aeubanks added a comment.

In D157020#4571332 <https://reviews.llvm.org/D157020#4571332>, @MaskRay wrote:

> In D157020#4570667 <https://reviews.llvm.org/D157020#4570667>, @aeubanks wrote:
>
>> we'd like to have a single configuration across all builds so that people don't have to manually specify `-Wl,--[no-]relax`
>> the problems with `--no-relax` are:
>>
>> - increases `.rela.dyn` and `.got` section sizes
>
> The `.rela.dyn` and `.got` size increase is negligible, mostly in the scale of less than 0.1% VM size.
>
>> - causes regressions on some microbenchmarks
>>
>> that's why I'd like to make this work in general without users having to specify flags
>
> What microbenchmarks? Are they good indicators of real-world application performance?

I can show you exactly which ones internally (and they're consistent even with alignment), although I'll dig into it more first.

> Note, for relocation overflow mitigation, it's natural to have a separate configuration. You can utilize a linker script using `INSERT [AFTER|BEFORE]` or `--no-relax`.

I'd really like for users not to have to specify flags or linker scripts, those are all unfortunate workarounds when we can just fix this in lld.

>> if we limit the extra scanning pass to cases where there may be a relocation overflow, so that it doesn't run for most normal-sized binaries, is that still a concern? if other arches already do this sort of thunk creation/relaxation I don't see why x86-64 can't do it
>
> An extra relocation scanning pass adds conceivable overhead. I am not sure how the pass can be limited. In theory one can add relocation distance metric to somewhere in `TargetInfo::relocateAlloc`, but this would bring overhead as well.

I mean we can check the min/max virtual address and see if it's greater than 2^31. This seems pretty quick and easy and should handle most normal sized binaries, unless I'm missing something?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157020