[PATCH] D13424: [WinEH] Create a separate MBB for funclet prologues

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp:223
@@ -220,4 +222,3 @@
         MMI.setHasEHFunclets(true);
-      if (isa<CatchPadInst>(I) || isa<CatchEndPadInst>(I) ||
-          isa<CleanupEndPadInst>(I)) {
+      if (isa<CatchEndPadInst>(I) || isa<CleanupEndPadInst>(I)) {
         assert(&*BB->begin() == I &&
----------------
JosephTremoulet wrote:
> majnemer wrote:
> > JosephTremoulet wrote:
> > > One the one hand, it seems odd to allow PHIs in catchpads; in the IR each catchpad happens before the next, so it would be legal to have a use of this PHI in another PHI in a subsequent catchpad, whereas the MBB you're creating for the next catchpad's prolog won't have the MBB for this catchpad's prolog as a predecessor.
> > > 
> > > On the other hand, at the IR level, the catchpad is the only place that has invokes as predecessors but doesn't have backedges to the normal dest as predecessors and so is the only place to put the sort of PHI that we need...
> > > 
> > > This suggests to me that maybe instead of what you're doing here, we simply want EHPrep to split critical catchpad-normal-dest edges (before it mucks with PHIs)?
> > I think we have problems with PHIs in catchpad predecessors regardless of this change because the catchpad predecessor isn't a CFG predecessor.  Our solution to that problem was to use further preparation to clean-up the PHIs.
> Right, ok, so in my example the MBB for the second catchpad would have the transitive preds and so it would have pulled in all the incoming values from the first catchpad.
> 
> But then I'm still confused:
>  1. That seems like something that changes with the eager demotion -> no-demotion change; why is this assertion changing as part of creating MBBs for catchpads?
>  2. Aren't we doing the same for catchendpad/cleanupendpad?  Why do we need to assert against their PHIs?
OK, I see where the miscommunication has happened.  I only cared about the continue, not the assertion.  We can reenable the assertion for CatchPad.


http://reviews.llvm.org/D13424