[PATCH] D62164: [PowerPC] Enable MachinePipeliner for P9 with -ppc-enable-pipeliner

[Jinsong Ji via Phabricator via llvm-commits]

jsji added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3936
+
+MachineInstr *PPCInstrInfo::findLoopInstr(
+    const MachineBasicBlock &BB, unsigned EndLoopOp,
----------------
hfinkel wrote:
> FWIW, I find this method a bit strange. Do we need have a MachineLoopInfo available such that we could just check in the preheader of the loop?
In short, yes. 
MachinePipeliner have MachineLoopInfo, and it also make sure that we call `reduceLoopCount` with prolog MBBs that set up the pipeline, so we are safe to assume that `the loop set-up instruction will be in a predecessor block` only.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3960
+    // Check the predecessors for the LOOP instruction.
+    if (MachineInstr *Loop = findLoopInstr(*PB, EndLoopOp, TargetBB, Visited))
+      return Loop;
----------------
hfinkel wrote:
> We generally don't directly recurse though the CFG, but use a worklist and a loop. Can you do that here?
Sure.


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

https://reviews.llvm.org/D62164