[PPC64] Optimize redundant instructions using R_PPC64_TOC16_HA in nop

[sfertile]

Summary

This patch optimizes the global variable access sequence when R_PPC64_TOC16_HA evaluates to 0.
The addis that the R_PPC64_TOC16_HA relocation operates on is overwritten with a nop and the related instruction that uses either a *_LO or *_LO_DS form is update to use the toc pointer a base register.

[sfertile]

Overall I think the patch looks good, but we should expand the ppc64-toc-addis-nop.s test to cover all the different sequences this will affect (like loading bytes/half-word/word signed and unsigned etc).

lld/ELF/Arch/PPC64.cpp

473

Should the mask for ds-form instructions be 0xFFE00003 instead? I think this will turn something like lwa 3,b@toc@l(4) into ld 3, b@toc@l(2)

[ruiu]

lld/ELF/Arch/PPC64.cpp

390–391

return Type == R_PPC64_TOC16_HA || Type == R_PPC64_TOC16_LO_DS ||
       Type == R_PPC64_TOC16_LO;

is better.

399

I don't think you have to call this function ahead of time. Instead, I'd replace IsTocRelType with isTocRelType(Type). I believe compiler can optimize it so that it is as fast as this code.

[sfertile]

I've picked this up from @syzaara. I'll post a revised patch shortly.

[sfertile]

• Error when optimizing an updated form load or store.
• Add an option that can disable the toc optimization.
• Expand the testing.

[ruiu]

ELF/Arch/PPC64.cpp

132 ↗

