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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Arch/PPC64.cpp:110
 
+// Looks up the symbol and addend that a .toc indirection refers to.
+template <typename ELFT>
----------------
It feels this comment can be improved. IIUC, this function scans relocations for a given section to find one whose offset is the same as a given offset. Is this correct? If so, can you add something like that as a comment?


================
Comment at: ELF/Arch/PPC64.cpp:114
+                                                     uint64_t Offset) {
+  ArrayRef<typename ELFT::Rela> Relas = TocSec->template relas<ELFT>();
+  if (Relas.size() == 0)
----------------
I'd define an alias at the beginning of this function like this:

  typedef typename ELFT::Rela RelTy;


================
Comment at: ELF/Arch/PPC64.cpp:120
+  auto SymAndAddend =
+      [TocSec](typename ELFT::Rela Rela) -> std::pair<Defined *, int64_t> {
+    Symbol &IndirectSym = TocSec->getFile<ELFT>()->getRelocTargetSym(Rela);
----------------
When we define a local helper lambda function inside a function, we capture variables just by `[&]` instead of explicitly enumerating variables.


================
Comment at: ELF/Arch/PPC64.cpp:121
+      [TocSec](typename ELFT::Rela Rela) -> std::pair<Defined *, int64_t> {
+    Symbol &IndirectSym = TocSec->getFile<ELFT>()->getRelocTargetSym(Rela);
+    return {dyn_cast_or_null<Defined>(&IndirectSym), getAddend<ELFT>(Rela)};
----------------
nit: use shorter variable name for a variable with just two lines of scope.


================
Comment at: ELF/Arch/PPC64.cpp:171
+
+bool elf::tryRelaxTocPPC64(RelType Type, const Relocation &Rel, RelExpr Expr,
+                           uint8_t *BufLoc) {
----------------
Can you add a high-level comment as to what optimization this function is supposed to do?


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D54720