[PATCH] D80047: Don't tail merge EHPads

[Arthur Eubanks via Phabricator via llvm-commits]

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

Don't jump to landing pads in Control Flow Optimizer

What I'm confused about though is this comment in MachineVerifier:

  // Block unconditionally branches somewhere.
  // If the block has exactly one successor, that happens to be a
  // landingpad, accept it as valid control flow.

which originates from fb6eeb74c58a

  [MachineVerifier] Accept a MBB with a single landing pad successor.
  
  The MachineVerifier used to check that there was always exactly one
  unconditional branch to a non-landingpad (normal) successor.
  If that normal successor to an invoke BB is unreachable, it seems
  reasonable to only have one successor, the landing pad.
  On targets other than AArch64 (and on AArch64 with a different testcase),
  the branch folder turns the branch to the landing pad into a fallthrough.
  The MachineVerifier, which relies on AnalyzeBranch, is unable to check
  the condition, and doesn't complain. However, it does in this specific
  testcase, where the branch to the landing pad remained.
  Make the MachineVerifier accept it.

I was under the impression that you couldn't jump/fallthrough to landing pads.


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


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.265527.patch
Type: text/x-patch
Size: 2150 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200521/483679dd/attachment.bin>