[ARM] Update check for CBZ in Ifcvt

[dmgreen]

Summary

This check in ifcvt mirrors the same one in constant island pass, for when a CBZ will be preferable to a IT block. The version in constant islands was updated recently to search backwards for CMP's, so this version should be updated too. (The code should probably be shared between the two, but I'm not sure exactly where that should go. I'm happy to move it.)

I was having trouble getting this to happen in a llc test without registry spilling or a schedule. MIR tests show the differences though. Test is new, I'm just showing the differences here for clarity.

[efriedma]

Yes, please refactor the code. We define a bunch of helpers in ARMBaseInstrInfo.h; that's probably fine.

Looks fine otherwise.

[dmgreen]

Refactored

[dmgreen]

ping

[jmolloy]

Seems reasonable to me.

[dmgreen]

Thanks James!

llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp

bool ARMBaseInstrInfo::isSchedulingBoundary(const MachineInstr &MI,
   // No ARM calling conventions change the stack pointer. (X86 calling
   // conventions sometimes do).
   if (!MI.isCall() && MI.definesRegister(ARM::SP))
     return true;
 
   return false;
 }
 
+static bool registerDefinedBetween(unsigned Reg,
+                                   MachineBasicBlock::iterator From,
+                                   MachineBasicBlock::iterator To,
+                                   const TargetRegisterInfo *TRI) {
+  for (auto I = From; I != To; ++I)
+    if (I->modifiesRegister(Reg, TRI))
+      return true;
+  return false;
+}
+
 bool ARMBaseInstrInfo::
 isProfitableToIfCvt(MachineBasicBlock &MBB,
                     unsigned NumCycles, unsigned ExtraPredCycles,
                     BranchProbability Probability) const {
   if (!NumCycles)
     return false;
 
   // If we are optimizing for size, see if the branch in the predecessor can be
   // lowered to cbn?z by the constant island lowering pass, and return false if
-  // so. This results in a shorter instruction sequence.
+  // so. This results in a shorter instruction sequence. This check mirrors that
+  // in constant island pass.
   if (MBB.getParent()->getFunction().optForSize()) {
     MachineBasicBlock *Pred = *MBB.pred_begin();
     if (!Pred->empty()) {
       MachineInstr *LastMI = &*Pred->rbegin();
       if (LastMI->getOpcode() == ARM::t2Bcc) {
         MachineBasicBlock::iterator CmpMI = LastMI;
-        if (CmpMI != Pred->begin()) {
+        const TargetRegisterInfo *TRI = &getRegisterInfo();
+        while (CmpMI != Pred->begin()) {
           --CmpMI;
-          if (CmpMI->getOpcode() == ARM::tCMPi8 ||
-              CmpMI->getOpcode() == ARM::t2CMPri) {
-            unsigned Reg = CmpMI->getOperand(0).getReg();
-            unsigned PredReg = 0;
-            ARMCC::CondCodes P = getInstrPredicate(*CmpMI, PredReg);
-            if (P == ARMCC::AL && CmpMI->getOperand(1).getImm() == 0 &&
-                isARMLowRegister(Reg))
-              return false;
+          if (CmpMI->modifiesRegister(ARM::CPSR, TRI))
+            break;
+          if (CmpMI->readsRegister(ARM::CPSR, TRI))
+            break;
+        }
+
+              isARMLowRegister(Reg) &&
+              !registerDefinedBetween(Reg, CmpMI->getNextNode(), LastMI, TRI)) {
           }
         }
       }
     }
   }
   return isProfitableToIfCvt(MBB, NumCycles, ExtraPredCycles,
                              MBB, 0, 0, Probability);
 }

llvm/test/CodeGen/Thumb2/ifcvt-cbz.mir

- { id: 0, name: '', type: spill-slot, offset: -4, size: 4, alignment: 4,
       stack-id: 0, callee-saved-register: '$lr', callee-saved-restored: true,
       debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
   - { id: 1, name: '', type: spill-slot, offset: -8, size: 4, alignment: 4,
       stack-id: 0, callee-saved-register: '$r7', callee-saved-restored: true,
       debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
 body:             |
   ; CHECK-LABEL: name: f3
   ; CHECK: bb.0:
+  ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
   ; CHECK:   liveins: $r0, $lr, $r7
   ; CHECK:   t2CMPri killed renamable $r0, 0, 14, $noreg, implicit-def $cpsr
-  ; CHECK:   dead $r1 = t2MOVi 0, 14, $noreg, $noreg
-  ; CHECK:   tBX_RET 1, killed $cpsr
+  ; CHECK:   $r1 = t2MOVi 0, 14, $noreg, $noreg
+  ; CHECK:   t2Bcc %bb.2, 1, killed $cpsr
+  ; CHECK: bb.1:
+  ; CHECK:   liveins: $r7, $lr
   ; CHECK:   $sp = frame-setup t2STMDB_UPD $sp, 14, $noreg, killed $r7, killed $lr
   ; CHECK:   frame-setup CFI_INSTRUCTION def_cfa_offset 8
   ; CHECK:   frame-setup CFI_INSTRUCTION offset $lr, -4
   ; CHECK:   frame-setup CFI_INSTRUCTION offset $r7, -8
   ; CHECK:   $r0 = t2MOVi 0, 14, $noreg, $noreg
   ; CHECK:   tBL 14, $noreg, @fn, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit $r0, implicit-def $sp, implicit-def dead $r0
   ; CHECK:   $sp = t2LDMIA_UPD $sp, 14, $noreg, def $r7, def $lr
   ; CHECK:   tBX_RET 14, $noreg
+  ; CHECK: bb.2:
+  ; CHECK:   liveins: $lr, $r7
+  ; CHECK:   tBX_RET 14, $noreg
   bb.0:
     successors: %bb.1(0x40000000), %bb.2(0x40000000)
     liveins: $r0, $lr, $r7
 
     t2CMPri killed renamable $r0, 0, 14, $noreg, implicit-def $cpsr
     $r1 = t2MOVi 0, 14, $noreg, $noreg
     t2Bcc %bb.2, 1, killed $cpsr