(On Diff #162427)

This variable is used only once. You can eliminate it.

560 ↗

(On Diff #162427)

Just return the result of the boolean expression instead of if (e) return true; return false;

633 ↗

(On Diff #162427)

Can't -> can't

634 ↗

(On Diff #162427)

We have llvm::utohexstr, so I believe you can omit Twine::.

ELF/Driver.cpp

287–288 ↗

(On Diff #162427)

It is indeed true that --no-toc-optimize is PPC64 only, but in other test patterns we don't care about that kind of negative flags. I'd check if Config->TocOptimize is true.

ELF/Options.td

319–320 ↗

(On Diff #162427)

Indentation.

Shouldn't TOC be all-caps?

[sfertile]

Addressed initial comments.

[ruiu]

ELF/Arch/PPC64.cpp

628 ↗

(On Diff #165566)

nit: you can omit {}.

649 ↗

(On Diff #165566)

Ditot

ELF/Driver.cpp

287–288 ↗

(On Diff #162427)

What about this?

[sfertile]

Moved checking of OPT_toc_optimise until after the machine has been decided and made it default to on only for PowerPC64.

[sfertile]

ELF/Driver.cpp

287–288 ↗

(On Diff #162427)

Forgot to update that in the diff, its fixed now.

[ruiu]

LGTM

lld/ELF/Arch/PPC64.cpp

static std::pair<RelType, uint64_t> toAddr16Rel(RelType Type, uint64_t Val) {
   case R_PPC64_DTPREL64:
     return {R_PPC64_ADDR64, DTPBiasedVal};
 
   default:
     return {Type, Val};
   }
 }
 
+static bool isTocRelType(RelType Type) {
+  if (Type == R_PPC64_TOC16_HA || Type == R_PPC64_TOC16_LO_DS ||
+      Type == R_PPC64_TOC16_LO)
+    return true;
+  return false;
+}
+
 void PPC64::relocateOne(uint8_t *Loc, RelType Type, uint64_t Val) const {
   // For a TOC-relative relocation, proceed in terms of the corresponding
   // ADDR16 relocation type.
+  bool IsTocRelType = isTocRelType(Type);
   std::tie(Type, Val) = toAddr16Rel(Type, Val);
 
   switch (Type) {
   case R_PPC64_ADDR14: {
     checkAlignment(Loc, Val, 4, Type);
     // Preserve the AA/LK bits in the branch instruction
     uint8_t AALK = Loc[3];
     write16(Loc + 2, (AALK & 3) | (Val & 0xfffc));
     break;
   }
   case R_PPC64_ADDR16:
   case R_PPC64_TPREL16:
     checkInt(Loc, Val, 16, Type);
     write16(Loc, Val);
     break;
   case R_PPC64_ADDR16_DS:
   case R_PPC64_TPREL16_DS:
     checkInt(Loc, Val, 16, Type);
     write16(Loc, (read16(Loc) & 3) | (Val & ~3));
     break;
   case R_PPC64_ADDR16_HA:
   case R_PPC64_REL16_HA:
   case R_PPC64_TPREL16_HA:
+    // When the high-adjusted part of a toc relocation evaluted to 0,
+    // we can optimize it into a nop.
+    if (IsTocRelType && ha(Val) == 0) {
+      uint32_t EndianOffset = Config->EKind == ELF64BEKind ? 2U : 0U;
+      write32(Loc - EndianOffset, 0x60000000);
+    }
+    else
       write16(Loc, ha(Val));
     break;
   case R_PPC64_ADDR16_HI:
   case R_PPC64_REL16_HI:
   case R_PPC64_TPREL16_HI:
     write16(Loc, hi(Val));
     break;
   case R_PPC64_ADDR16_HIGHER:
   case R_PPC64_TPREL16_HIGHER:
case R_PPC64_TPREL16_HIGHEST:
     break;
   case R_PPC64_ADDR16_HIGHESTA:
   case R_PPC64_TPREL16_HIGHESTA:
     write16(Loc, highesta(Val));
     break;
   case R_PPC64_ADDR16_LO:
   case R_PPC64_REL16_LO:
   case R_PPC64_TPREL16_LO:
+    // When the high-adjusted part of a toc relocation evalutes to 0, it is
+    // changed into a nop. The lo part then needs to be updated to use the toc
+    // pointer register r2, as the base register.
+    if (IsTocRelType && ha(Val) == 0) {
+      uint32_t EndianOffset = Config->EKind == ELF64BEKind ? 2U : 0U;
+      uint32_t Inst = (read32(Loc - EndianOffset) & 0xFFE00000) | 0x00020000;
+      write32(Loc - EndianOffset, Inst);
+    }
     write16(Loc, lo(Val));
     break;
   case R_PPC64_ADDR16_LO_DS:
   case R_PPC64_TPREL16_LO_DS:
+    // When the high-adjusted part of a toc relocation evalutes to 0, it is
+    // changed into a nop. The lo part then needs to be updated to use the toc
+    // pointer register r2, as the base register.
+    if (IsTocRelType && ha(Val) == 0) {
+      uint32_t EndianOffset = Config->EKind == ELF64BEKind ? 2U : 0U;
+      uint32_t Inst = (read32(Loc - EndianOffset) & 0xFFE00000) | 0x00020000;
+      write32(Loc - EndianOffset, Inst);
+    }
     write16(Loc, (read16(Loc) & 3) | (lo(Val) & ~3));
     break;
   case R_PPC64_ADDR32:
   case R_PPC64_REL32:
     checkInt(Loc, Val, 32, Type);
     write32(Loc, Val);
     break;
   case R_PPC64_ADDR64:

lld/test/ELF/ppc64-func-entry-points.s

 # point and we branch to start of foo_external_same.
 
 // CHECK: _start:
 // CHECK: 10010020:       {{.*}}     bl .+144
 // CHECK: 10010034:       {{.*}}     bl .+84
 // CHECK: foo_external_diff:
 // CHECK-NEXT: 10010080:       {{.*}}     addis 2, 12, 2
 // CHECK-NEXT: 10010084:       {{.*}}     addi 2, 2, 32640
-// CHECK-NEXT: 10010088:       {{.*}}     addis 5, 2, 0
+// CHECK-NEXT: 10010088:       {{.*}}     nop
 // CHECK: foo_external_same:
 // CHECK-NEXT: 100100b0:       {{.*}}     add 3, 4, 3

lld/test/ELF/ppc64-got-indirect.s

 # Verify that the global variable access is done through the correct
 # toc entry:
 # r2 = .TOC. = 0x10038000.
 # r3 = r2 - 32760 = 0x10030008 -> .toc entry for glob.
 
 # CHECK: _start:
 # CHECK-NEXT: 10010000:  {{.*}}   addis 2, 12, 3
 # CHECK-NEXT: 10010004:  {{.*}}   addi 2, 2, -32768
-# CHECK-NEXT: 10010008:  {{.*}}   addis 3, 2, 0
-# CHECK-NEXT: 1001000c:  {{.*}}   ld 3, -32760(3)
+# CHECK-NEXT: 10010008:  {{.*}}   nop
+# CHECK-NEXT: 1001000c:  {{.*}}   ld 3, -32760(2)
 # CHECK: 1001001c:  {{.*}}   lwa 3, 0(3)
 
 # CHECK-LE: Disassembly of section .data:
 # CHECK-LE-NEXT: glob:
 # CHECK-LE-NEXT: 10020000:       37 00 00 00
 
 # CHECK-LE: Disassembly of section .got:
 # CHECK-LE-NEXT: .got:

lld/test/ELF/ppc64-relocs.s

 
 .section .R_PPC64_TOC16_HA,"ax",@progbits
 .globl .FR_PPC64_TOC16_HA
 .FR_PPC64_TOC16_HA:
   addis 1, 2, .L1@toc@ha
 
 # CHECK: Disassembly of section .R_PPC64_TOC16_HA:
 # CHECK: .FR_PPC64_TOC16_HA:
-# CHECK: 10010018: {{.*}} addis 1, 2, 0
+# CHECK: 10010018: {{.*}} nop
 
 .section .R_PPC64_REL24,"ax",@progbits
 .globl .FR_PPC64_REL24
 .FR_PPC64_REL24:
   b .Lfoox
 .section .R_PPC64_REL24_2,"ax",@progbits
 .Lfoox:
 

 # DATABE: .rodata:
 # DATABE: 10000190: 00 00 fe b4
 
 # Address of rodata + value stored at rodata entry
 # should equal address of LBB0_2.
 # 0x10000190 + 0xfeb4 = 0x10010044
 # CHECK: Disassembly of section .R_PPC64_REL32:
 # CHECK: .FR_PPC64_REL32:
-# CHECK: 1001003c: {{.*}} addis 5, 2, 0
-# CHECK: 10010040: {{.*}} ld 5, -32736(5)
+# CHECK: 1001003c: {{.*}} nop
+# CHECK: 10010040: {{.*}} ld 5, -32736(2)
 # CHECK: 10010044: {{.*}} add 3, 3, 4
 
 .section .R_PPC64_REL64, "ax",@progbits
 .globl  .FR_PPC64_REL64
 .FR_PPC64_REL64:
         .cfi_startproc
         .cfi_personality 148, __foo
         li 0, 1