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

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexander-shaposhnikov added inline comments.


================
Comment at: lld/ELF/Arch/AArch64.cpp:578
+  // always appear in pairs.
+  for (size_t i = 0, size = relocs.size(); i + 1 < size; ++i) {
+    if (relocs[i].type == R_AARCH64_LD64_GOT_LO12_NC) {
----------------
MaskRay wrote:
> `i + 1 < size` has a bug that the last relocation is not checked.
> 
> If `i` is moved outside the loop one single `safeToRelaxAdrpLdr = i == size;` can be used.
> 
> That said, the same check has been repeated in `tryRelaxAdrpLdr`. Maybe `AArch64Relaxer` is not needed.
that was intentional (potentially not checking the last relocation), but to be rigorous we can do that.


================
Comment at: lld/ELF/Arch/AArch64.cpp:595
+
+static bool withinRange(uint64_t lhs, uint64_t rhs, uint64_t range) {
+  uint64_t distance = lhs > rhs ? lhs - rhs : rhs - lhs;
----------------
MaskRay wrote:
> `inBranchRange` names the parameters src and dst.
> 
> Inline the function (only called once) into the caller.
in follow up patches this will be used with other constants (not 4gb), that's why this helper function is useful.


================
Comment at: lld/test/ELF/aarch64-adrp-ldr-got.s:81
+.word 10
+.space 1048640, 0
+.text
----------------
MaskRay wrote:
> This uses unreliable dependency on LLD's layout algorithm. Second, this creates very large input and output files with zeros. 
> 
> Prefer a linker script a la ppc64-long-branch-pi.s ppc64-pcrel-long-branch.s
using a linker script is a good idea, will switch to it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112063