[LLD][PowerPC] Add support for R_PPC64_GOT_PCREL34

[stefanp]

Summary

Add support for the 34bit relocation R_PPC64_GOT_PCREL34 for PC Relative in LLD.

[sfertile]

lld/ELF/Arch/PPC64.cpp

1017

@MaskRay comments on the other patch about lowercase hex literals applies here as well. Ditto for the suggested test change (ie labels instead of sections)

lld/test/ELF/ppc64-got-pcrel34-reloc.s

33 ↗

(On Diff #271128)

It this test just to show the linker doesn't modify the load instruction that follows the PC-relative load from the got?

[MaskRay]

lld/ELF/Arch/PPC64.cpp

1017

In https://reviews.llvm.org/D81986#inline-753874 @grimar asked whether we should use const. It seems to me that is a good idea. Many other components of LLVM use const whenever appropriate.

lld/test/ELF/ppc64-got-pcrel34-reloc.s

1 ↗

(On Diff #271128)

The naming is ppc64-reloc-* (x86-64-reloc- tests stick with this rule more consistently)

18 ↗

(On Diff #271128)

Consider using a label instead of a .section as the anchor in the output. This allows you to use -NEXT: which makes tests easier to update when something goes off.

# CHECK:      <foo>:
# CHECK-NEXT:   pld 3, ...   # I tend to indent instructions to make it clear the region is covered by the previous label

Omit unneeded addresses unless you do some arithmetic and somewhere else in the test requires exact addresses. In that case, you should write the formula in a comment (example: ppc64-tls-*.s)

33 ↗

(On Diff #271128)

If you want to check not TOC (a variant of GOT) optimization is performed, use -no-pie (default) or -pie, instead of -shared.

44 ↗

(On Diff #271128)

This creates a large file. Use a linker script like ppc32-long-thunk.s

[stefanp]

lld/test/ELF/ppc64-got-pcrel34-reloc.s

33 ↗

(On Diff #271128)

Yes, this test is just to make sure that we don't make changes to the lwa. It may not be necessary from that perspective because that instruction does not have a relocation on it so there really is no risk of this problem. However, in my head they are the part of the same set so I want to check them both.

I'm not trying to check for not TOC. We have removed all of the TOC references on the compiler side. Or, maybe I didn't understand your comment...

[stefanp]

Addressed review comments.

[MaskRay]

LGTM, with one last nit. Please wait for @sfertile's approval.

lld/test/ELF/ppc64-reloc-got-pcrel34.s

28

It seems strange that lwa is indented differently. Please fix it.

[sfertile]

LGTM.

lld/ELF/Arch/PPC64.cpp

1017

Big +1 to judiciously adding const wherever appropriate.

lld/test/ELF/ppc64-got-pcrel34-reloc.s

33 ↗

(On Diff #271128)

If it was to test we don't try to perform toc-optimizations then I was going to say you need to make it an executable like MaskRay suggested. But since its to make sure we don't modify past the end of the prefixed instructions its good the way you have it, this test won't change when you do go to implement the got optimizations which is how we want it I think 👍

lld/test/ELF/ppc64-reloc-got-pcrel34.s

18

If the 'SYMBOL' checks are to verify the referenced symbols make it into the dynamic symbol table maybe have a first check line:

SYMBOL: Symbol table '.dynsym' contains 4 entries:

[stefanp]

Updated with last nits.

lld/ELF/Arch/PPC64.cpp

RelExpr PPC64::getRelExpr(RelType type, const Symbol &s,
   case R_PPC64_GOT16_LO:
   case R_PPC64_GOT16_LO_DS:
     return R_GOT_OFF;
   case R_PPC64_TOC16:
   case R_PPC64_TOC16_DS:
   case R_PPC64_TOC16_HI:
   case R_PPC64_TOC16_LO:
     return R_GOTREL;
+  case R_PPC64_GOT_PCREL34:
+    return R_GOT_PC;
   case R_PPC64_TOC16_HA:
   case R_PPC64_TOC16_LO_DS:
     return config->tocOptimize ? R_PPC64_RELAX_TOC : R_GOTREL;
   case R_PPC64_TOC:
     return R_PPC64_TOCBASE;
   case R_PPC64_REL14:
   case R_PPC64_REL24:
     return R_PPC64_CALL_PLT;
case R_PPC64_PCREL34: {
     const uint64_t fullMask = 0x0003ffff0000ffff;
     checkInt(loc, val, 34, rel);
 
     uint64_t instr = readPrefixedInstruction(loc) & ~fullMask;
     writePrefixedInstruction(loc, instr | ((val & si0Mask) << 16) |
                              (val & si1Mask));
     break;
   }
+  case R_PPC64_GOT_PCREL34: {
+    const uint64_t si0Mask = 0x00000003ffff0000;
+    const uint64_t si1Mask = 0x000000000000ffff;
+    const uint64_t fullMask = 0x0003ffff0000ffff;
+    checkInt(loc, val, 34, rel);
+
+    uint64_t instr = readPrefixedInstruction(loc) & ~fullMask;
+    writePrefixedInstruction(loc, instr | ((val & si0Mask) << 16) |
+                             (val & si1Mask));
+    break;
+  }
   default:
     llvm_unreachable("unknown relocation");
   }
 }
 
 bool PPC64::needsThunk(RelExpr expr, RelType type, const InputFile *file,
                        uint64_t branchAddr, const Symbol &s, int64_t a) const {
   if (type != R_PPC64_REL14 && type != R_PPC64_REL24)

lld/test/ELF/ppc64-reloc-got-pcrel34.s

+# REQUIRES: ppc
+# RUN: echo 'SECTIONS { \
+# RUN:       .text_low 0x10010000: { *(.text_low) } \
+# RUN:       .text_high 0x10080000 : { *(.text_high) } \
+# RUN:       }' > %t.script
+
+# RUN: llvm-mc -filetype=obj -triple=powerpc64le %s -o %t.o
+# RUN: ld.lld -T %t.script --shared %t.o -o %t
+# RUN: llvm-readelf -s %t | FileCheck %s --check-prefix=SYMBOL
+# RUN: llvm-readelf -r %t | FileCheck %s --check-prefix=RELA
+# RUN: llvm-objdump -d --no-show-raw-insn --mcpu=future %t | FileCheck %s
+
+# RUN: llvm-mc -filetype=obj -triple=powerpc64 %s -o %t.o
+# RUN: ld.lld -T %t.script --shared %t.o -o %t
+# RUN: llvm-readelf -s %t | FileCheck %s --check-prefix=SYMBOL
+# RUN: llvm-readelf -r %t | FileCheck %s --check-prefix=RELA
+# RUN: llvm-objdump -d --no-show-raw-insn --mcpu=future %t | FileCheck %s
+
+.text
+.section .text_low, "ax", %progbits
+# CHECK-LABEL: <GlobIntPCRel>:
+# CHECK-NEXT:    pld 3, 458928(0), 1
+# CHECK-NEXT:    lwa 3, 0(3)
+# SYMBOL:      00000000     0 NOTYPE  GLOBAL  DEFAULT     UND glob_int
+# RELA:        100800b0  0000000100000014 R_PPC64_GLOB_DAT       0000000000000000 glob_int + 0
+GlobIntPCRel:
+  pld 3, glob_int@got@PCREL(0), 1
+        lwa 3, 0(3)
+  blr
+
+# CHECK-LABEL: <GlobIntPCRelOffset>:
+# CHECK-NEXT:    pld 3, 458920(0), 1
+# CHECK-NEXT:    lwa 3, 8(3)
+# SYMBOL:      00000000     0 NOTYPE  GLOBAL  DEFAULT     UND glob_int8
+# RELA:        100800b8  0000000200000014 R_PPC64_GLOB_DAT       0000000000000000 glob_int8 + 0
+GlobIntPCRelOffset:
+  pld 3, glob_int8@got@PCREL(0), 1
+        lwa 3, 8(3)
+  blr
+
+# CHECK-LABEL: <GlobIntPCRelBigOffset>:
+# CHECK-NEXT:    pld 3, 192(0), 1
+# CHECK-NEXT:    lwa 3, 64(3)
+# SYMBOL:      00000000     0 NOTYPE  GLOBAL  DEFAULT     UND glob_int8_big
+# RELA:        100800c0  0000000300000014 R_PPC64_GLOB_DAT       0000000000000000 glob_int8_big + 0
+.section .text_high, "ax", %progbits
+GlobIntPCRelBigOffset:
+  pld 3, glob_int8_big@got@PCREL(0), 1
+        lwa 3, 64(3)
+  blr