[PATCH] D70690: [ELF][ARM] Add getPCBias()

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D70690#1759922 <https://reviews.llvm.org/D70690#1759922>, @peter.smith wrote:

> Is there any reason not to have just one function inBranchRange() and assume that it needs to take care of the relocation addend explicitly, that way the PC Bias could be limited to the ARM implementation of inBranchRange() and inBranchRangeBeforePCBias wouldn't be needed?


If we require that the call sites of `inBranchRange()` account for both the addend and the PC Bias (e.g. `target->inBranchRange(rel.type, src, t->getThunkTargetSym()->getVA(rel.addend) + getPCBias(rel.type))`), I find that we just need to adjust `ThunkCreator::getThunk` and `ThunkCreator::normalizeExistingThunk`.

> The only other call sites that I could find that would need to be adjusted are: 
>  In Relocations.cpp the relocation addend could be passed to getISDThunkSec (line 1634) so that it can be passed through to the two calls to target->inBranchRange() in there.
>  In ARMErrataFix.cpp we are generating the branch instruction  (it is always Thumb and its addend is hard-coded to -4) so we could easily account for this at the call site.
>  needsThunk() would either need the addend to pass through to inBranchRange(). In D70637 <https://reviews.llvm.org/D70637> you are passing in the addend, in this patch it might be simple enough to say if Arm Relocation pass in -8, if thumb pass in -4.

The two places will not be needed to be adjusted.

> It maybe that across this patch and D70637 <https://reviews.llvm.org/D70637> we can convert all the callsites so that there is just inBranchRange() which expects the relocation addend to have been accounted for in dst.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70690