Skip to content

Commit a7b662d

Browse files
[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
1 parent c914877 commit a7b662d

File tree

2 files changed

+19
-3
lines changed

2 files changed

+19
-3
lines changed

llvm/include/llvm/Analysis/BranchProbabilityInfo.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ class BranchProbabilityInfo {
6262
}
6363

6464
BranchProbabilityInfo(BranchProbabilityInfo &&Arg)
65-
: Probs(std::move(Arg.Probs)), LastF(Arg.LastF),
65+
: Probs(std::move(Arg.Probs)), MaxSuccIdx(std::move(Arg.MaxSuccIdx)),
66+
LastF(Arg.LastF),
6667
PostDominatedByUnreachable(std::move(Arg.PostDominatedByUnreachable)),
6768
PostDominatedByColdCall(std::move(Arg.PostDominatedByColdCall)) {}
6869

@@ -72,6 +73,7 @@ class BranchProbabilityInfo {
7273
BranchProbabilityInfo &operator=(BranchProbabilityInfo &&RHS) {
7374
releaseMemory();
7475
Probs = std::move(RHS.Probs);
76+
MaxSuccIdx = std::move(RHS.MaxSuccIdx);
7577
PostDominatedByColdCall = std::move(RHS.PostDominatedByColdCall);
7678
PostDominatedByUnreachable = std::move(RHS.PostDominatedByUnreachable);
7779
return *this;
@@ -273,6 +275,9 @@ class BranchProbabilityInfo {
273275

274276
DenseMap<Edge, BranchProbability> Probs;
275277

278+
// The maximum successor index ever entered for a given basic block.
279+
DenseMap<const BasicBlock *, unsigned> MaxSuccIdx;
280+
276281
/// Track the last function we run over for printing.
277282
const Function *LastF = nullptr;
278283

llvm/lib/Analysis/BranchProbabilityInfo.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,7 @@ bool BranchProbabilityInfo::calcInvokeHeuristics(const BasicBlock *BB) {
10311031

10321032
void BranchProbabilityInfo::releaseMemory() {
10331033
Probs.clear();
1034+
MaxSuccIdx.clear();
10341035
Handles.clear();
10351036
}
10361037

@@ -1136,6 +1137,11 @@ void BranchProbabilityInfo::setEdgeProbability(const BasicBlock *Src,
11361137
LLVM_DEBUG(dbgs() << "set edge " << Src->getName() << " -> "
11371138
<< IndexInSuccessors << " successor probability to " << Prob
11381139
<< "\n");
1140+
1141+
if (MaxSuccIdx.find(Src) == MaxSuccIdx.end())
1142+
MaxSuccIdx[Src] = IndexInSuccessors;
1143+
else
1144+
MaxSuccIdx[Src] = std::max(MaxSuccIdx[Src], IndexInSuccessors);
11391145
}
11401146

11411147
/// Set the edge probability for all edges at once.
@@ -1173,11 +1179,16 @@ BranchProbabilityInfo::printEdgeProbability(raw_ostream &OS,
11731179
}
11741180

11751181
void BranchProbabilityInfo::eraseBlock(const BasicBlock *BB) {
1176-
for (const_succ_iterator I = succ_begin(BB), E = succ_end(BB); I != E; ++I) {
1177-
auto MapI = Probs.find(std::make_pair(BB, I.getSuccessorIndex()));
1182+
auto It = MaxSuccIdx.find(BB);
1183+
if (It == MaxSuccIdx.end())
1184+
return;
1185+
1186+
for (unsigned I = 0, E = It->second; I <= E; ++I) {
1187+
auto MapI = Probs.find(std::make_pair(BB, I));
11781188
if (MapI != Probs.end())
11791189
Probs.erase(MapI);
11801190
}
1191+
MaxSuccIdx.erase(BB);
11811192
}
11821193

11831194
void BranchProbabilityInfo::calculate(const Function &F, const LoopInfo &LI,

0 commit comments

Comments
 (0)