[PATCH] D12434: [WinEH] Teach SimplfyCFG to eliminate empty cleanup pads.

Joseph Tremoulet via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 28 11:29:37 PDT 2015


JosephTremoulet added a comment.

>   It seems unlikely that anything would put a PHI node at the top of an empty cleanup pad


What about something like

  struct S { public: ~S() { } };
  int foo() {
    int state = 1;
    try {
      S destructible;
      may_throw();
      state = 2;
      may_throw();
      // cleanup for S inserted here, which could be empty after inlining; wouldn't it get a phi for state?
    } catch (...) {
      return state;
    }
    return 0;
  }

?

> There are a couple of places where it seemed like this could benefit from refactoring, but it seemed best for review purposes to simply note that and follow up after discussion of the basic implementation.


I agree.  Even in this same file, `SimplifyUnreachable` has redundant code for rewriting `invoke` -> `call` and probably should be extended to handle the EH pad -> 'unwind to caller' rewrites, and there's (going to be) a spot in WinEHPrepare where it would be useful to invoke this as a utility (see http://reviews.llvm.org/D12433#inline-100516 ).


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2965
@@ -2938,1 +2964,3 @@
+  BasicBlock *BB = RI->getParent();
+  Instruction *CPInst = cast<CleanupPadInst>(BB->getFirstNonPHI());
 
----------------
Cleanups can have internal control flow, so this `cast` could fail.  Presumably you want a `dyn_cast` and return `false` if it returns `nullptr`, since that would indicate a non-empty cleanup?  (I'm not sure if this routine should be assuming that trivial flow will have already been removed or not)

================
Comment at: test/Transforms/SimplifyCFG/empty-cleanuppad.ll:70-72
@@ +69,5 @@
+; CHECK:   catchret
+; CHECK-NOT: cleanuppad
+; CHECK: catchendblock:                                    ; preds = %catch.dispatch
+; CHECK:   catchendpad unwind to caller
+; CHECK: }
----------------
Should the CHECK-NOT follow the two CHECKs here instead of preceding them?  In the input the cleanup comes after the catchendpad.


Repository:
  rL LLVM

http://reviews.llvm.org/D12434





More information about the llvm-commits mailing list