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

[Andy Kaylor via llvm-commits]

andrew.w.kaylor added a comment.

I understood that iterating sets and maps was not deterministic and thought I had accounted for that.  There are several places in this code where the non-determinism is OK because it doesn't lead to non-deterministic output.  I just had one bug where I was looking at unwind edges that hadn't been processed yet.

In any event, reverting was the right thing to do.  I should have this ready to go back in shortly.


================
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:
> 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?


Repository:
  rL LLVM

http://reviews.llvm.org/D13274