[PATCH] D67203: [IfConversion] Fix diamond conversion with unanalyzable branches.

[Eli Friedman via Phabricator via llvm-commits]

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

The code was incorrectly counting the number of identical instructions, and therefore tried to predicate an instruction which should not have been predicated.  This could have various effects: a compiler crash, an assembler failure, a miscompile, or just generating an extra, unnecessary instruction.

Instead of depending on TargetInstrInfo::removeBranch, which only works on analyzable branches, just remove all branch instructions.

Fixes https://bugs.llvm.org/show_bug.cgi?id=43121 and https://bugs.llvm.org/show_bug.cgi?id=41121 .


Repository:
  rL LLVM

https://reviews.llvm.org/D67203

Files:
  lib/CodeGen/IfConversion.cpp
  test/CodeGen/ARM/ifcvt-diamond-unanalyzable-common.mir


Index: test/CodeGen/ARM/ifcvt-diamond-unanalyzable-common.mir
===================================================================
--- /dev/null
+++ test/CodeGen/ARM/ifcvt-diamond-unanalyzable-common.mir
@@ -0,0 +1,58 @@
+# RUN: llc %s -o - -run-pass=if-converter | FileCheck %s
+# Make sure we correctly if-convert blocks containing an INLINEASM_BR.
+# CHECK: t2CMPri killed renamable $r2, 34
+# CHECK-NEXT: $r0 = t2MOVi 2, 1, $cpsr, $noreg
+# CHECK-NEXT: $r0 = t2MOVi 3, 0, killed $cpsr, $noreg, implicit killed $r0
+# CHECK-NEXT: tBL 14, $noreg, @fn2
+# CHECK-NEXT: INLINEASM_BR &"", 9, 13, 0, 13, blockaddress(@fn1, %ir-block.l_yes)
+# CHECK-NEXT: t2B %bb.1, 14, $noreg
+--- |
+  target triple = "thumbv7-unknown-linux-gnueabi"
+  
+  define dso_local void @fn1() {
+  l_yes:
+    ret void
+  }
+  
+  declare dso_local i32 @fn2(...)
+...
+---
+name:            fn1
+alignment:       1
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    successors: %bb.1(0x40000000), %bb.2(0x40000000)
+    liveins: $r0, $r1, $r2, $r4, $lr
+  
+    $sp = frame-setup t2STMDB_UPD $sp, 14, $noreg, killed $r4, killed $lr
+    t2CMPri killed renamable $r2, 34, 14, $noreg, implicit-def $cpsr
+    t2Bcc %bb.2, 1, killed $cpsr
+  
+  bb.1:
+    successors: %bb.3(0x40000000), %bb.4(0x40000000)
+    liveins: $r1
+  
+    $r0 = t2MOVi 3, 14, $noreg, $noreg
+    tBL 14, $noreg, @fn2, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit $r0, implicit $r1, implicit-def $sp, implicit-def dead $r0
+    INLINEASM_BR &"", 9, 13, 0, 13, blockaddress(@fn1, %ir-block.l_yes)
+    t2B %bb.3, 14, $noreg
+  
+  bb.2:
+    successors: %bb.3(0x40000000), %bb.4(0x40000000)
+    liveins: $r1
+  
+    $r0 = t2MOVi 2, 14, $noreg, $noreg
+    tBL 14, $noreg, @fn2, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit $r0, implicit $r1, implicit-def $sp, implicit-def dead $r0
+    INLINEASM_BR &"", 9, 13, 0, 13, blockaddress(@fn1, %ir-block.l_yes)
+    t2B %bb.3, 14, $noreg
+  
+  bb.3:
+    successors: %bb.4(0x80000000)
+  
+    INLINEASM &"", 1
+  
+  bb.4.l_yes (address-taken):
+    $sp = t2LDMIA_RET $sp, 14, $noreg, def $r4, def $pc
+
+...
Index: lib/CodeGen/IfConversion.cpp
===================================================================
--- lib/CodeGen/IfConversion.cpp
+++ lib/CodeGen/IfConversion.cpp
@@ -1758,9 +1758,15 @@
   if (!BBI1->IsBrAnalyzable)
     verifySameBranchInstructions(&MBB1, &MBB2);
 #endif
-  BBI1->NonPredSize -= TII->removeBranch(*BBI1->BB);
-  // Remove duplicated instructions.
+  // Remove duplicated instructions from the tail of MBB1: any branch
+  // instructions, and the common instructions counted by NumDups2.
   DI1 = MBB1.end();
+  while (DI1 != MBB1.begin()) {
+    MachineBasicBlock::iterator Prev = std::prev(DI1);
+    if (!Prev->isBranch() && !Prev->isDebugInstr())
+      break;
+    DI1 = Prev;
+  }
   for (unsigned i = 0; i != NumDups2; ) {
     // NumDups2 only counted non-dbg_value instructions, so this won't
     // run off the head of the list.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67203.218819.patch
Type: text/x-patch
Size: 3000 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190905/2b08e7f6/attachment.bin>