[PATCH] D67832: [IfConversion] Handle when CvtMBB==NextMBB in IfConvertTriangle

[Mikael Holmén via Phabricator via llvm-commits]

uabelho created this revision.
uabelho added reviewers: efriedma, dmgreen, kparzysz.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

When we remove CvtMBB as a successor to our starting BB (due to merging
CvtMBB into BB), we must be careful if CvtMBB is the same block as NextMBB,
the block we will still branch to from BB. In that case we want to keep
NextMBB (== CvtMBB) as a successor, so avoid the CFG update.


Repository:
  rL LLVM

https://reviews.llvm.org/D67832

Files:
  llvm/lib/CodeGen/IfConversion.cpp
  llvm/test/CodeGen/ARM/ifcvt_triangleSameCvtNext.mir


Index: llvm/test/CodeGen/ARM/ifcvt_triangleSameCvtNext.mir
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/ARM/ifcvt_triangleSameCvtNext.mir
@@ -0,0 +1,30 @@
+# 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
Index: llvm/lib/CodeGen/IfConversion.cpp
===================================================================
--- llvm/lib/CodeGen/IfConversion.cpp
+++ llvm/lib/CodeGen/IfConversion.cpp
@@ -1584,8 +1584,11 @@
     MergeBlocks(BBI, *CvtBBI, false);
   }
 
-  // Keep the CFG updated.
-  BBI.BB->removeSuccessor(&CvtMBB, true);
+  // 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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67832.221017.patch
Type: text/x-patch
Size: 1686 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190920/50894c87/attachment.bin>