[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:3039-3053
@@ +3038,17 @@
+      }
+    } else if (auto *CRI = dyn_cast<CleanupReturnInst>(Terminator)) {
+      // If we're cloning a cleanup pad and the uncloned cleanup pad unwinds
+      // to a block in the clone parent, we need to replace the cleanupret with
+      // an unreachable instruction.
+      BasicBlock *UnwindDest = CRI->getUnwindDest();
+      if (UnwindDest && FuncletBlocks[Parent].count(UnwindDest)) {
+        CRI->eraseFromParent();
+        new UnreachableInst(BB->getContext(), BB);
+        // Loop over all of the successors, removing BB's entry from any PHI
+        // nodes.
+        for (succ_iterator SI = succ_begin(BB), SE = succ_end(BB); SI != SE;
+             ++SI)
+          (*SI)->removePredecessor(BB);
+      }
+    }
+  }
----------------
JosephTremoulet wrote:
> andrew.w.kaylor wrote:
> > JosephTremoulet wrote:
> > > Cleanupret targets a sibling, not a parent.  So,
> > >  1. I don't think this code should be here.
> > >  2. I guess you should have code somewhere that updates these sibling->sibling edges, though you probably can't do that until both siblings have been cloned...
> > > 
> > > Also, whatever you do about this with cleanupret, I think cleanupendpad should get the same treatment
> > I see what you're saying, but cleanupret doesn't always unwind to a sibling.  If it's the last cleanup inside a catch it can unwind to the parent (as it does in most of the current test cases).
> > 
> > I'll add some test cases involving sibling and then see if I can work out a good way to handle them.
> Oh, yeah, huh, good point.
> 
> Thinking about that, I think that given a cleanupret/cleanupendpad:
>  - if it unwinds to a catchendpad, it's an edge to the grandparent (we give the catchendpad the color of the parent of the catch, and the catch it's exiting is the parent of the cleanup)
>  - if it unwinds to a cleanupendpad, it's an edge to the parent (we give the cleanupendpad the color of its cleanup, so this is a child cleanup unwinding to its parent cleanup)
>  - if it unwinds to a catchpad or a cleanup pad or a terminatepad, it's an edge to a sibling
> 
> Does that sound right?
That sounds right for IR generated in the expected ways.  I'm not certain that it isn't possible to construct legal IR that does other things (like cleanret returning to an arbitrary block in the parent).  I don't know why it would ever happen, but do our written rules disallow it?


Repository:
  rL LLVM

http://reviews.llvm.org/D13274