[PATCH] D84360: [LLD][PowerPC] Implement GOT to PC-Rel relaxation

Victor Huang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 12 15:31:48 PDT 2020


NeHuang added a comment.

Overall the patch looks good to me. I only have two questions and a clang-format nit which can be resolved before committing the code.



================
Comment at: lld/ELF/Arch/PPC64.cpp:620
+    if ((insn & 0xfc000000) != 0xe4000000)
+      error ("expected a 'pld' for got-indirect to pc-relative relaxing");
+    insn &= ~0xff000000fc000000;
----------------
nit: clang-format, this can be resolved before committing the changes. 


================
Comment at: lld/ELF/Arch/PPC64.cpp:1272
+    assert(data && "Expecting an instruction encoding here");
+    if ((readPrefixedInstruction(data) & 0xfc000000) == 0xe4000000)
+      return R_PPC64_RELAX_GOT_PC;
----------------
I have two questions regarding `void PPC64::relaxGot(uint8_t *loc, const Relocation &rel, uint64_t val)` in `lld/ELF/Arch/PCC64cpp` line 619, where had similar check added as below:
```
case R_PPC64_GOT_PCREL34: {
    // Clear the first 8 bits of the prefix and the first 6 bits of the
...
    if ((insn & 0xfc000000) != 0xe4000000)
      error ("expected a 'pld' for got-indirect to pc-relative relaxing");
...
```

1. Did it catch the case that `paddi` instruction with `R_PPC64_GOT_PCREL34` emitted from compiler side when it tried to optimize it?
2. After adding the new check here, does it make sense to remove the same check in `PPC64::relaxGot`?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84360



More information about the llvm-commits mailing list