[WinEH] Simplify unreachable catchpads

Summary:
At least for CoreCLR, a catchpad which immediately executes an
`unreachable` instruction indicates that the exception can never have a
matching type, and so such catchpads can be removed, and so can their
catchswitches if the catchswitch becomes empty.

Reviewers: rnk, andrew.w.kaylor, majnemer

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D15846

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@256809 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/IR/Instructions.h b/include/llvm/IR/Instructions.h
index d781c7af36d7e3dda810dc5e68034fdbcd55e0de..1659ebf9d7c866f3fafcd78b4cd3edcc5483ad89 100644 (file)
--- a/include/llvm/IR/Instructions.h
+++ b/include/llvm/IR/Instructions.h
@@ -3966,6 +3966,8 @@ public:
   /// point to the added handler.
   void addHandler(BasicBlock *Dest);
 
   /// point to the added handler.
   void addHandler(BasicBlock *Dest);
 

+  void removeHandler(handler_iterator HI);
+

   unsigned getNumSuccessors() const { return getNumOperands() - 1; }
   BasicBlock *getSuccessor(unsigned Idx) const {
     assert(Idx < getNumSuccessors() &&
   unsigned getNumSuccessors() const { return getNumOperands() - 1; }
   BasicBlock *getSuccessor(unsigned Idx) const {
     assert(Idx < getNumSuccessors() &&

diff --git a/lib/IR/Instructions.cpp b/lib/IR/Instructions.cpp
index 4ae2fd522b52a5d3b81653177dd93f3e44c72907..0d6d23bb45db8275775747c7931af7a3e3dffabe 100644 (file)
--- a/lib/IR/Instructions.cpp
+++ b/lib/IR/Instructions.cpp
@@ -934,6 +934,17 @@ void CatchSwitchInst::addHandler(BasicBlock *Handler) {
   getOperandList()[OpNo] = Handler;
 }
 
   getOperandList()[OpNo] = Handler;
 }
 

+void CatchSwitchInst::removeHandler(handler_iterator HI) {
+  // Move all subsequent handlers up one.
+  Use *EndDst = op_end() - 1;
+  for (Use *CurDst = HI.getCurrent(); CurDst != EndDst; ++CurDst)
+    *CurDst = *(CurDst + 1);
+  // Null out the last handler use.
+  *EndDst = nullptr;
+
+  setNumHungOffUseOperands(getNumOperands() - 1);
+}
+

 BasicBlock *CatchSwitchInst::getSuccessorV(unsigned idx) const {
   return getSuccessor(idx);
 }
 BasicBlock *CatchSwitchInst::getSuccessorV(unsigned idx) const {
   return getSuccessor(idx);
 }

diff --git a/lib/Transforms/Utils/SimplifyCFG.cpp b/lib/Transforms/Utils/SimplifyCFG.cpp
index d0932f834cf58328af94480140d71b4d6678f3fa..3bb3fa5a301f2c117675495c52c894f8df37dd44 100644 (file)
--- a/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/ConstantFolding.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/ConstantFolding.h"

+#include "llvm/Analysis/EHPersonalities.h"

 #include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/ValueTracking.h"


@@ -3448,18 +3449,26 @@ bool SimplifyCFGOpt::SimplifyUnreachable(UnreachableInst *UI) {
     if (isa<CallInst>(BBI) && !isa<DbgInfoIntrinsic>(BBI)) break;
 
     if (BBI->mayHaveSideEffects()) {
     if (isa<CallInst>(BBI) && !isa<DbgInfoIntrinsic>(BBI)) break;
 
     if (BBI->mayHaveSideEffects()) {

-      if (StoreInst *SI = dyn_cast<StoreInst>(BBI)) {
+      if (auto *SI = dyn_cast<StoreInst>(BBI)) {

         if (SI->isVolatile())
           break;
         if (SI->isVolatile())
           break;

-      } else if (LoadInst *LI = dyn_cast<LoadInst>(BBI)) {
+      } else if (auto *LI = dyn_cast<LoadInst>(BBI)) {

         if (LI->isVolatile())
           break;
         if (LI->isVolatile())
           break;

-      } else if (AtomicRMWInst *RMWI = dyn_cast<AtomicRMWInst>(BBI)) {
+      } else if (auto *RMWI = dyn_cast<AtomicRMWInst>(BBI)) {

         if (RMWI->isVolatile())
           break;
         if (RMWI->isVolatile())
           break;

-      } else if (AtomicCmpXchgInst *CXI = dyn_cast<AtomicCmpXchgInst>(BBI)) {
+      } else if (auto *CXI = dyn_cast<AtomicCmpXchgInst>(BBI)) {

         if (CXI->isVolatile())
           break;
         if (CXI->isVolatile())
           break;

+      } else if (isa<CatchPadInst>(BBI)) {
+        // A catchpad may invoke exception object constructors and such, which
+        // in some languages can be arbitrary code, so be conservative by
+        // default.
+        // For CoreCLR, it just involves a type test, so can be removed.
+        if (classifyEHPersonality(BB->getParent()->getPersonalityFn()) !=
+            EHPersonality::CoreCLR)
+          break;

       } else if (!isa<FenceInst>(BBI) && !isa<VAArgInst>(BBI) &&
                  !isa<LandingPadInst>(BBI)) {
         break;
       } else if (!isa<FenceInst>(BBI) && !isa<VAArgInst>(BBI) &&
                  !isa<LandingPadInst>(BBI)) {
         break;


@@ -3485,7 +3494,7 @@ bool SimplifyCFGOpt::SimplifyUnreachable(UnreachableInst *UI) {
   for (unsigned i = 0, e = Preds.size(); i != e; ++i) {
     TerminatorInst *TI = Preds[i]->getTerminator();
     IRBuilder<> Builder(TI);
   for (unsigned i = 0, e = Preds.size(); i != e; ++i) {
     TerminatorInst *TI = Preds[i]->getTerminator();
     IRBuilder<> Builder(TI);

-    if (BranchInst *BI = dyn_cast<BranchInst>(TI)) {
+    if (auto *BI = dyn_cast<BranchInst>(TI)) {

       if (BI->isUnconditional()) {
         if (BI->getSuccessor(0) == BB) {
           new UnreachableInst(TI->getContext(), TI);
       if (BI->isUnconditional()) {
         if (BI->getSuccessor(0) == BB) {
           new UnreachableInst(TI->getContext(), TI);


@@ -3502,7 +3511,7 @@ bool SimplifyCFGOpt::SimplifyUnreachable(UnreachableInst *UI) {
           Changed = true;
         }
       }
           Changed = true;
         }
       }

-    } else if (SwitchInst *SI = dyn_cast<SwitchInst>(TI)) {
+    } else if (auto *SI = dyn_cast<SwitchInst>(TI)) {

       for (SwitchInst::CaseIt i = SI->case_begin(), e = SI->case_end();
            i != e; ++i)
         if (i.getCaseSuccessor() == BB) {
       for (SwitchInst::CaseIt i = SI->case_begin(), e = SI->case_end();
            i != e; ++i)
         if (i.getCaseSuccessor() == BB) {


@@ -3511,18 +3520,49 @@ bool SimplifyCFGOpt::SimplifyUnreachable(UnreachableInst *UI) {
           --i; --e;
           Changed = true;
         }
           --i; --e;
           Changed = true;
         }

-    } else if ((isa<InvokeInst>(TI) &&
-                cast<InvokeInst>(TI)->getUnwindDest() == BB) ||
-               isa<CatchSwitchInst>(TI)) {
-      removeUnwindEdge(TI->getParent());
-      Changed = true;
+    } else if (auto *II = dyn_cast<InvokeInst>(TI)) {
+      if (II->getUnwindDest() == BB) {
+        removeUnwindEdge(TI->getParent());
+        Changed = true;
+      }
+    } else if (auto *CSI = dyn_cast<CatchSwitchInst>(TI)) {
+      if (CSI->getUnwindDest() == BB) {
+        removeUnwindEdge(TI->getParent());
+        Changed = true;
+        continue;
+      }
+
+      for (CatchSwitchInst::handler_iterator I = CSI->handler_begin(),
+                                             E = CSI->handler_end();
+           I != E; ++I) {
+        if (*I == BB) {
+          CSI->removeHandler(I);
+          --I;
+          --E;
+          Changed = true;
+        }
+      }
+      if (CSI->getNumHandlers() == 0) {
+        BasicBlock *CatchSwitchBB = CSI->getParent();
+        if (CSI->hasUnwindDest()) {
+          // Redirect preds to the unwind dest
+          CatchSwitchBB->replaceAllUsesWith(CSI->getUnwindDest());
+        } else {
+          // Rewrite all preds to unwind to caller (or from invoke to call).
+          SmallVector<BasicBlock *, 8> EHPreds(predecessors(CatchSwitchBB));
+          for (BasicBlock *EHPred : EHPreds)
+            removeUnwindEdge(EHPred);
+        }
+        // The catchswitch is no longer reachable.
+        new UnreachableInst(CSI->getContext(), CSI);
+        CSI->eraseFromParent();
+        Changed = true;
+      }

     } else if (isa<CleanupReturnInst>(TI)) {
       new UnreachableInst(TI->getContext(), TI);
       TI->eraseFromParent();
       Changed = true;
     }
     } else if (isa<CleanupReturnInst>(TI)) {
       new UnreachableInst(TI->getContext(), TI);
       TI->eraseFromParent();
       Changed = true;
     }

-    // TODO: We can remove a catchswitch if all it's catchpads end in
-    // unreachable.

   }
 
   // If this block is now dead, remove it.
   }
 
   // If this block is now dead, remove it.

diff --git a/test/Transforms/SimplifyCFG/empty-catchpad.ll b/test/Transforms/SimplifyCFG/empty-catchpad.ll
new file mode 100644 (file)
index 0000000..2926cd3
--- /dev/null
+++ b/test/Transforms/SimplifyCFG/empty-catchpad.ll
@@ -0,0 +1,115 @@
+; RUN: opt < %s -simplifycfg -S | FileCheck %s
+
+declare void @f()
+declare void @llvm.foo(i32) nounwind
+declare void @ProcessCLRException()
+
+define void @test1() personality void ()* @ProcessCLRException {
+entry:
+  invoke void @f()
+    to label %exit unwind label %exn.dispatch
+exn.dispatch:
+  %cs = catchswitch within none [label %pad1, label %pad2] unwind to caller
+pad1:
+  %cp1 = catchpad within %cs [i32 1]
+  call void @llvm.foo(i32 1)
+  catchret from %cp1 to label %exit
+pad2:
+  %cp2 = catchpad within %cs [i32 2]
+  unreachable
+exit:
+  ret void
+}
+; Remove unreachble catch2, leave catch1 as-is
+; CHECK-LABEL: define void @test1()
+; CHECK: %cs = catchswitch within none [label %pad1] unwind to caller
+; CHECK-NOT: catchpad
+; CHECK: %cp1 = catchpad within %cs [i32 1]
+; CHECK-NOT: catchpad
+
+; Remove both catchpads and the catchswitch from exn.dispatch
+; CHECK-LABEL: define void @test2()
+define void @test2() personality void ()* @ProcessCLRException {
+entry:
+  invoke void @f()
+    to label %via.cleanup unwind label %exn.dispatch
+  ; CHECK-NOT: invoke
+  ; CHECK: call void @f()
+via.cleanup:
+  invoke void @f()
+    to label %via.catchswitch unwind label %cleanup.inner
+cleanup.inner:
+  %cp.inner = cleanuppad within none []
+  call void @llvm.foo(i32 0)
+  cleanupret from %cp.inner unwind label %exn.dispatch
+  ; CHECK: cleanupret from %cp.inner unwind to caller
+via.catchswitch:
+  invoke void @f()
+    to label %exit unwind label %dispatch.inner
+dispatch.inner:
+  %cs.inner = catchswitch within none [label %pad.inner] unwind label %exn.dispatch
+  ; CHECK: %cs.inner = catchswitch within none [label %pad.inner] unwind to caller
+pad.inner:
+  %catch.inner = catchpad within %cs.inner [i32 0]
+  ; CHECK: %catch.inner = catchpad within %cs.inner
+  call void @llvm.foo(i32 1)
+  catchret from %catch.inner to label %exit
+exn.dispatch:
+  %cs = catchswitch within none [label %pad1, label %pad2] unwind to caller
+  ; CHECK-NOT: catchswitch within
+  ; CHECK-NOT: catchpad
+pad1:
+  catchpad within %cs [i32 1]
+  unreachable
+pad2:
+  catchpad within %cs [i32 2]
+  unreachable
+exit:
+  ret void
+}
+
+; Same as @test2, but exn.dispatch catchswitch has an unwind dest that
+; preds need to be reidrected to
+; CHECK-LABEL: define void @test3()
+define void @test3() personality void ()* @ProcessCLRException {
+entry:
+  invoke void @f()
+    to label %via.cleanup unwind label %exn.dispatch
+  ; CHECK: invoke void @f()
+  ; CHECK-NEXT: to label %via.cleanup unwind label %cleanup
+via.cleanup:
+  invoke void @f()
+    to label %via.catchswitch unwind label %cleanup.inner
+cleanup.inner:
+  %cp.inner = cleanuppad within none []
+  call void @llvm.foo(i32 0)
+  cleanupret from %cp.inner unwind label %exn.dispatch
+  ; CHECK: cleanupret from %cp.inner unwind label %cleanup
+via.catchswitch:
+  invoke void @f()
+    to label %exit unwind label %dispatch.inner
+dispatch.inner:
+  %cs.inner = catchswitch within none [label %pad.inner] unwind label %exn.dispatch
+  ; CHECK: %cs.inner = catchswitch within none [label %pad.inner] unwind label %cleanup
+pad.inner:
+  %catch.inner = catchpad within %cs.inner [i32 0]
+  ; CHECK: %catch.inner = catchpad within %cs.inner
+  call void @llvm.foo(i32 1)
+  catchret from %catch.inner to label %exit
+exn.dispatch:
+  %cs = catchswitch within none [label %pad1, label %pad2] unwind label %cleanup
+  ; CHECK-NOT: catchswitch within
+  ; CHECK-NOT: catchpad
+pad1:
+  catchpad within %cs [i32 1]
+  unreachable
+pad2:
+  catchpad within %cs [i32 2]
+  unreachable
+cleanup:
+  %cp = cleanuppad within none []
+  call void @llvm.foo(i32 0)
+  cleanupret from %cp unwind to caller
+exit:
+  ret void
+}