[BranchProbabilityInfo] Fix eraseBlock
authorKazu Hirata <kazu@google.com>
Tue, 27 Oct 2020 23:14:25 +0000 (16:14 -0700)
committerKazu Hirata <kazu@google.com>
Tue, 27 Oct 2020 23:14:25 +0000 (16:14 -0700)
commita7b662d0f4098371b96ce4446fb0eba79b0b649f
tree15a1de717c2a79a1193953295b3b32093807fa92
parentc91487769d80487eba1712a7a172a1c8977a9b4f
[BranchProbabilityInfo] Fix eraseBlock

This patch ensures that BranchProbabilityInfo::eraseBlock(BB) deletes
all entries in Probs associated with with BB.

Without this patch, stale entries for BB may remain in Probs after
eraseBlock(BB), leading to a situation where a newly created basic
block has an edge probability associated with it even before the pass
responsible for creating the basic block adds any edge probability to
it.

Consider the current implementation of eraseBlock(BB):

  for (const_succ_iterator I = succ_begin(BB), E = succ_end(BB); I != E; ++I) {
    auto MapI = Probs.find(std::make_pair(BB, I.getSuccessorIndex()));
    if (MapI != Probs.end())
      Probs.erase(MapI);
  }

Notice that it uses succ_begin(BB) and succ_end(BB), which are based
on BB->getTerminator().  This means that if the terminator changes
between calls to setEdgeProbability and eraseBlock, then we may not
examine all pairs associated with BB.

This is exactly what happens in MaybeMergeBasicBlockIntoOnlyPred,
which merges basic blocks A into B if A is the sole predecessor of B,
and B is the sole successor of A.  It replaces the terminator of A
with UnreachableInst before (indirectly) calling eraseBlock(A).

The patch fixes the problem by keeping track of all edge probablities
entered with setEdgeProbability in a map from BasicBlock* to a
successor index.

Differential Revision: https://reviews.llvm.org/D90272
llvm/include/llvm/Analysis/BranchProbabilityInfo.h
llvm/lib/Analysis/BranchProbabilityInfo.cpp