[BOLT] Remove old layout from function layout

To track whether a function's new layout is different from its old
layout when updating it, the old layout would be kept around in memory
indefinitely (if the new layout is different). This was used only for
debugging/logging purposes. This patch forces the caller of function
layout's update method to copy the old layout into a temporary if they
need it by removing the old layout fields.

Reviewed By: rafauler

Differential Revision: https://reviews.llvm.org/D131413
This commit is contained in:
Fabian Parzefall 2022-08-17 15:06:15 -07:00
parent 0f8412c19c
commit aed75748de
4 changed files with 54 additions and 42 deletions

View File

@ -145,8 +145,6 @@ private:
/// `Fragments.size()` equals `this->size() + 1`. Always contains at least one
/// fragment.
FragmentListType Fragments = {0, 0};
BasicBlockListType PreviousBlocks;
FragmentListType PreviousFragments;
public:
/// Add an empty fragment.
@ -174,12 +172,9 @@ public:
/// Replace the current layout with NewLayout. Uses the block's
/// self-identifying fragment number to assign blocks to infer function
/// fragments.
void update(const ArrayRef<BinaryBasicBlock *> NewLayout);
/// Return true if the layout has been changed by basic block reordering,
/// false otherwise.
bool hasLayoutChanged() const { return !PreviousBlocks.empty(); }
/// fragments. Returns `true` if the new layout is different from the current
/// layout.
bool update(ArrayRef<BinaryBasicBlock *> NewLayout);
/// Clear layout releasing memory.
void clear();
@ -196,7 +191,8 @@ public:
/// Get the edit distance of the new layout with respect to the previous
/// layout after basic block reordering.
uint64_t getEditDistance() const;
uint64_t
getEditDistance(ArrayRef<const BinaryBasicBlock *> OldBlockOrder) const;
/// True if the function is split into at most 2 fragments. Mostly used for
/// checking whether a function can be processed in places that do not support

View File

