[llvm] 02d4b36 - [GlobalISel] Check if branches use the same MBB in matchOptBrCondByInvertingCond

[Jessica Paquette via llvm-commits]

Author: Jessica Paquette
Date: 2021-02-02T15:38:48-08:00
New Revision: 02d4b365bf4f8c2cb56e5612902f6c3bb4316493

URL: https://github.com/llvm/llvm-project/commit/02d4b365bf4f8c2cb56e5612902f6c3bb4316493
DIFF: https://github.com/llvm/llvm-project/commit/02d4b365bf4f8c2cb56e5612902f6c3bb4316493.diff

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

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

Differential Revision: https://reviews.llvm.org/D95895

Added: 
    

Modified: 
    llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
    llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-br.mir

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index df0219fcfa647..a9353bdfb780e 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -968,10 +968,11 @@ bool CombinerHelper::matchOptBrCondByInvertingCond(MachineInstr &MI) {
   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()))
-    return false;
-  return true;
+  // 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();
+  return BrCondTarget != MI.getOperand(0).getMBB() &&
+         MBB->isLayoutSuccessor(BrCondTarget);
 }
 
 void CombinerHelper::applyOptBrCondByInvertingCond(MachineInstr &MI) {

diff  --git a/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-br.mir b/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-br.mir
index 0631ff89ade0d..0647de44c4b81 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-br.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-br.mir
@@ -29,6 +29,7 @@
     ret i32 %retval.0
   }
 
+  define void @dont_combine_same_block() { ret void }
 
 ...
 ---
@@ -87,3 +88,26 @@ body:             |
     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
+...