[PATCH] D14080: [PowerPC] Add an MI SSA peephole pass.

hfinkel@anl.gov via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 27 12:01:22 PDT 2015


hfinkel added inline comments.

================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:71
@@ +70,3 @@
+  MRI = &MF->getRegInfo();
+  TII = static_cast<const PPCInstrInfo*>(MF->getSubtarget().getInstrInfo());
+  DEBUG(dbgs() << "*** PowerPC MI peephole pass ***\n\n");
----------------
You should use:

   TII = MF.getSubtarget<PPCSubtarget>().getInstrInfo();

just like we have in PPCTLSDynamicCall.cpp (looks like you should fix this in PPCVSXSwapRemoval.cpp too).


================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:96
@@ +95,3 @@
+      // Per-opcode peepholes.
+      switch(MI.getOpcode()) {
+
----------------
Add space before (.

================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:133
@@ +132,3 @@
+                DEBUG(dbgs()
+                      << "Optimizing splat/swap or splat/splat to copy: ");
+                DEBUG(MI.dump());
----------------
This seems somewhat misleading. I think it should say:

  "Optimizing splat/swap or splat/splat to splat/copy:

because you're not actually eliminating the splat.


================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:177
@@ +176,3 @@
+    if (ToErase) {
+      ToErase->eraseFromParent();
+      ToErase = nullptr;
----------------
You need to make sure that the MI you want to erase does not have other users before you erase it.

================
Comment at: lib/Target/PowerPC/PPCMIPeephole.cpp:208
@@ +207,3 @@
+
+  return lookThruCopyLike(CopySrcReg);
+}
----------------
We should not use unbounded recursion without a good reason, and making this into a loop seems straightforward. Please make this loop instead.




Repository:
  rL LLVM

http://reviews.llvm.org/D14080





More information about the llvm-commits mailing list