[PATCH] D90106: [JumpThreading] Set edge probabilities when creating basic blocks

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