[PATCH] D90591: [ARM] Introduce t2DoLoopStartTP

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:439
     unsigned getStartOpcode() const {
-      bool IsDo = Start->getOpcode() == ARM::t2DoLoopStart;
+      bool IsDo = Start->getOpcode() != ARM::t2WhileLoopStart;
       if (!IsTailPredicationLegal())
----------------
This change highlights how many times we query the LoopStart opcode and it looks worth while to have this IsDo as a little helper!


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:625
   // TODO: We should try to insert the [W|D]LSTP after any of the other uses.
-  Register StartReg = Start->getOpcode() == ARM::t2DoLoopStart
+  Register StartReg = Start->getOpcode() != ARM::t2WhileLoopStart
                           ? Start->getOperand(1).getReg()
----------------
Call getLoopStartOperand instead?


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1444
   MachineInstr *Def = RDA->getMIOperand(
-      LoLoop.Start, LoLoop.Start->getOpcode() == ARM::t2DoLoopStart ? 1 : 0);
+      LoLoop.Start, LoLoop.Start->getOpcode() != ARM::t2WhileLoopStart ? 1 : 0);
   if (!Def) {
----------------
Can use getLoopStartOperand again.


================
Comment at: llvm/lib/Target/ARM/MVEVPTOptimisationsPass.cpp:189
+  int PredOp = findFirstVPTPredOperandIdx(*FirstVCTP);
+  if (FirstVCTP->getOperand(PredOp).getImm() != ARMVCC::None)
+    return false;
----------------
What about only checking that it's not predicated in the case where there's more than one VCTP? At least then we can handle the VPT -> VCTP case.


================
Comment at: llvm/lib/Target/ARM/MVEVPTOptimisationsPass.cpp:230
+  // in the preheader, but may need to move that earlier based on uses.
+  MachineBasicBlock *MBB = LoopStart->getParent();
+  MachineBasicBlock::iterator InsertPt = MBB->getFirstTerminator();
----------------
I don't follow what's happening here, what uses could there be which we need to schedule for?


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

https://reviews.llvm.org/D90591