[PATCH] D15139: [IR] Reformulate LLVM's EH funclet IR

[Andy Kaylor via llvm-commits]

andrew.w.kaylor accepted this revision.
andrew.w.kaylor added a comment.
This revision is now accepted and ready to land.

Except for the irreducible loop issue, this looks good to me.  I would be OK with putting off addressing that problem until some later time, as it seems like a fairly unlikely corner case, but I do think we should probably think about it.


================
Comment at: lib/CodeGen/WinEHPrepare.cpp:223-232
@@ -250,13 +222,12 @@
 
-static void calculateExplicitCXXStateNumbers(WinEHFuncInfo &FuncInfo,
-                                             const BasicBlock &BB,
-                                             int ParentState) {
-  assert(BB.isEHPad());
-  const Instruction *FirstNonPHI = BB.getFirstNonPHI();
-  // All catchpad instructions will be handled when we process their
-  // respective catchendpad instruction.
-  if (isa<CatchPadInst>(FirstNonPHI))
-    return;
+static void calculateCXXStateNumbers(WinEHFuncInfo &FuncInfo,
+                                     const Instruction *FirstNonPHI,
+                                     int ParentState) {
+  const BasicBlock *BB = FirstNonPHI->getParent();
+  assert(BB->isEHPad() && "not a funclet!");
+
+  if (auto *CatchSwitch = dyn_cast<CatchSwitchInst>(FirstNonPHI)) {
+    assert(FuncInfo.EHPadStateMap.count(CatchSwitch) == 0 &&
+           "shouldn't revist catch funclets!");
 
     SmallVector<const CatchPadInst *, 2> Handlers;
----------------
Sounds like a plan.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:250
@@ +249,3 @@
+    for (const auto *CatchPad : Handlers) {
+      FuncInfo.FuncletBaseStateMap[CatchPad] = CatchLow;
+      for (const User *U : CatchPad->users()) {
----------------
OK, if MSVC does this also then I feel a lot better about the long term viability of doing it this way.

================
Comment at: test/CodeGen/WinEH/wineh-cloning.ll:288-289
@@ -370,75 +287,4 @@
     to label %unreachable unwind label %right
 left:
-  cleanuppad []
-  br label %shared
-right:
-  catchpad []
-    to label %right.catch unwind label %right.end
-right.catch:
-  br label %shared
-right.end:
-  catchendpad unwind to caller
-shared:
-  invoke void @f()
-    to label %unreachable unwind label %inner
-inner:
-  cleanuppad []
-  invoke void @f()
-    to label %unreachable unwind label %inner.child
-inner.child:
-  cleanuppad []
-  %x = call i32 @g()
-  call void @h(i32 %x)
-  unreachable
-unreachable:
-  unreachable
-}
-; %inner is a two-parent child which itself has a child; need
-; to make two copies of both the %inner and %inner.child.
-; CHECK-LABEL: define void @test8(
-; CHECK:     invoke.cont:
-; CHECK:               to label %[[UNREACHABLE_ENTRY:.+]] unwind label %right
-; CHECK:     left:
-; CHECK:               to label %[[UNREACHABLE_LEFT:.+]] unwind label %[[INNER_LEFT:.+]]
-; CHECK:     right:
-; CHECK:               to label %right.catch unwind label %right.end
-; CHECK:     right.catch:
-; CHECK:               to label %[[UNREACHABLE_RIGHT:.+]] unwind label %[[INNER_RIGHT:.+]]
-; CHECK:     right.end:
-; CHECK:       catchendpad unwind to caller
-; CHECK:     [[INNER_RIGHT]]:
-; CHECK:               to label %[[UNREACHABLE_INNER_RIGHT:.+]] unwind label %[[INNER_CHILD_RIGHT:.+]]
-; CHECK:     [[INNER_LEFT]]:
-; CHECK:               to label %[[UNREACHABLE_INNER_LEFT:.+]] unwind label %[[INNER_CHILD_LEFT:.+]]
-; CHECK:     [[INNER_CHILD_RIGHT]]:
-; CHECK:       [[TMP:\%.+]] = cleanuppad []
-; CHECK:       [[X:\%.+]] = call i32 @g()
-; CHECK:       call void @h(i32 [[X]])
-; CHECK:       unreachable
-; CHECK:     [[INNER_CHILD_LEFT]]:
-; CHECK:       [[TMP:\%.+]] = cleanuppad []
-; CHECK:       [[X:\%.+]] = call i32 @g()
-; CHECK:       call void @h(i32 [[X]])
-; CHECK:       unreachable
-; CHECK:     [[UNREACHABLE_INNER_RIGHT]]:
-; CHECK:       unreachable
-; CHECK:     [[UNREACHABLE_INNER_LEFT]]:
-; CHECK:       unreachable
-; CHECK:     [[UNREACHABLE_RIGHT]]:
-; CHECK:       unreachable
-; CHECK:     [[UNREACHABLE_LEFT]]:
-; CHECK:       unreachable
-; CHECK:     [[UNREACHABLE_ENTRY]]:
-; CHECK:       unreachable
-
-
-define void @test9() personality i32 (...)* @__CxxFrameHandler3 {
-entry:
-  invoke void @f()
-    to label %invoke.cont unwind label %left
-invoke.cont:
-  invoke void @f()
-    to label %unreachable unwind label %right
-left:
-  cleanuppad []
+  cleanuppad within none []
   call void @h(i32 1)
----------------
The original concern was that some future optimization would innocently transform otherwise reasonable code into this form.  I think the hypothetically dangerous input looked something like this:
```
left:
  cleanuppad within none []
  call void @h(i32 1)
  invoke void @f()
    to label %unreachable unwind label %not_right
not_right:
  cleanuppad within none []
  call void @h(i32 2)
  call void @f()
  unreachable
right:
  cleanuppad within none []
  call void @h(i32 2)
  invoke void @f()
    to label %unreachable unwind label %not_left
not_left:
  cleanuppad within none []
  call void @h(i32 1)
  call void @f()
  unreachable
```
So if we can get that out of the clang front end, then a reasonable optimization could transform it into this test case, and I don't see how the code generator could do anything reasonable with the form shown in this test case.


http://reviews.llvm.org/D15139