[PATCH] D90106: [JumpThreading] Set edge probabilities when creating basic blocks
Kazu Hirata via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 3 14:07:22 PST 2020
kazu added a comment.
In D90106#2371631 <https://reviews.llvm.org/D90106#2371631>, @anna wrote:
> Hi @kazu, just wanted to point out - We have seen multiple assertion failures in benchmarks downstream with this patch (or a related one built on this patch). Trying to revert this downstream shows there are dependent patches. Is this assertion seen and already fixed in trunk?
>
> Assertion failure is `BranchProbabilityInfo.cpp:1163: void llvm::BranchProbabilityInfo::setEdgeProbability(const llvm::BasicBlock*, const llvm::SmallVectorImpl&): Assertion `TotalNumerator <= BranchProbability::getDenominator() + Probs.size()'
>
> @yrouban narrowed it down to:
> "Debugging showed that the assertion detects incorrect set of probabilities (summ != 1.0) when they are copied from PredBB to NewBB in JumpThreadingPass::ThreadThroughTwoBasicBlocks(). The probabilities are corrupted in PredBB when it gets cloned instructions from its successor in JumpThreadingPass::DuplicateCondBranchOnPHIIntoPred(). So the bug is in this method which does not fix the probabilities. "
> He is working on a fix for this with upstream testcase.
Those follow-up patches do not touch the code to call `setEdgeProbability` from `JumpThreadingPass::ThreadThroughTwoBasicBlocks`, so I'm pretty sure that the issue isn't fixed on the trunk yet. Thank you for tracking down the bug. I'd appreciate if you put me as a reviewer on your patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90106/new/
https://reviews.llvm.org/D90106
More information about the llvm-commits
mailing list