[PATCH] D81457: [LLD][PowerPC] Add support for R_PPC64_PCREL34

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Arch/PPC64.cpp:1011
+    uint64_t si1 = (val & si1Mask);
+    uint64_t instr = readPrefixedInstruction(loc) & ~fullMask;
+
----------------
sfertile wrote:
> I've got a patch which removes the masking of the instruction bits on the other relocations types we were doing it for [[ https://reviews.llvm.org/D81106 | (D81106)]].  My understanding is that for REL ELF archs the addend is encoded into the bits to be relocated, and for RELA ELF archs the bits to be relocated are zeros, with the addend in the relocation. The extra masking doesn't hurt anything functionally but it can lead to misunderstandings on what the bits to be relocated should  [[ https://reviews.llvm.org/D81082 | contain]] so I think its important to be precise and omit the extra masking despite it not hurting anything. 
Even if powerpc is a RELA target, if it is easy to drop the requirement, I hope we can make some efforts making that work. See D80496 (-z rel)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81457