[PATCH] D13274: [WinEH] Clone funclets with multiple parents

[Andy Kaylor via llvm-commits]

andrew.w.kaylor added inline comments.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:856-860
@@ +855,7 @@
+//
+// If the terminator is a catchret its successor is a block in its parent
+// funclet.  If the instruction returns to a block in the parent for which the
+// cloned funclet was created, the terminator in the original block must be
+// replaced by an unreachable instruction.  Otherwise the terminator in the
+// clone block must be replaced by an unreachable instruction.
+//
----------------
andrew.w.kaylor wrote:
> andrew.w.kaylor wrote:
> > JosephTremoulet wrote:
> > > Do the new tests cover these cases?  I.e. where funclet A includes a branch to block Shared, and funclet B has child catch funclet C, and Shared is the target of the catchret from C back to B?
> > I don't think so.  I'll add something.
> This isn't working and I'm not sure how it should be handled.  Consider the following function:
> 
> ```
> define void @test13() personality i32 (...)* @__CxxFrameHandler3 {
> entry:
>   invoke void @f()
>     to label %invoke.cont unwind label %left
> invoke.cont:
>   invoke void @f()
>     to label %exit unwind label %right
> left:
>   %l = catchpad []
>     to label %left.cont unwind label %left.end
> left.cont:
>   invoke void @f()
>     to label %left.ret unwind label %inner
> left.ret:
>   catchret %l to label %invoke.cont
> left.end:
>   catchendpad unwind to caller
> right:
>   %r = catchpad []
>     to label %right.catch unwind label %right.end
> right.catch:
>   invoke void @f()
>     to label %right.ret unwind label %inner
> right.ret:
>   catchret %r to label %exit
> right.end:
>   catchendpad unwind to caller
> shared:
>   call void @h(i32 0)
>   unreachable
> inner:
>   %i = catchpad []
>     to label %inner.catch unwind label %inner.end
> inner.catch:
>   call void @h(i32 1)
>   catchret %i to label %shared
> inner.end:
>   catchendpad unwind label %left.end
> exit:
>   ret void
> }
> 
> ```
> 
> The existing funclet coloring code will recognize that it needs a copy of %shared for both %left and %right, but because both are reached from the same catchret instruction one of the clones will be unreachable following cloneCommonBlocks().  Then when my new code gets to the point of cloning %inner it will remove the catchret in %inner.catch for whichever of the parents received the unreachable clone of %shared (because the reachable %shared belongs to the other funclet).
> 
> I could instead make yet another clone of %shared at this point, but that really just pushes the problem to the next block in cases where %shared has successors.  What I really need to do is to be able to find the unreachable clone.  Does that sound reasonable?
This was an old comment.  I'm not sure why it only showed up now.


Repository:
  rL LLVM

http://reviews.llvm.org/D13274