A temporary created AssertVH should be destroyed before the Value is deleted.
The bug has been introduced in https://reviews.llvm.org/D44177.
skatkov on Sep 4 2019, 9:24 PM.Authored by
Note: I've got the crash by changing the DomTreeUpdater form Lazy mode to Eager. Even with this fix it will not work because of LVI->eraseBlock(&BB);
It seems like there are multiple problems here if "BB" gets deleted eagerly: there the AssertingVH failure you're mentioning here, LVI->eraseBlock doesn't work, and the for loop "for (auto &BB : F)" also breaks. So I don't think this patch really solves anything.
Maybe it makes sense to change the TryToSimplifyUncondBranchFromEmptyBlock API so it doesn't erase the basic block, then call DeleteDeadBlock after we've cleaned up LVI etc.?
Depending on the number of call-sites of TryToSimplifyUncondBranchFromEmptyBlock this may be the way to go.
Originally the updates to DT were all immediate (Eager) and compile-time performance of the JumpThreading pass became considerably expensive. It's not uncommon for the pass to create BBs, modify the edges in/out of them, only to completely remove the code later when analysis at a different BB` shows the path is dead or unreachable.
The other, less-ideal, alternative is to always call LVI->eraseBlock(&BB) and simply lose the analysis for the block. It's no ideal but was actually the way this code behaved before Deferred DTU.
Thank you guys, for feedback.
I found this issue fighting against the crash in jump-threading. It is in progress now and goes slowly due to intermittent behavior.
I agree that this patch does not resolve the issue.
So, thank you again and sorry if I bothered you.
Hello @skatkov , I'm sorry to hear you're dealing with an intermittent crash. If you find a reproducible .ll file you can feed into opt -jump-threading -S < input.ll that crashes I would be interested in examining it. The Deferred usage of DTU causes unexpected issues and it is entirely possible there is still a bug here.