Skip to content

[BOLT] Remove mutable from BB::LayoutIndex #93224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bolt/include/bolt/Core/BinaryBasicBlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ class BinaryBasicBlock {
unsigned Index{InvalidIndex};

/// Index in the current layout.
mutable unsigned LayoutIndex{InvalidIndex};
unsigned LayoutIndex{InvalidIndex};

/// Number of pseudo instructions in this block.
uint32_t NumPseudos{0};
Expand Down Expand Up @@ -891,7 +891,7 @@ class BinaryBasicBlock {
}

/// Set layout index. To be used by BinaryFunction.
void setLayoutIndex(unsigned Index) const { LayoutIndex = Index; }
void setLayoutIndex(unsigned Index) { LayoutIndex = Index; }

/// Needed by graph traits.
BinaryFunction *getParent() const { return getFunction(); }
Expand Down
10 changes: 3 additions & 7 deletions bolt/lib/Core/BinaryFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3641,8 +3641,8 @@ bool BinaryFunction::forEachEntryPoint(EntryPointCallbackTy Callback) const {

BinaryFunction::BasicBlockListType BinaryFunction::dfs() const {
BasicBlockListType DFS;
unsigned Index = 0;
std::stack<BinaryBasicBlock *> Stack;
std::set<BinaryBasicBlock *> Visited;

// Push entry points to the stack in reverse order.
//
Expand All @@ -3659,17 +3659,13 @@ BinaryFunction::BasicBlockListType BinaryFunction::dfs() const {
for (BinaryBasicBlock *const BB : reverse(EntryPoints))
Stack.push(BB);

for (BinaryBasicBlock &BB : blocks())
BB.setLayoutIndex(BinaryBasicBlock::InvalidIndex);

while (!Stack.empty()) {
BinaryBasicBlock *BB = Stack.top();
Stack.pop();

if (BB->getLayoutIndex() != BinaryBasicBlock::InvalidIndex)
if (Visited.find(BB) != Visited.end())
continue;

BB->setLayoutIndex(Index++);
Visited.insert(BB);
DFS.push_back(BB);

for (BinaryBasicBlock *SuccBB : BB->landing_pads()) {
Expand Down
5 changes: 4 additions & 1 deletion bolt/lib/Passes/IdenticalCodeFolding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,10 @@ Error IdenticalCodeFolding::runOnFunctions(BinaryContext &BC) {
"ICF breakdown", opts::TimeICF);
ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
// Make sure indices are in-order.
BF.getLayout().updateLayoutIndices();
if (opts::ICFUseDFS)
BF.getLayout().updateLayoutIndices(BF.dfs());
else
BF.getLayout().updateLayoutIndices();

// Pre-compute hash before pushing into hashtable.
// Hash instruction operands to minimize hash collisions.
Expand Down
3 changes: 3 additions & 0 deletions bolt/lib/Profile/YAMLProfileWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ YAMLProfileWriter::convert(const BinaryFunction &BF, bool UseDFS,
llvm::copy(UseDFS ? BF.dfs() : BF.getLayout().blocks(),
std::back_inserter(Order));

const FunctionLayout Layout = BF.getLayout();
Layout.updateLayoutIndices(Order);

for (const BinaryBasicBlock *BB : Order) {
yaml::bolt::BinaryBasicBlockProfile YamlBB;
YamlBB.Index = BB->getLayoutIndex();
Expand Down
Loading