[PATCH] D16319: [Inliner/WinEH] Honor implicit nounwinds

[Joseph Tremoulet via llvm-commits]

JosephTremoulet added inline comments.

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:615-624
@@ +614,12 @@
+        Value *UnwindDestToken;
+        if (auto *ParentPad =
+                dyn_cast<Instruction>(CatchSwitch->getParentPad())) {
+          // This catchswitch is nested inside another funclet.  If that
+          // funclet has an unwind destination within the inlinee, then
+          // unwinding out of this catchswitch would be UB.  Rewriting this
+          // catchswitch to unwind to the inlined invoke's unwind dest would
+          // give the parent funclet multiple unwind destinations, which is
+          // something that subsequent EH table generation can't handle and
+          // that the veirifer rejects.  So when we see such a call, leave it
+          // as "unwind to caller".
+          UnwindDestToken = getUnwindDestToken(ParentPad, FuncletUnwindMap);
----------------
majnemer wrote:
> JosephTremoulet wrote:
> > No, this happens when you have an unwind-to-caller catchswitch nested inside an outer pad which unwinds somewhere else in the inlinee.  E.g. if you start with
> > 
> > ```
> > define void inlinee() {
> >   ...
> >   %parent = cleanuppad within none []
> > ...
> >   %current = catchswitch within %parent [label ...] unwind label %sibling
> > ...
> > sibling:
> >   %sib = cleanuppad within %parent []
> >   unreachable
> > ...
> >   cleanupret from %parent unwind label %uncle
> > ```
> > 
> > then SimplifyUnreachable could turn that into
> > 
> > ```
> > define void inlinee() {
> >   ...
> >   %parent = cleanuppad within none []
> > ...
> >   %current = catchswitch within %parent [label ...] unwind to caller
> > ...
> >   cleanupret from %parent unwind label %uncle
> > ```
> > 
> > and //that's// the case this code is dealing with.
> Would we need to do this if we had catchswitches which could be annotated as nounwind?
We would not.  I think that would be a better solution long-term, though we'd still have to keep much of the rest of the logic in this patch unless we start requiring IR producers to explicitly annotate the analogous calls 'nounwind' (which might be a reasonable thing to require... fwiw, I've been going back and forth on whether I like that idea).


http://reviews.llvm.org/D16319