@ -152,7 +152,9 @@ public:
};
private:
void modifyFunctionLayout(BinaryFunction &Function, LayoutType Type,
/// Run the specified layout algorithm on the given function. Returns `true`
/// if the order of blocks was changed.
bool modifyFunctionLayout(BinaryFunction &Function, LayoutType Type,
bool MinBranchClusters) const;
public:

View File

@ -76,18 +76,22 @@ void FunctionLayout::updateLayoutIndices() const {
}
}
void FunctionLayout::update(const ArrayRef<BinaryBasicBlock *> NewLayout) {
PreviousBlocks = std::move(Blocks);
PreviousFragments = std::move(Fragments);
bool FunctionLayout::update(const ArrayRef<BinaryBasicBlock *> NewLayout) {
const bool EqualBlockOrder = llvm::equal(Blocks, NewLayout);
if (EqualBlockOrder) {
const bool EqualPartitioning =
llvm::all_of(fragments(), [](const FunctionFragment FF) {
return llvm::all_of(FF, [&](const BinaryBasicBlock *const BB) {
return FF.Num == BB->getFragmentNum();
});
});
if (EqualPartitioning)
return false;
}
Blocks.clear();
Blocks = BasicBlockListType(NewLayout.begin(), NewLayout.end());
Fragments = {0, 0};
if (NewLayout.empty())
return;
copy(NewLayout, std::back_inserter(Blocks));
// Generate fragments
for (const auto &BB : enumerate(Blocks)) {
unsigned FragmentNum = BB.value()->getFragmentNum().get();
@ -106,19 +110,12 @@ void FunctionLayout::update(const ArrayRef<BinaryBasicBlock *> NewLayout) {
Fragments[FragmentNum + 1] = BB.index() + 1;
}
if (PreviousBlocks == Blocks && PreviousFragments == Fragments) {
// If new layout is the same as previous layout, clear previous layout so
// hasLayoutChanged() returns false.
PreviousBlocks = {};
PreviousFragments = {};
}
return true;
}
void FunctionLayout::clear() {
Blocks = {};
Fragments = {0, 0};
PreviousBlocks = {};
PreviousFragments = {0, 0};
}
BinaryBasicBlock *FunctionLayout::getBasicBlockAfter(const BinaryBasicBlock *BB,
@ -144,8 +141,9 @@ bool FunctionLayout::isSplit() const {
return NonEmptyFragCount >= 2;
}
uint64_t FunctionLayout::getEditDistance() const {
return ComputeEditDistance<BinaryBasicBlock *>(PreviousBlocks, Blocks);
uint64_t FunctionLayout::getEditDistance(
const ArrayRef<const BinaryBasicBlock *> OldBlockOrder) const {
return ComputeEditDistance<const BinaryBasicBlock *>(OldBlockOrder, Blocks);
}
FunctionLayout::block_const_iterator

View File

@ -11,11 +11,13 @@
//===----------------------------------------------------------------------===//
#include "bolt/Passes/BinaryPasses.h"
#include "bolt/Core/FunctionLayout.h"
#include "bolt/Core/ParallelUtilities.h"
#include "bolt/Passes/ReorderAlgorithm.h"
#include "bolt/Passes/ReorderFunctions.h"
#include "llvm/Support/CommandLine.h"
#include <atomic>
#include <mutex>
#include <numeric>
#include <vector>
@ -388,12 +390,25 @@ void ReorderBasicBlocks::runOnFunctions(BinaryContext &BC) {
if (opts::ReorderBlocks == ReorderBasicBlocks::LT_NONE)
return;
std::atomic<uint64_t> ModifiedFuncCount{0};
std::atomic_uint64_t ModifiedFuncCount(0);
std::mutex FunctionEditDistanceMutex;
DenseMap<const BinaryFunction *, uint64_t> FunctionEditDistance;
ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
modifyFunctionLayout(BF, opts::ReorderBlocks, opts::MinBranchClusters);
if (BF.getLayout().hasLayoutChanged())
++ModifiedFuncCount;
SmallVector<const BinaryBasicBlock *, 0> OldBlockOrder;
if (opts::PrintFuncStat > 0)
llvm::copy(BF.getLayout().blocks(), std::back_inserter(OldBlockOrder));
const bool LayoutChanged =
modifyFunctionLayout(BF, opts::ReorderBlocks, opts::MinBranchClusters);
if (LayoutChanged) {
ModifiedFuncCount.fetch_add(1, std::memory_order_relaxed);
if (opts::PrintFuncStat > 0) {
const uint64_t Distance = BF.getLayout().getEditDistance(OldBlockOrder);
std::lock_guard<std::mutex> Lock(FunctionEditDistanceMutex);
FunctionEditDistance[&BF] = Distance;
}
}
};
ParallelUtilities::PredicateTy SkipFunc = [&](const BinaryFunction &BF) {
@ -405,8 +420,9 @@ void ReorderBasicBlocks::runOnFunctions(BinaryContext &BC) {
"ReorderBasicBlocks");
outs() << "BOLT-INFO: basic block reordering modified layout of "
<< format("%zu (%.2lf%%) functions\n", ModifiedFuncCount.load(),
100.0 * ModifiedFuncCount.load() /
<< format("%zu (%.2lf%%) functions\n",
ModifiedFuncCount.load(std::memory_order_relaxed),
100.0 * ModifiedFuncCount.load(std::memory_order_relaxed) /
BC.getBinaryFunctions().size());
if (opts::PrintFuncStat > 0) {
@ -421,7 +437,7 @@ void ReorderBasicBlocks::runOnFunctions(BinaryContext &BC) {
OS << "\nBOLT-INFO: Printing Function Statistics:\n\n";
OS << " There are " << BFs.size() << " functions in total. \n";
OS << " Number of functions being modified: "
<< ModifiedFuncCount.load() << "\n";
<< ModifiedFuncCount.load(std::memory_order_relaxed) << "\n";
OS << " User asks for detailed information on top "
<< opts::PrintFuncStat << " functions. (Ranked by function score)"
<< "\n\n";
@ -439,23 +455,23 @@ void ReorderBasicBlocks::runOnFunctions(BinaryContext &BC) {
OS << " There are " << Function.getInstructionCount()
<< " number of instructions in this function.\n";
OS << " The edit distance for this function is: "
<< Function.getLayout().getEditDistance() << "\n\n";
<< FunctionEditDistance.lookup(&Function) << "\n\n";
}
}
}
void ReorderBasicBlocks::modifyFunctionLayout(BinaryFunction &BF,
bool ReorderBasicBlocks::modifyFunctionLayout(BinaryFunction &BF,
LayoutType Type,
bool MinBranchClusters) const {
if (BF.size() == 0 || Type == LT_NONE)
return;
return false;
BinaryFunction::BasicBlockOrderType NewLayout;
std::unique_ptr<ReorderAlgorithm> Algo;
// Cannot do optimal layout without profile.
if (Type != LT_REVERSE && !BF.hasValidProfile())
return;
return false;
if (Type == LT_REVERSE) {
Algo.reset(new ReverseReorderAlgorithm());
@ -500,7 +516,7 @@ void ReorderBasicBlocks::modifyFunctionLayout(BinaryFunction &BF,
Algo->reorderBasicBlocks(BF, NewLayout);
BF.getLayout().update(NewLayout);
return BF.getLayout().update(NewLayout);
}
void FixupBranches::runOnFunctions(BinaryContext &BC) {