[PATCH] D86893: [PowerPC] Add support for R_PPC64_GOT_TPREL_PCREL34 used in TLS Initial Exec

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

Please mention the relevant sections of the ABI in the description (i.e. where it mentions the 1-byte offset). Other than that and the inline nits, LGTM. Please give @MaskRay some time to have another look before committing.



================
Comment at: lld/ELF/Arch/PPC64.cpp:863
+      // If the offset is not 4 byte aligned then we have a PCRel type reloc.
+      // This version of the relocation is offset by one byte form the
+      // instruction it references.
----------------
s/form/from


================
Comment at: lld/ELF/Arch/PPC64.cpp:865
+      // instruction it references.
+      uint32_t primaryOp = getPrimaryOpCode(read32(loc - 1));
+      if (primaryOp != 31)
----------------
We have three calls to `read32(loc - 1)`. I think that is a valid reason to have a temporary var.


================
Comment at: lld/ELF/Arch/PPC64.cpp:869
+      uint32_t secondaryOp = (read32(loc - 1) & 0x000007FE) >> 1; // bits 21-30
+      // The add is a special case and can be turned into a nop.
+      if (secondaryOp == 266)
----------------
Elaborate slightly. Why can it be turned into a `nop`? Presumably because the address computed by the `pld` points to the actual symbol now. Also, I imagine `can` should really be `must`.


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

https://reviews.llvm.org/D86893