[PATCH] D79864: [PowerPC] Add new linker optimization for PowerPC

[Anil Mahmud via Phabricator via llvm-commits]

anil9 added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp:228
+
+  // Obtian the label symbol and if the instruction is PLDpc return immediately.
+  // The PLDpc is the first of the two instrctions that are linked together by
----------------
nit : Obtian -> Obtaini


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp:246
+  assert(DF && "Expecting a valid data fragment.");
+  DF->getFixups().push_back(MCFixup::create(LabelSym->getOffset()-8,
+                            SubExpr2,
----------------
Can this -8 be handled in some other way, like defined above as something. Looks a bit like a magic number.


================
Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:50
+    switch(Use.getOpcode()) {
+    default:
+      return false;
----------------
Indent ?


================
Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:223
+        MachineInstr &DefMI = *BBI;
+        const MachineOperand &LoadedAddressReg = BBI->getOperand(0);
+        const MachineOperand &SymbolOp = BBI->getOperand(1);
----------------
nit: 

We can get one operand, check condition and exit, instead of getting both of them and then checkiing one by one.


================
Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:238
+        for (auto SearchBBI = ++BBI; SearchBBI != MBB.instr_end();
+             ++SearchBBI) {
+          if (SearchBBI->readsRegister(Reg, TRI) &&
----------------
This did not fit in one line ?


================
Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:257
+          // We have found a potential use. Search through the instruction
+          // between the def and the use to see if this valid to mark as a
+          // linker opt.
----------------
" if this is valid to mark it as a "


================
Comment at: llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll:6
+
+; On future CPU with PC Relative addressing enabled it is possible for the
+; linker to optimize GOT indirect accesses. In order for the linker to do this
----------------
enabled, it is 


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

https://reviews.llvm.org/D79864