[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/trunk/lib/Target/ARM/ARMBaseInstrInfo.h

 bool rewriteARMFrameIndex(MachineInstr &MI, unsigned FrameRegIdx,
                           unsigned FrameReg, int &Offset,
                           const ARMBaseInstrInfo &TII);
 
 bool rewriteT2FrameIndex(MachineInstr &MI, unsigned FrameRegIdx,
                          unsigned FrameReg, int &Offset,
                          const ARMBaseInstrInfo &TII);
 
+/// Return true if Reg is defd between From and To
+bool registerDefinedBetween(unsigned Reg, MachineBasicBlock::iterator From,
+                            MachineBasicBlock::iterator To,
+                            const TargetRegisterInfo *TRI);
+
+/// Search backwards from a tBcc to find a tCMPi8 against 0, meaning
+/// we can convert them to a tCBZ or tCBNZ. Return nullptr if not found.
+MachineInstr *findCMPToFoldIntoCBZ(MachineInstr *Br,
+                                   const TargetRegisterInfo *TRI);
+
 } // end namespace llvm
 
 #endif // LLVM_LIB_TARGET_ARM_ARMBASEINSTRINFO_H

llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.cpp

isProfitableToIfCvt(MachineBasicBlock &MBB,
   // 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.
   if (MBB.getParent()->getFunction().hasOptSize()) {
     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()) {
-          --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;
-          }
-        }
-      }
-    }
-  }
+        const TargetRegisterInfo *TRI = &getRegisterInfo();
+        MachineInstr *CmpMI = findCMPToFoldIntoCBZ(LastMI, TRI);
+        if (CmpMI)
   return isProfitableToIfCvt(MBB, NumCycles, ExtraPredCycles,
                              MBB, 0, 0, Probability);
 }
 
 bool ARMBaseInstrInfo::
 isProfitableToIfCvt(MachineBasicBlock &TBB,
                     unsigned TCycles, unsigned TExtra,
                     MachineBasicBlock &FBB,
static const std::pair<unsigned, const char *> TargetFlags[] = {
       {MO_COFFSTUB, "arm-coffstub"},
       {MO_GOT, "arm-got"},
       {MO_SBREL, "arm-sbrel"},
       {MO_DLLIMPORT, "arm-dllimport"},
       {MO_SECREL, "arm-secrel"},
       {MO_NONLAZY, "arm-nonlazy"}};
   return makeArrayRef(TargetFlags);
 }
+
+bool llvm::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;
+}
+
+MachineInstr *llvm::findCMPToFoldIntoCBZ(MachineInstr *Br,
+                                         const TargetRegisterInfo *TRI) {
+  // Search backwards to the instruction that defines CSPR. This may or not
+  // be a CMP, we check that after this loop. If we find another instruction
+  // that reads cpsr, we return nullptr.
+  MachineBasicBlock::iterator CmpMI = Br;
+  while (CmpMI != Br->getParent()->begin()) {
+    --CmpMI;
+    if (CmpMI->modifiesRegister(ARM::CPSR, TRI))
+      break;
+    if (CmpMI->readsRegister(ARM::CPSR, TRI))
+      break;
+  }
+
+  // Check that this inst is a CMP r[0-7], #0 and that the register
+  // is not redefined between the cmp and the br.
+  if (CmpMI->getOpcode() != ARM::tCMPi8 && CmpMI->getOpcode() != ARM::t2CMPri)
+    return nullptr;
+  unsigned Reg = CmpMI->getOperand(0).getReg();
+  unsigned PredReg = 0;
+  ARMCC::CondCodes Pred = getInstrPredicate(*CmpMI, PredReg);
+  if (Pred != ARMCC::AL || CmpMI->getOperand(1).getImm() != 0)
+    return nullptr;
+  if (!isARMLowRegister(Reg))
+    return nullptr;
+  if (registerDefinedBetween(Reg, CmpMI->getNextNode(), Br, TRI))
+    return nullptr;
+
+  return &*CmpMI;
+}

llvm/trunk/lib/Target/ARM/ARMConstantIslandPass.cpp

if (isCPEntryInRange(U.MI, UserOffset, U.CPEMI, MaxOffs, false, true)) {
       ++NumT2CPShrunk;
       MadeChange = true;
     }
   }
 
   return MadeChange;
 }
 
