From 4d2bc0adc63cf90111d849911ccdddaa0d886e60 Mon Sep 17 00:00:00 2001 From: Enna1 Date: Wed, 27 Nov 2024 09:01:12 +0800 Subject: [PATCH] [BOLT] Extract comparator for sorting functions by index into helper function (#116217) 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. --- bolt/include/bolt/Core/BinaryFunction.h | 13 +++++++++++++ bolt/lib/Core/BinaryContext.cpp | 8 +------- bolt/lib/Core/BinaryFunction.cpp | 11 +---------- bolt/lib/Passes/ReorderFunctions.cpp | 11 +---------- 4 files changed, 16 insertions(+), 27 deletions(-) diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h index 9730cca148c7..7560908c250c 100644 --- a/bolt/include/bolt/Core/BinaryFunction.h +++ b/bolt/include/bolt/Core/BinaryFunction.h @@ -2407,6 +2407,19 @@ inline raw_ostream &operator<<(raw_ostream &OS, return OS; } +/// Compare function by index if it is valid, fall back to the original address +/// otherwise. +inline bool compareBinaryFunctionByIndex(const BinaryFunction *A, + const BinaryFunction *B) { + if (A->hasValidIndex() && B->hasValidIndex()) + return A->getIndex() < B->getIndex(); + if (A->hasValidIndex() && !B->hasValidIndex()) + return true; + if (!A->hasValidIndex() && B->hasValidIndex()) + return false; + return A->getAddress() < B->getAddress(); +} + } // namespace bolt // GraphTraits specializations for function basic block graphs (CFGs) diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp index f246750209d6..a808ece12da1 100644 --- a/bolt/lib/Core/BinaryContext.cpp +++ b/bolt/lib/Core/BinaryContext.cpp @@ -1607,13 +1607,7 @@ std::vector BinaryContext::getSortedFunctions() { SortedFunctions.begin(), [](BinaryFunction &BF) { return &BF; }); - llvm::stable_sort(SortedFunctions, - [](const BinaryFunction *A, const BinaryFunction *B) { - if (A->hasValidIndex() && B->hasValidIndex()) { - return A->getIndex() < B->getIndex(); - } - return A->hasValidIndex(); - }); + llvm::stable_sort(SortedFunctions, compareBinaryFunctionByIndex); return SortedFunctions; } diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp index 5da777411ba7..a9ccaea3c438 100644 --- a/bolt/lib/Core/BinaryFunction.cpp +++ b/bolt/lib/Core/BinaryFunction.cpp @@ -385,16 +385,7 @@ bool BinaryFunction::isForwardCall(const MCSymbol *CalleeSymbol) const { if (CalleeBF) { if (CalleeBF->isInjected()) return true; - - if (hasValidIndex() && CalleeBF->hasValidIndex()) { - return getIndex() < CalleeBF->getIndex(); - } else if (hasValidIndex() && !CalleeBF->hasValidIndex()) { - return true; - } else if (!hasValidIndex() && CalleeBF->hasValidIndex()) { - return false; - } else { - return getAddress() < CalleeBF->getAddress(); - } + return compareBinaryFunctionByIndex(this, CalleeBF); } else { // Absolute symbol. ErrorOr CalleeAddressOrError = BC.getSymbolValue(*CalleeSymbol); diff --git a/bolt/lib/Passes/ReorderFunctions.cpp b/bolt/lib/Passes/ReorderFunctions.cpp index c2d540135bda..1256d71342b1 100644 --- a/bolt/lib/Passes/ReorderFunctions.cpp +++ b/bolt/lib/Passes/ReorderFunctions.cpp @@ -473,16 +473,7 @@ Error ReorderFunctions::runOnFunctions(BinaryContext &BC) { [](BinaryFunction &BF) { return &BF; }); // Sort functions by index. - llvm::stable_sort(SortedFunctions, - [](const BinaryFunction *A, const BinaryFunction *B) { - if (A->hasValidIndex() && B->hasValidIndex()) - return A->getIndex() < B->getIndex(); - if (A->hasValidIndex() && !B->hasValidIndex()) - return true; - if (!A->hasValidIndex() && B->hasValidIndex()) - return false; - return A->getAddress() < B->getAddress(); - }); + llvm::stable_sort(SortedFunctions, compareBinaryFunctionByIndex); for (const BinaryFunction *Func : SortedFunctions) { if (!Func->hasValidIndex())