[PATCH] D24117: Fix inliner funclet unwind memoization

[Joseph Tremoulet via llvm-commits]

JosephTremoulet marked an inline comment as done.

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:442-444
@@ +441,5 @@
+    // the MemoMap on that invocation, which isn't the case if we got here.
+    assert(!MemoMap.count(UselessPad) || TempMemos.count(UselessPad));
+    // Assert as we enumerate users that 'UselessPad' doesn't have any unwind
+    // information that we'd be contradicting by making a map entry for it
+    // (which is something that getUnwindDestTokenHelper must have proved for
----------------
> I think it would make it clear that it is safe to propagate UnwindDestToken downwards.

I agree.  Writing it up, I noticed that while the assert just looks at the current pad's users (avoiding the quadratic behavior of re-calling `getUnwindDestTokenHelper`), the fact that this walk keeps going and makes the same assertion at each level (plus the assertion that any unwind we do find targets a sibling) means that cumulatively the assertions are made throughout the tree, so I think it's pretty solid.

The one wrinkle was that when I said "in either case [`UselessPad`] has no `invoke` users", I was wrong -- what's actually true is "in either case, if `UselessPad` has any `invoke` users, they unwind to a child of `UselessPad`, rather than unwinding out of `UselessPad`"

Updated.


https://reviews.llvm.org/D24117