[PowerPC][NFC] Don't need to and instruction with inverse of mask.

[sfertile]

Summary

The bits to be relocated should already contain all zeros, no need to and the call instructions with the inverse mask before relocating.

[MaskRay]

lld/ELF/Arch/PPC64.cpp

976

This existing code have one fewer assumption. What's the motivation behind the change?

[sfertile]

lld/ELF/Arch/PPC64.cpp

976

The motivation is consistency: We don't zero out the bits before writing them in the handling of any of the other relocations, we assume the bits to be relocated already contain zeros.

[MaskRay]

Looks great!

[MaskRay]

Shall we mask out the bits (keep existing (read32(loc) & ~mask))? If other relocations don't behave this way, let's fix it. Writing zeros to the bits is a courtesy of the assembly but should we make use of the assumption? PPC64 is a RELA target and for an RELA target the implicit addend should be relied upon. AArch64.cpp masks out the bits as well.

[MaskRay]

Cleared the LGTM state.

[sfertile]

In D81106#2090619, @MaskRay wrote:

Shall we mask out the bits (keep existing (read32(loc) & ~mask))? If other relocations don't behave this way, let's fix it. Writing zeros to the bits is a courtesy of the assembly but should we make use of the assumption? PPC64 is a RELA target and for an RELA target the implicit addend should be relied upon. AArch64.cpp masks out the bits as well.

My concern was how the linker being nice lead to a misunderstanding of how to encode an instruction. I've left a more detailed comment in the PCREL34 review so we can keep the discussion there and I can either abandon this or commit it depending on which way the discussion goes.

[sfertile]

Abandoning based on the discussion in D81457.

lld/ELF/Arch/PPC64.cpp

void PPC64::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
   case R_PPC64_REL64:
   case R_PPC64_TOC:
     write64(loc, val);
     break;
   case R_PPC64_REL14: {
     uint32_t mask = 0x0000FFFC;
     checkInt(loc, val, 16, rel);
     checkAlignment(loc, val, 4, rel);
-    write32(loc, (read32(loc) & ~mask) | (val & mask));
+    write32(loc, read32(loc) | (val & mask));
     break;
   }
   case R_PPC64_REL24: {
     uint32_t mask = 0x03FFFFFC;
     checkInt(loc, val, 26, rel);
     checkAlignment(loc, val, 4, rel);
-    write32(loc, (read32(loc) & ~mask) | (val & mask));
+    write32(loc, read32(loc) | (val & mask));
     break;
   }
   case R_PPC64_DTPREL64:
     write64(loc, val - dynamicThreadPointerOffset);
     break;
   default:
     llvm_unreachable("unknown relocation");
   }