-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 ARMConstantIslands::optimizeThumb2Branches() {
   bool MadeChange = false;
 
   // The order in which branches appear in ImmBranches is approximately their
   // order within the function body. By visiting later branches first, we reduce
   // the distance between earlier forward branches and their targets, making it
   // more likely that the cbn?z optimization, which can only apply to forward
   // branches, will succeed.
for (unsigned i = ImmBranches.size(); i != 0; --i) {
     MachineBasicBlock *DestBB = Br.MI->getOperand(0).getMBB();
     // Check if the distance is within 126. Subtract starting offset by 2
     // because the cmp will be eliminated.
     unsigned BrOffset = getOffsetOf(Br.MI) + 4 - 2;
     unsigned DestOffset = BBInfo[DestBB->getNumber()].Offset;
     if (BrOffset >= DestOffset || (DestOffset - BrOffset) > 126)
       continue;
 
-    // Search backwards to the instruction that defines CSPR. This may or not
-    // be a CMP, we check that after this loop. If we find an instruction that
-    // reads cpsr, we need to keep the original cmp.
+    // Search backwards to find a tCMPi8
     auto *TRI = STI->getRegisterInfo();
-    MachineBasicBlock::iterator CmpMI = Br.MI;
-    while (CmpMI != Br.MI->getParent()->begin()) {
-      --CmpMI;
-      if (CmpMI->modifiesRegister(ARM::CPSR, TRI))
-        break;
-      if (CmpMI->readsRegister(ARM::CPSR, TRI))
-        break;
-    }
-
-    // Check that this inst is a CMP r[0-7], #0 and that the register
-    // is not redefined between the cmp and the br.
-    if (CmpMI->getOpcode() != ARM::tCMPi8)
+    MachineInstr *CmpMI = findCMPToFoldIntoCBZ(Br.MI, TRI);
+    if (!CmpMI || CmpMI->getOpcode() != ARM::tCMPi8)
       continue;
+
     unsigned Reg = CmpMI->getOperand(0).getReg();
-    Pred = getInstrPredicate(*CmpMI, PredReg);
-    if (Pred != ARMCC::AL || CmpMI->getOperand(1).getImm() != 0)
-      continue;
-    if (registerDefinedBetween(Reg, CmpMI->getNextNode(), Br.MI, TRI))
-      continue;
 
     // Check for Kill flags on Reg. If they are present remove them and set kill
     // on the new CBZ.
     MachineBasicBlock::iterator KillMI = Br.MI;
     bool RegKilled = false;
     do {
       --KillMI;
       if (KillMI->killsRegister(Reg, TRI)) {

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

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=thumbv7m-none-eabi -run-pass=if-converter -o - %s | FileCheck %s
+
+--- |
+  define i32 @f1(i32 %x) #0 { ret i32 %x }
+  define i32 @f2(i32 %x) #0 { ret i32 %x }
+  define i32 @f3(i32 %x) #0 { ret i32 %x }
+  declare i32 @fn(i32 %x) #0
+
+  attributes #0 = { minsize nounwind optsize }
+
+...
+---
+name:            f1
+tracksRegLiveness: true
+liveins:
+  - { reg: '$r0', virtual-reg: '' }
+stack:
+  - { 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: f1
+  ; CHECK: bb.0:
+  ; CHECK:   liveins: $r0, $lr, $r7
+  ; CHECK:   t2CMPri killed renamable $r0, 1, 14, $noreg, implicit-def $cpsr
+  ; CHECK:   tBX_RET 1, killed $cpsr
+  ; 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
+  bb.0:
+    successors: %bb.1(0x40000000), %bb.2(0x40000000)
+    liveins: $r0, $lr, $r7
+
+    t2CMPri killed renamable $r0, 1, 14, $noreg, implicit-def $cpsr
+    t2Bcc %bb.2, 1, killed $cpsr
+
+  bb.1:
+    liveins: $r7, $lr
+
+    $sp = frame-setup t2STMDB_UPD $sp, 14, $noreg, killed $r7, killed $lr
+    frame-setup CFI_INSTRUCTION def_cfa_offset 8
+    frame-setup CFI_INSTRUCTION offset $lr, -4
+    frame-setup CFI_INSTRUCTION offset $r7, -8
+    $r0 = t2MOVi 0, 14, $noreg, $noreg
+    tBL 14, $noreg, @fn, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit $r0, implicit-def $sp, implicit-def dead $r0
+    $sp = t2LDMIA_UPD $sp, 14, $noreg, def $r7, def $lr
+    tBX_RET 14, $noreg
+
+  bb.2:
+    liveins: $lr, $r7
+
+    tBX_RET 14, $noreg
+
+...
+---
+name:            f2
+tracksRegLiveness: true
+liveins:
+  - { reg: '$r0', virtual-reg: '' }
+stack:
+  - { 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: f2
+  ; 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:   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
+    t2Bcc %bb.2, 1, killed $cpsr
+
+  bb.1:
+    liveins: $r7, $lr
+
+    $sp = frame-setup t2STMDB_UPD $sp, 14, $noreg, killed $r7, killed $lr
+    frame-setup CFI_INSTRUCTION def_cfa_offset 8
+    frame-setup CFI_INSTRUCTION offset $lr, -4
+    frame-setup CFI_INSTRUCTION offset $r7, -8
+    $r0 = t2MOVi 0, 14, $noreg, $noreg
+    tBL 14, $noreg, @fn, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit $r0, implicit-def $sp, implicit-def dead $r0
+    $sp = t2LDMIA_UPD $sp, 14, $noreg, def $r7, def $lr
+    tBX_RET 14, $noreg
+
+  bb.2:
+    liveins: $lr, $r7
+
+    tBX_RET 14, $noreg
+
+...
+---
+name:            f3
+tracksRegLiveness: true
+liveins:
+  - { reg: '$r0', virtual-reg: '' }
+stack:
+  - { 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:   $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
+
+  bb.1:
+    liveins: $r7, $lr
+
+    $sp = frame-setup t2STMDB_UPD $sp, 14, $noreg, killed $r7, killed $lr
+    frame-setup CFI_INSTRUCTION def_cfa_offset 8
+    frame-setup CFI_INSTRUCTION offset $lr, -4
+    frame-setup CFI_INSTRUCTION offset $r7, -8
+    $r0 = t2MOVi 0, 14, $noreg, $noreg
+    tBL 14, $noreg, @fn, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit $r0, implicit-def $sp, implicit-def dead $r0
+    $sp = t2LDMIA_UPD $sp, 14, $noreg, def $r7, def $lr
+    tBX_RET 14, $noreg
+
+  bb.2:
+    liveins: $lr, $r7
+
+    tBX_RET 14, $noreg
+
+...