[PATCH] D80047: Don't tail merge EHPads

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks updated this revision to Diff 265584.
aeubanks added a comment.

Add test
 Made sure it fails without this patch
 I thought the MIR serialization didn't support landing pads but upon closer look at https://llvm.org/docs/MIRLangRef.html#miscellaneous-attributes it does. I also figured out that you could manually specify successors even if they aren't implicit, else the landing pad BB would get removed.

If we could get rid of the edge cases and add a check to the machine verifier for control flow into landing pads, that could help with potential similar future issues.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80047/new/

https://reviews.llvm.org/D80047

Files:
  llvm/lib/CodeGen/BranchFolding.cpp
  llvm/test/CodeGen/X86/branchfolding-ehpad.mir


Index: llvm/test/CodeGen/X86/branchfolding-ehpad.mir
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/X86/branchfolding-ehpad.mir
@@ -0,0 +1,23 @@
+# RUN: llc -mtriple=x86_64-windows-msvc -verify-machineinstrs -run-pass branch-folder -o - %s | FileCheck %s
+---
+name: foo
+body: |
+  ; CHECK-LABEL: name: foo
+  bb.0:
+    successors: %bb.1, %bb.3
+  bb.1:
+    JCC_1 %bb.4, 5, implicit killed $eflags
+  bb.2:
+    MOV8mi $r13, 1, $noreg, 0, $noreg, 0
+    JMP_1 %bb.5
+  ; CHECK: bb.2:
+  ; CHECK-NOT: successors: {{.*}}bb.3
+  ; CHECK: bb.3 (landing-pad):
+  bb.3(landing-pad):
+    MOV8mi $r13, 1, $noreg, 0, $noreg, 0
+    JMP_1 %bb.5
+  bb.4:
+    MOV8mi $r13, 2, $noreg, 0, $noreg, 0
+  bb.5:
+    RET 0
+...
Index: llvm/lib/CodeGen/BranchFolding.cpp
===================================================================
--- llvm/lib/CodeGen/BranchFolding.cpp
+++ llvm/lib/CodeGen/BranchFolding.cpp
@@ -921,10 +921,10 @@
       continue;
     }
 
-    // If one of the blocks is the entire common tail (and not the entry
-    // block, which we can't jump to), we can treat all blocks with this same
-    // tail at once.  Use PredBB if that is one of the possibilities, as that
-    // will not introduce any extra branches.
+    // If one of the blocks is the entire common tail (and is not the entry
+    // block/an EH pad, which we can't jump to), we can treat all blocks with
+    // this same tail at once.  Use PredBB if that is one of the possibilities,
+    // as that will not introduce any extra branches.
     MachineBasicBlock *EntryBB =
         &MergePotentials.front().getBlock()->getParent()->front();
     unsigned commonTailIndex = SameTails.size();
@@ -932,19 +932,21 @@
     // into the other.
     if (SameTails.size() == 2 &&
         SameTails[0].getBlock()->isLayoutSuccessor(SameTails[1].getBlock()) &&
-        SameTails[1].tailIsWholeBlock())
+        SameTails[1].tailIsWholeBlock() && !SameTails[1].getBlock()->isEHPad())
       commonTailIndex = 1;
     else if (SameTails.size() == 2 &&
              SameTails[1].getBlock()->isLayoutSuccessor(
-                                                     SameTails[0].getBlock()) &&
-             SameTails[0].tailIsWholeBlock())
+                 SameTails[0].getBlock()) &&
+             SameTails[0].tailIsWholeBlock() &&
+             !SameTails[0].getBlock()->isEHPad())
       commonTailIndex = 0;
     else {
       // Otherwise just pick one, favoring the fall-through predecessor if
       // there is one.
       for (unsigned i = 0, e = SameTails.size(); i != e; ++i) {
         MachineBasicBlock *MBB = SameTails[i].getBlock();
-        if (MBB == EntryBB && SameTails[i].tailIsWholeBlock())
+        if ((MBB == EntryBB || MBB->isEHPad()) &&
+            SameTails[i].tailIsWholeBlock())
           continue;
         if (MBB == PredBB) {
           commonTailIndex = i;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80047.265584.patch
Type: text/x-patch
Size: 2922 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200521/274d2ab1/attachment-0001.bin>