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

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: include/llvm/IR/InstrTypes.h:1122
@@ +1121,3 @@
+
+  explicit FuncletPadInst(Instruction::FuncletPadOps Op, Value *OuterScope,
+                          ArrayRef<Value *> Args, unsigned Values,
----------------
JosephTremoulet wrote:
> (here and other constructors and `Create` methods) parameter should probably be renamed `ParentPad` as well.
Done.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:207
@@ -243,1 +206,3 @@
+static const BasicBlock *getEHPadFromPredecessor(const BasicBlock *BB,
+                                                 Value *OuterScope) {
   const TerminatorInst *TI = BB->getTerminator();
----------------
JosephTremoulet wrote:
> This parameter name too
Done.

================
Comment at: lib/IR/Verifier.cpp:2994
@@ -3032,1 +2993,3 @@
 
+  auto *OuterScope = CPI.getParentPad();
+  Assert(isa<CatchSwitchInst>(OuterScope) ||
----------------
JosephTremoulet wrote:
> nit: Also these locals (this, visitCatchSwitchInst, and visitTerminatePadInst)
Done.

================
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)
----------------
andrew.w.kaylor wrote:
> 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.
Such a hypothetical optimization would be defeated once we add support for bundle operands in the front-end.  The bundle operands will defeat tail-merging/CSE because the call-sites will look like they have different operands.


http://reviews.llvm.org/D15139