[GlobalISel] Check if branches use the same MBB in matchOptBrCondByInvertingCond

[paquette]

Summary

If the G_BR + G_BRCOND in this combine use the same MBB, then it will infinite loop. Don't allow that to happen.

[arsenm]

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

975

I would check this first (also reformat as return X || Y)

[paquette]

Simplify logic

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

bool CombinerHelper::matchOptBrCondByInvertingCond(MachineInstr &MI) {
   if (BrIt == MBB->begin())
     return false;
   assert(std::next(BrIt) == MBB->end() && "expected G_BR to be a terminator");
 
   MachineInstr *BrCond = &*std::prev(BrIt);
   if (BrCond->getOpcode() != TargetOpcode::G_BRCOND)
     return false;
 
-  // Check that the next block is the conditional branch target.
-  if (!MBB->isLayoutSuccessor(BrCond->getOperand(1).getMBB()))
+  // Check that the next block is the conditional branch target. Also make sure
+  // that it isn't the same as the G_BR's target (otherwise, this will loop.)
+  MachineBasicBlock *BrCondTarget = BrCond->getOperand(1).getMBB();
+  if (!MBB->isLayoutSuccessor(BrCondTarget) ||
+      BrCondTarget == MI.getOperand(0).getMBB())
     return false;
   return true;
 }
 
 void CombinerHelper::applyOptBrCondByInvertingCond(MachineInstr &MI) {
   MachineBasicBlock *BrTarget = MI.getOperand(0).getMBB();
   MachineBasicBlock::iterator BrIt(MI);
   MachineInstr *BrCond = &*std::prev(BrIt);

llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-br.mir

if.end:
     %add2 = add nuw nsw i32 %mul, 2
     br label %return
 
   return:
     %retval.0 = phi i32 [ %add1, %if.then ], [ %add2, %if.end ]
     ret i32 %retval.0
   }
 
+  define void @dont_combine_same_block() { ret void }
 
 ...
 ---
 name:            foo
 tracksRegLiveness: true
 body:             |
   ; CHECK-LABEL: name: foo
   ; CHECK: bb.0.entry:
bb.3.if.end:
     %6:_(s32) = nuw nsw G_ADD %4, %5
 
   bb.4.return:
     %10:_(s32) = G_PHI %8(s32), %bb.2, %6(s32), %bb.3
     $w0 = COPY %10(s32)
     RET_ReallyLR implicit $w0
 
 ...
+---
+name:            dont_combine_same_block
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: dont_combine_same_block
+  ; CHECK: bb.0:
+  ; CHECK:   successors: %bb.1(0x80000000)
+  ; CHECK:   liveins: $w0, $w1
+  ; CHECK:   %cond:_(s1) = G_IMPLICIT_DEF
+  ; CHECK:   G_BRCOND %cond(s1), %bb.1
+  ; CHECK:   G_BR %bb.1
+  ; CHECK: bb.1:
+  ; CHECK:   RET_ReallyLR
+  bb.0:
+    liveins: $w0, $w1
+    %cond:_(s1) = G_IMPLICIT_DEF
+
+    ; The G_BRCOND and G_BR have the same target here. Don't change anything.
+    G_BRCOND %cond(s1), %bb.1
+    G_BR %bb.1
+  bb.1:
+    RET_ReallyLR
+...