[PATCH] D54720: [PPC64] toc-indirect to toc-relative relaxation.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.
Herald added a subscriber: jdoerfert.
Herald added a project: LLVM.

The patch needs a rebase.



================
Comment at: ELF/Arch/PPC64.cpp:123
+    Symbol &Sym = TocSec->getFile<ELFT>()->getRelocTargetSym(Rela);
+    return {dyn_cast_or_null<Defined>(&Sym), getAddend<ELFT>(Rela)};
+  };
----------------
`Sym` as a reference can't be null, so you can use `dyn_cast` instead of `dyn_cast_or_null`.


================
Comment at: ELF/Arch/PPC64.cpp:126
+
+  // The relocations are sorted by offset, and while the most common case is
+  // that every .toc entry will have a relocation, it is not guaranteed. For
----------------
> The relocations are sorted by offset

IIRC this is not guaranteed but I think it is fine to lose toc optimization if the input does not have this property.

The linear search below `for (auto R : llvm::reverse(Relas.slice(0, Index))) {` concerns me. 

Is it possible to add a `LastRelaIndex` variable in `InputSectionBase::relocateAlloc`:

      if (!tryRelaxTocPPC64(Type, Rel, Expr, BufLoc, LastRelaIndex))

and keep it monotonically increasing in `getSymAndAddend`? If the input doesn't guarantee increasing `r_offset`, it may lose TOC optimizations for some relocations, but as I said it should be fine.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D54720