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

Thu Aug 6 19:08:55 PDT 2020

sfertile added inline comments.

================
Comment at: lld/ELF/Arch/PPC64.cpp:405
+  if (opc == 61 && (encoding & 0x3) == 0x1) // LXV/STXV.
+    opc |= (encoding & 0x7) << 29;
+
----------------
Instead of shifting the primary opcode down to the least significant bits, and then shifting the secondary opcodes into the more significant bits, why don't we just use the significant opcode bits as the enum value. 

For example: LDU  0xE8000001, LD = E8000000, LDU = 7C000000. It won't simplify this function since you still need the primary opcode to know how to mask out the encoding bits, but we could follow this patch with an NFC patch where we convert all the other uses of 
DFormOpcd and XFormOpcode enums to use PPCInsn and get rid of the other 2 enums. If we do take this root I would suggest having separate enums for the 32-bit instructions and the prefixed instructions.

================
Comment at: lld/ELF/Relocations.h:99
   R_PPC64_TOCBASE,
+  R_PPC64_RELAX_GOT_PCREL34,
   R_RISCV_ADD,
----------------
Should we drop the 'REL34' in the Expr? Its not really important how many bits it represents, and I think the REL is implied by 'PC' in `R_RELAX_GOT_PC`.

Also, should it be added to `isRelExpr` in `Relocations.cpp`?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84360