[PATCH] D24117: Fix inliner funclet unwind memoization
Joseph Tremoulet via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 2 18:08:53 PDT 2016
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`"
More information about the llvm-commits