[PATCH] D112063: [lld][ELF] Add first bits to support relocation relaxations for AArch64

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Thanks for the updates, just a few small comments from me.



================
Comment at: lld/ELF/Arch/AArch64.cpp:652
+  // Check if the address difference is within 4GB range.
+  if (!within4GBRange(secAddr + adrpRel.offset, sym.getVA()))
+    return false;
----------------
Strictly speaking the calculation is getAArch64Page(secAddr + adrpRel.offset), getAArch64Page(sym.getVA()) where getAArch64Page is defined in Target.h I think in some corner cases this could permit a different range.

It may be worth mirroring the relocation calculation and using the same logic as checkInt (also in Target.h) just not the error message.

I think that this would be something like (we already know addend is 0):
```
int64_t val = getAArch64Page(sym.getVA()) - getAArch64Page(secAddr + adrpRel.offset);
if (val != llvm::SignExtend(val, 33))
  return false;
```
This doesn't quite fit into a general withinRange function, but I'm sure it could be adapted.


================
Comment at: lld/ELF/Options.td:304
 
+def no_relaxations: F<"no-relax">,
+  HelpText<"Disable relocation relaxations">;
----------------
Could you add the new option to the man page in lld/docs/ld.lld.1 ? This is really easy to miss.

We may need a follow up patch to make sure that it also turns off any other relaxations such as PPC and potentially RiscV.

I think the option name is the right one as BFD also has the same option name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112063