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

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 28 20:39:06 PDT 2019

hfinkel added inline comments.

Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:3936
+MachineInstr *PPCInstrInfo::findLoopInstr(
+    const MachineBasicBlock &BB, unsigned EndLoopOp,
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?

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;
We generally don't directly recurse though the CFG, but use a worklist and a loop. Can you do that here?

Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:4001
+  // The loop trip count is a run-time value.
+  // We need  to subtract one from the trip count,
+  // and insert branch later to check if we're done with the loop.
Extra space before 'to'.

Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:4005
+  // Since BDZ/BDZ8 that we will insert will also decrease the ctr by 1,
+  // so we don't need to generate any thing here
+  Cond.push_back(MachineOperand::CreateImm(0));
Period at the end of the comment.

Comment at: llvm/lib/Target/PowerPC/PPCSubtarget.cpp:192
+bool PPCSubtarget::enableMachinePipeliner() const {
+  return (DarwinDirective == PPC::DIR_PWR9) && EnableMachinePipeliner;
Given that this is off by default anyway, why is it enabled only for the `P9`?




More information about the llvm-commits mailing list