[PATCH] D79864: [PowerPC] Add linker opt for PC Relative GOT indirect accesses

[Nemanja Ivanovic via Phabricator via llvm-commits]

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

The remaining comments are minor nits that can be addressed on commit. LGTM.



================
Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:315
+
+        if (Pair->StillValid) {
+          // Just to be safe.
----------------
I think you can just flip this to
```
if (!Pair->StillValid)
  continue;
```
and reduce the nesting in the following code.


================
Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:317
+          // Just to be safe.
+          // Add implicit def to the PLD and implicit use to the second load.
+          MachineOperand ImplDef =
----------------
Elaborate slightly. Implicit def of what and implicit use of what and why.


================
Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:329
+              Context.createTempSymbol(Twine("pcrel"), false, false);
+          MachineOperand LabelNum =
+              MachineOperand::CreateMCSymbol(Symbol, PPCII::MO_PCREL_OPT_FLAG);
----------------
I think the name `LabelNum` is a bit awkward. Maybe it should be something like `PCRelLabel` or `PCRelNumberedLabel`?


================
Comment at: llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll:45
+; CHECK-NEXT:    lbz r3, 0(r3)
+; CHECK-NEXT:    stb r3, 0(r4)
+; CHECK-NEXT:    blr
----------------
Why is there no relocation that refers to the store? Similarly below.


================
Comment at: llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll:105
+; CHECK-NEXT:    pld r3, input128 at got@pcrel(0), 1
+; CHECK-NEXT:    lxvx vs0, 0, r3
+; CHECK-NEXT:    pld r3, output128 at got@pcrel(0), 1
----------------
Please add a FIXME here for us to follow up on this. We should favour the D-Forms in cases such as this so that they can be optimized.


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

https://reviews.llvm.org/D79864