Skip to content

Commit 24b522d

Browse files
jmorsetstellar
authored andcommitted
[BPI] Transfer value-handles when assign/move constructing BPI (llvm#77774)
Background: BPI stores a collection of edge branch-probabilities, and also a set of Callback value-handles for the blocks in the edge-collection. When a block is deleted, BPI's eraseBlock method is called to clear the edge-collection of references to that block, to avoid dangling pointers. However, when move-constructing or assigning a BPI object, the edge-collection gets moved, but the value-handles are discarded. This can lead to to stale entries in the edge-collection when blocks are deleted without the callback -- not normally a problem, but if a new block is allocated with the same address as an old block, spurious branch probabilities will be recorded about it. The fix is to transfer the handles from the source BPI object. This was exposed by an unrelated debug-info change, it probably just shifted around allocation orders to expose this. Detected as nondeterminism and reduced by Zequan Wu: llvm@f1b0a54#commitcomment-136737090 (No test because IMHO testing for a behaviour that varies with memory allocators is likely futile; I can add the reproducer with a CHECK for the relevant branch weights if it's desired though) (cherry picked from commit 604a6c4)
1 parent 9c992d8 commit 24b522d

File tree

1 file changed

+11
-2
lines changed

1 file changed

+11
-2
lines changed

llvm/include/llvm/Analysis/BranchProbabilityInfo.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,16 +122,23 @@ class BranchProbabilityInfo {
122122
}
123123

124124
BranchProbabilityInfo(BranchProbabilityInfo &&Arg)
125-
: Probs(std::move(Arg.Probs)), LastF(Arg.LastF),
126-
EstimatedBlockWeight(std::move(Arg.EstimatedBlockWeight)) {}
125+
: Handles(std::move(Arg.Handles)), Probs(std::move(Arg.Probs)),
126+
LastF(Arg.LastF),
127+
EstimatedBlockWeight(std::move(Arg.EstimatedBlockWeight)) {
128+
for (auto &Handle : Handles)
129+
Handle.setBPI(this);
130+
}
127131

128132
BranchProbabilityInfo(const BranchProbabilityInfo &) = delete;
129133
BranchProbabilityInfo &operator=(const BranchProbabilityInfo &) = delete;
130134

131135
BranchProbabilityInfo &operator=(BranchProbabilityInfo &&RHS) {
132136
releaseMemory();
137+
Handles = std::move(RHS.Handles);
133138
Probs = std::move(RHS.Probs);
134139
EstimatedBlockWeight = std::move(RHS.EstimatedBlockWeight);
140+
for (auto &Handle : Handles)
141+
Handle.setBPI(this);
135142
return *this;
136143
}
137144

@@ -279,6 +286,8 @@ class BranchProbabilityInfo {
279286
}
280287

281288
public:
289+
void setBPI(BranchProbabilityInfo *BPI) { this->BPI = BPI; }
290+
282291
BasicBlockCallbackVH(const Value *V, BranchProbabilityInfo *BPI = nullptr)
283292
: CallbackVH(const_cast<Value *>(V)), BPI(BPI) {}
284293
};

0 commit comments

Comments
 (0)