[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/trunk/lib/CodeGen/IfConversion.cpp

 /// predecessor) forms a valid triangle shape for ifcvt. If 'FalseBranch' is
 /// true, it checks if 'true' block's false branch branches to the 'false' block
 /// rather than the other way around. It also returns the number of instructions
 /// that the ifcvt would need to duplicate if performed in 'Dups'.
 bool IfConverter::ValidTriangle(BBInfo &TrueBBI, BBInfo &FalseBBI,
                                 bool FalseBranch, unsigned &Dups,
                                 BranchProbability Prediction) const {
   Dups = 0;
+  if (TrueBBI.BB == FalseBBI.BB)
+    return false;
+
   if (TrueBBI.IsBeingAnalyzed || TrueBBI.IsDone)
     return false;
 
   if (TrueBBI.BB->pred_size() > 1) {
     if (TrueBBI.CannotBeCopied)
       return false;
 
     unsigned Size = TrueBBI.NonPredSize;

llvm/trunk/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 shouldn't treat this as a
+# tringle and insert the tADDspi in bb3, but leave it as it is.
+
+# CHECK:     bb.3:
+# CHECK:       successors: %bb.1
+# CHECK-NOT:   tADDspi
+# CHECK:       Bcc %bb.1, 1, $cpsr
+# CHECK:       B %bb.1