[IfConversion] Disallow TBB == FBB for valid triangles

[uabelho]

Summary

Previously the case

EBB
| \_
|  |
| TBB
|  /
FBB

was treated as a valid triangle also when TBB and FBB was the same basic
block. This could then lead to an invalid CFG when we removed the edge
from EBB to TBB, since that meant we would also remove the edge from EBB
to FBB.

Since TBB == FBB is quite a degenerated case of a triangle, we now
don't treat it as a valid triangle anymore, and thus we will avoid the
trouble with updating the CFG.

[efriedma]

Would it make more sense to just say this isn't a triangle? We don't really want to do loop peeling in IfConversion.

[uabelho]

Disallow this case in ValidTriangle instead.

[uabelho]

In D67832#1681258, @efriedma wrote:

Would it make more sense to just say this isn't a triangle? We don't really want to do loop peeling in IfConversion.

Sure! Updated the patch to do that instead.

[efriedma]

LGTM

[uabelho]

In D67832#1683474, @efriedma wrote:

LGTM

Thanks!

llvm/lib/CodeGen/IfConversion.cpp

if (CvtMBB.pred_size() > 1) {
     // Predicate the 'true' block after removing its branch.
     CvtBBI->NonPredSize -= TII->removeBranch(CvtMBB);
     PredicateBlock(*CvtBBI, CvtMBB.end(), Cond);
 
     // Now merge the entry of the triangle with the true block.
     MergeBlocks(BBI, *CvtBBI, false);
   }
 
-  // Keep the CFG updated.
+  // Keep the CFG updated. If CvtMBB and NextMBB happen to be the same we skip
+  // removing CvtMBB as successor since that would also remove NextMBB (which we
+  // want to keep).
+  if (&CvtMBB != &NextMBB)
     BBI.BB->removeSuccessor(&CvtMBB, true);
 
   // If 'true' block has a 'false' successor, add an exit branch to it.
   if (HasEarlyExit) {
     SmallVector<MachineOperand, 4> RevCond(CvtBBI->BrCond.begin(),
                                            CvtBBI->BrCond.end());
     if (TII->reverseBranchCondition(RevCond))
       llvm_unreachable("Unable to reverse branch condition!");
 

llvm/test/CodeGen/ARM/ifcvt_triangleSameCvtNext.mir

+# RUN: llc -mtriple=arm-apple-ios -run-pass=if-converter -verify-machineinstrs %s -o - | FileCheck %s
+...
+---
+name:            foo
+body:             |
+  bb.0:
+    Bcc %bb.2, 1, $cpsr
+
+  bb.1:
+    $sp = tADDspi $sp, 2, 14, _
+    B %bb.1
+
+  bb.2:
+    Bcc %bb.3, 0, $cpsr
+    B %bb.2
+
+  bb.3:
+    Bcc %bb.1, 1, $cpsr
+    B %bb.1
+...
+
+# Both branches in bb.3 jump to bb.1. IfConversion treats it as a triangle with
+# CvtBB and NextBB both being bb.1. We must make sure that bb.3 at the end still
+# has bb.1 as a successor or the CFG will be broken and verifiers will complain
+# that the block ends with a branch but the successor list is empty.
+
+# CHECK:     bb.3:
+# CHECK:       successors: %bb.1
+# CHECK:       $sp = tADDspi $sp, 2, 1, $cpsr
+# CHECK:       B %bb.1