Skip to content

Commit 83ea7ce

Browse files
authored
[BOLT][NFC] Track fragment relationships using EquivalenceClasses
Three-way splitting can create references between split fragments (warm to cold or vice versa) that are not handled by `isChildOf/isParentOf/isChildOrParentOf`. Generalize fragment relationships to allow checking if two functions belong to one group, potentially in presence of ICF which can join multiple groups. Test Plan: NFC for existing tests Reviewers: maksfb, ayermolo, rafaelauler, dcci Reviewed By: rafaelauler Pull Request: #99979
1 parent 1feef92 commit 83ea7ce

File tree

4 files changed

+20
-16
lines changed

4 files changed

+20
-16
lines changed

bolt/include/bolt/Core/BinaryContext.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "bolt/RuntimeLibs/RuntimeLibrary.h"
2424
#include "llvm/ADT/AddressRanges.h"
2525
#include "llvm/ADT/ArrayRef.h"
26+
#include "llvm/ADT/EquivalenceClasses.h"
2627
#include "llvm/ADT/StringMap.h"
2728
#include "llvm/ADT/iterator.h"
2829
#include "llvm/BinaryFormat/Dwarf.h"
@@ -241,6 +242,10 @@ class BinaryContext {
241242
/// Function fragments to skip.
242243
std::unordered_set<BinaryFunction *> FragmentsToSkip;
243244

245+
/// Fragment equivalence classes to query belonging to the same "family" in
246+
/// presence of multiple fragments/multiple parents.
247+
EquivalenceClasses<const BinaryFunction *> FragmentClasses;
248+
244249
/// The runtime library.
245250
std::unique_ptr<RuntimeLibrary> RtLibrary;
246251

@@ -1032,7 +1037,15 @@ class BinaryContext {
10321037
/// fragment_name == parent_name.cold(.\d+)?
10331038
/// True if the Function is registered, false if the check failed.
10341039
bool registerFragment(BinaryFunction &TargetFunction,
1035-
BinaryFunction &Function) const;
1040+
BinaryFunction &Function);
1041+
1042+
/// Return true if two functions belong to the same "family": are fragments
1043+
/// of one another, or fragments of the same parent, or transitively fragment-
1044+
/// related.
1045+
bool areRelatedFragments(const BinaryFunction *LHS,
1046+
const BinaryFunction *RHS) const {
1047+
return FragmentClasses.isEquivalent(LHS, RHS);
1048+
}
10361049

10371050
/// Add interprocedural reference for \p Function to \p Address
10381051
void addInterproceduralReference(BinaryFunction *Function, uint64_t Address) {

bolt/include/bolt/Core/BinaryFunction.h

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1793,11 +1793,6 @@ class BinaryFunction {
17931793
return ParentFragments.contains(&Other);
17941794
}
17951795

1796-
/// Returns if this function is a parent of \p Other function.
1797-
bool isParentOf(const BinaryFunction &Other) const {
1798-
return Fragments.contains(&Other);
1799-
}
1800-
18011796
/// Return the child fragment form parent function
18021797
iterator_range<FragmentsSetTy::const_iterator> getFragments() const {
18031798
return iterator_range<FragmentsSetTy::const_iterator>(Fragments.begin(),
@@ -1807,11 +1802,6 @@ class BinaryFunction {
18071802
/// Return the parent function for split function fragments.
18081803
FragmentsSetTy *getParentFragments() { return &ParentFragments; }
18091804

1810-
/// Returns if this function is a parent or child of \p Other function.
1811-
bool isParentOrChildOf(const BinaryFunction &Other) const {
1812-
return isChildOf(Other) || isParentOf(Other);
1813-
}
1814-
18151805
/// Set the profile data for the number of times the function was called.
18161806
BinaryFunction &setExecutionCount(uint64_t Count) {
18171807
ExecutionCount = Count;

bolt/lib/Core/BinaryContext.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ bool BinaryContext::analyzeJumpTable(const uint64_t Address,
646646
const BinaryFunction *TargetBF = getBinaryFunctionContainingAddress(Value);
647647
const bool DoesBelongToFunction =
648648
BF.containsAddress(Value) ||
649-
(TargetBF && TargetBF->isParentOrChildOf(BF));
649+
(TargetBF && areRelatedFragments(TargetBF, &BF));
650650
if (!DoesBelongToFunction) {
651651
LLVM_DEBUG({
652652
if (!BF.containsAddress(Value)) {
@@ -841,7 +841,7 @@ BinaryContext::getOrCreateJumpTable(BinaryFunction &Function, uint64_t Address,
841841
// Prevent associating a jump table to a specific fragment twice.
842842
// This simple check arises from the assumption: no more than 2 fragments.
843843
if (JT->Parents.size() == 1 && JT->Parents[0] != &Function) {
844-
assert(JT->Parents[0]->isParentOrChildOf(Function) &&
844+
assert(areRelatedFragments(JT->Parents[0], &Function) &&
845845
"cannot re-use jump table of a different function");
846846
// Duplicate the entry for the parent function for easy access
847847
JT->Parents.push_back(&Function);
@@ -1209,12 +1209,13 @@ void BinaryContext::generateSymbolHashes() {
12091209
}
12101210

12111211
bool BinaryContext::registerFragment(BinaryFunction &TargetFunction,
1212-
BinaryFunction &Function) const {
1212+
BinaryFunction &Function) {
12131213
assert(TargetFunction.isFragment() && "TargetFunction must be a fragment");
12141214
if (TargetFunction.isChildOf(Function))
12151215
return true;
12161216
TargetFunction.addParentFragment(Function);
12171217
Function.addFragment(TargetFunction);
1218+
FragmentClasses.unionSets(&TargetFunction, &Function);
12181219
if (!HasRelocations) {
12191220
TargetFunction.setSimple(false);
12201221
Function.setSimple(false);
@@ -1336,7 +1337,7 @@ void BinaryContext::processInterproceduralReferences() {
13361337

13371338
if (TargetFunction) {
13381339
if (TargetFunction->isFragment() &&
1339-
!TargetFunction->isChildOf(Function)) {
1340+
!areRelatedFragments(TargetFunction, &Function)) {
13401341
this->errs()
13411342
<< "BOLT-WARNING: interprocedural reference between unrelated "
13421343
"fragments: "

bolt/lib/Core/Exceptions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ Error BinaryFunction::parseLSDA(ArrayRef<uint8_t> LSDASectionData,
207207
"BOLT-ERROR: cannot find landing pad fragment");
208208
BC.addInterproceduralReference(this, Fragment->getAddress());
209209
BC.processInterproceduralReferences();
210-
assert(isParentOrChildOf(*Fragment) &&
210+
assert(BC.areRelatedFragments(this, Fragment) &&
211211
"BOLT-ERROR: cannot have landing pads in different functions");
212212
setHasIndirectTargetToSplitFragment(true);
213213
BC.addFragmentsToSkip(this);

0 commit comments

Comments
 (0)