Skip to content

Commit 714df37

Browse files
committed
[BOLT] Extract comparator for sorting functions by index into helper function
This change extracts the comparator for sorting functions by index into a helper function `compareBinaryFunctionByIndex()` Not sure why the comparator used in `BinaryContext::getSortedFunctions()` is not same as the other two places. I think they should use the same comparator, so I also change `BinaryContext::getSortedFunctions()` to use `compareBinaryFunctionByIndex()` for sorting functions.
1 parent e5ca3ed commit 714df37

File tree

4 files changed

+16
-27
lines changed

4 files changed

+16
-27
lines changed

bolt/include/bolt/Core/BinaryFunction.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2366,6 +2366,19 @@ inline raw_ostream &operator<<(raw_ostream &OS,
23662366
return OS;
23672367
}
23682368

2369+
/// Compare function by index if it is valid, fall back to the original address
2370+
/// otherwise.
2371+
inline bool compareBinaryFunctionByIndex(const BinaryFunction *A,
2372+
const BinaryFunction *B) {
2373+
if (A->hasValidIndex() && B->hasValidIndex())
2374+
return A->getIndex() < B->getIndex();
2375+
if (A->hasValidIndex() && !B->hasValidIndex())
2376+
return true;
2377+
if (!A->hasValidIndex() && B->hasValidIndex())
2378+
return false;
2379+
return A->getAddress() < B->getAddress();
2380+
}
2381+
23692382
} // namespace bolt
23702383

23712384
// GraphTraits specializations for function basic block graphs (CFGs)

bolt/lib/Core/BinaryContext.cpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,13 +1607,7 @@ std::vector<BinaryFunction *> BinaryContext::getSortedFunctions() {
16071607
SortedFunctions.begin(),
16081608
[](BinaryFunction &BF) { return &BF; });
16091609

1610-
llvm::stable_sort(SortedFunctions,
1611-
[](const BinaryFunction *A, const BinaryFunction *B) {
1612-
if (A->hasValidIndex() && B->hasValidIndex()) {
1613-
return A->getIndex() < B->getIndex();
1614-
}
1615-
return A->hasValidIndex();
1616-
});
1610+
llvm::stable_sort(SortedFunctions, compareBinaryFunctionByIndex);
16171611
return SortedFunctions;
16181612
}
16191613

bolt/lib/Core/BinaryFunction.cpp

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -385,16 +385,7 @@ bool BinaryFunction::isForwardCall(const MCSymbol *CalleeSymbol) const {
385385
if (CalleeBF) {
386386
if (CalleeBF->isInjected())
387387
return true;
388-
389-
if (hasValidIndex() && CalleeBF->hasValidIndex()) {
390-
return getIndex() < CalleeBF->getIndex();
391-
} else if (hasValidIndex() && !CalleeBF->hasValidIndex()) {
392-
return true;
393-
} else if (!hasValidIndex() && CalleeBF->hasValidIndex()) {
394-
return false;
395-
} else {
396-
return getAddress() < CalleeBF->getAddress();
397-
}
388+
return compareBinaryFunctionByIndex(this, CalleeBF);
398389
} else {
399390
// Absolute symbol.
400391
ErrorOr<uint64_t> CalleeAddressOrError = BC.getSymbolValue(*CalleeSymbol);

bolt/lib/Passes/ReorderFunctions.cpp

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -473,16 +473,7 @@ Error ReorderFunctions::runOnFunctions(BinaryContext &BC) {
473473
[](BinaryFunction &BF) { return &BF; });
474474

475475
// Sort functions by index.
476-
llvm::stable_sort(SortedFunctions,
477-
[](const BinaryFunction *A, const BinaryFunction *B) {
478-
if (A->hasValidIndex() && B->hasValidIndex())
479-
return A->getIndex() < B->getIndex();
480-
if (A->hasValidIndex() && !B->hasValidIndex())
481-
return true;
482-
if (!A->hasValidIndex() && B->hasValidIndex())
483-
return false;
484-
return A->getAddress() < B->getAddress();
485-
});
476+
llvm::stable_sort(SortedFunctions, compareBinaryFunctionByIndex);
486477

487478
for (const BinaryFunction *Func : SortedFunctions) {
488479
if (!Func->hasValidIndex())

0 commit comments

Comments
 (0)