[PATCH] D15846: [WinEH] Simplify unreachable catchpads

[David Majnemer via llvm-commits]

majnemer added a comment.

Can we get a test with a `cleanupret` which unwinds to a `catchswitch` which has unreachable `catchpads` with and without it being `unwinds to caller`?


================
Comment at: lib/IR/Instructions.cpp:938
@@ +937,3 @@
+void CatchSwitchInst::removeHandler(handler_iterator HI) {
+  // Move all subsequent handlers up one
+  Use *EndDst = op_end() - 1;
----------------
Please end this with a period.

================
Comment at: lib/IR/Instructions.cpp:942
@@ +941,3 @@
+    *CurDst = *(CurDst + 1);
+  // Null out the last handler use
+  *EndDst = nullptr;
----------------
Ditto.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3523
@@ -3513,6 +3522,3 @@
         }
-    } else if ((isa<InvokeInst>(TI) &&
-                cast<InvokeInst>(TI)->getUnwindDest() == BB) ||
-               isa<CatchSwitchInst>(TI)) {
-      removeUnwindEdge(TI->getParent());
-      Changed = true;
+    } else if (InvokeInst *II = dyn_cast<InvokeInst>(TI)) {
+      if (II->getUnwindDest() == BB) {
----------------
Please use `auto *II` here.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3528
@@ +3527,3 @@
+      }
+    } else if (CatchSwitchInst *CSI = dyn_cast<CatchSwitchInst>(TI)) {
+      if (CSI->getUnwindDest() == BB) {
----------------
Please use `auto *CSI` here.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3545
@@ +3544,3 @@
+      }
+      if (!CSI->getNumHandlers()) {
+        assert(Changed);
----------------
I'd write this as `CSI->getNumHandlers() == 0`.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3546
@@ +3545,3 @@
+      if (!CSI->getNumHandlers()) {
+        assert(Changed);
+        // Redirect preds to the unwind dest if there is one, then insert
----------------
Couldn't some other predecessor cause `Changed` to be true?  I think we could rephrase this as an assertion that `CSI->getNumHandlers() != 0` before the loop.


http://reviews.llvm.org/D15846