[PATCH] D15846: [WinEH] Simplify unreachable catchpads

Joseph Tremoulet via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 4 10:26:40 PST 2016


JosephTremoulet marked 5 inline comments as done.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3528
@@ +3527,3 @@
+      }
+    } else if (auto *CSI = dyn_cast<CatchSwitchInst>(TI)) {
+      if (CSI->getUnwindDest() == BB) {
----------------
Sure.  I went ahead and changed the rest of the function to use auto in these situations, to keep it consistent.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3546
@@ +3545,3 @@
+      if (CSI->getNumHandlers() == 0) {
+        assert(Changed);
+        // Redirect preds to the unwind dest if there is one, then insert
----------------
I guess the intention of the assertion wasn't clear -- you seem to be interpreting it as "assert that we only eliminate a catchswitch if the code just above is what made it empty", but that's not what I was worried about (empty catchswitches fail the verifier anyway).  This was just "We're about to change the IR, but we don't need `Changed = true;` here because we know `Changed` is already `true` if we've reached this spot".

I'm happy to add an assert message, or put in both asserts, or just replace the assertion with `Changed = true;` for simplicity; do you have a preference?


http://reviews.llvm.org/D15846





More information about the llvm-commits mailing list