From 4a0c494bc10eec3637fb1179aeea85137b1b1271 Mon Sep 17 00:00:00 2001 From: Bill Nell Date: Wed, 7 Sep 2016 18:59:23 -0700 Subject: [PATCH] BOLT: Remove restrictions on unreachable code elimination Summary: Allow UCE when blocks have EH info. Since UCE may remove blocks that are referenced from debugging info data structures, we don't actually delete them. We just mark them with an "invalid" index and store them in a different vector to be cleaned up later once the BinaryFunction is destroyed. The debugging code just skips any BBs that have an invalid index. Eliminating blocks may also expose useless jmp instructions, i.e. a jmp around a dead block could just be a fallthrough. I've added a new routine to cleanup these jmps. Although, @maks is working on changing fixBranches() so that it can be used instead. (cherry picked from FBD3793259) --- bolt/BinaryBasicBlock.cpp | 6 +- bolt/BinaryBasicBlock.h | 34 +++++--- bolt/BinaryFunction.cpp | 172 ++++++++++++++++++++++++++++--------- bolt/BinaryFunction.h | 50 +++++++++-- bolt/BinaryPassManager.cpp | 13 +-- bolt/BinaryPassManager.h | 1 - bolt/BinaryPasses.cpp | 57 ++++-------- bolt/BinaryPasses.h | 11 ++- bolt/DebugData.cpp | 3 + 9 files changed, 231 insertions(+), 116 deletions(-) diff --git a/bolt/BinaryBasicBlock.cpp b/bolt/BinaryBasicBlock.cpp index 9e0b3ff9b9d1..6432d103cc99 100644 --- a/bolt/BinaryBasicBlock.cpp +++ b/bolt/BinaryBasicBlock.cpp @@ -16,7 +16,6 @@ #include "llvm/MC/MCAsmInfo.h" #include "llvm/MC/MCContext.h" #include "llvm/MC/MCInst.h" -#include "llvm/MC/MCInstPrinter.h" #include #include @@ -27,7 +26,7 @@ namespace llvm { namespace bolt { bool operator<(const BinaryBasicBlock &LHS, const BinaryBasicBlock &RHS) { - return LHS.Offset < RHS.Offset; + return LHS.Index < RHS.Index; } MCInst *BinaryBasicBlock::getFirstNonPseudo() { @@ -185,7 +184,8 @@ uint32_t BinaryBasicBlock::getNumPseudos() const { return NumPseudos; } -void BinaryBasicBlock::dump(BinaryContext& BC) const { +void BinaryBasicBlock::dump() const { + auto &BC = Function->getBinaryContext(); if (Label) outs() << Label->getName() << ":\n"; BC.printInstructions(outs(), Instructions.begin(), Instructions.end(), Offset); outs() << "preds:"; diff --git a/bolt/BinaryBasicBlock.h b/bolt/BinaryBasicBlock.h index a96f510560e3..25e8c331baa5 100644 --- a/bolt/BinaryBasicBlock.h +++ b/bolt/BinaryBasicBlock.h @@ -14,11 +14,12 @@ #ifndef LLVM_TOOLS_LLVM_BOLT_BINARY_BASIC_BLOCK_H #define LLVM_TOOLS_LLVM_BOLT_BINARY_BASIC_BLOCK_H -#include "llvm/ADT/StringRef.h" #include "llvm/ADT/GraphTraits.h" +#include "llvm/ADT/StringRef.h" #include "llvm/MC/MCInst.h" #include "llvm/MC/MCSymbol.h" #include "llvm/Support/Debug.h" +#include "llvm/Support/ErrorOr.h" #include "llvm/Support/raw_ostream.h" #include #include @@ -28,7 +29,6 @@ namespace llvm { namespace bolt { class BinaryFunction; -class BinaryContext; /// The intention is to keep the structure similar to MachineBasicBlock as /// we might switch to it at some point. @@ -73,11 +73,13 @@ private: /// Number of times this basic block was executed. uint64_t ExecutionCount{COUNT_NO_PROFILE}; + static constexpr unsigned InvalidIndex = ~0u; + /// Index to BasicBlocks vector in BinaryFunction. - unsigned Index{~0u}; + unsigned Index{InvalidIndex}; /// Index in the current layout. - unsigned LayoutIndex{~0u}; + unsigned LayoutIndex{InvalidIndex}; /// Number of pseudo instructions in this block. uint32_t NumPseudos{0}; @@ -89,6 +91,10 @@ private: /// Indicates if the block could be outlined. bool CanOutline{true}; + /// Flag to indicate whether this block is valid or not. Invalid + /// blocks may contain out of date or incorrect information. + bool IsValid{true}; + private: BinaryBasicBlock() = delete; @@ -96,10 +102,9 @@ private: BinaryFunction *Function, MCSymbol *Label, uint64_t Offset = std::numeric_limits::max()) - : Function(Function), Label(Label), Offset(Offset) {} - - explicit BinaryBasicBlock(uint64_t Offset) - : Offset(Offset) {} + : Function(Function), Label(Label), Offset(Offset) { + assert(Function && "Function must be non-null"); + } // Exclusively managed by BinaryFunction. friend class BinaryFunction; @@ -464,6 +469,14 @@ public: ExecutionCount = Count; } + bool isValid() const { + return IsValid; + } + + void markValid(const bool Valid) { + IsValid = Valid; + } + bool isCold() const { return IsCold; } @@ -571,7 +584,7 @@ public: } /// A simple dump function for debugging. - void dump(BinaryContext &BC) const; + void dump() const; private: @@ -597,6 +610,7 @@ private: /// Get the index of this basic block. unsigned getIndex() const { + assert(isValid()); return Index; } @@ -609,6 +623,7 @@ private: /// making sure the indices are up to date, /// e.g. by calling BinaryFunction::updateLayoutIndices(); unsigned getLayoutIndex() const { + assert(isValid()); return LayoutIndex; } @@ -620,7 +635,6 @@ private: bool operator<(const BinaryBasicBlock &LHS, const BinaryBasicBlock &RHS); - } // namespace bolt diff --git a/bolt/BinaryFunction.cpp b/bolt/BinaryFunction.cpp index be4d5d8a79ff..e83b66cf772e 100644 --- a/bolt/BinaryFunction.cpp +++ b/bolt/BinaryFunction.cpp @@ -161,17 +161,22 @@ BinaryFunction::getBasicBlockContainingOffset(uint64_t Offset) { if (Offset > Size) return nullptr; - if (BasicBlocks.empty()) + if (BasicBlockOffsets.empty()) return nullptr; - // This is commented out because it makes BOLT too slow. - // assert(std::is_sorted(begin(), end())); - auto I = std::upper_bound(begin(), - end(), - BinaryBasicBlock(Offset)); - assert(I != begin() && "first basic block not at offset 0"); - - return &*--I; + /* + * This is commented out because it makes BOLT too slow. + * assert(std::is_sorted(BasicBlockOffsets.begin(), + * BasicBlockOffsets.end(), + * CompareBasicBlockOffsets()))); + */ + auto I = std::upper_bound(BasicBlockOffsets.begin(), + BasicBlockOffsets.end(), + BasicBlockOffset(Offset, nullptr), + CompareBasicBlockOffsets()); + assert(I != BasicBlockOffsets.begin() && "first basic block not at offset 0"); + --I; + return I->second; } size_t @@ -184,21 +189,78 @@ BinaryFunction::getBasicBlockOriginalSize(const BinaryBasicBlock *BB) const { } } -unsigned BinaryFunction::eraseDeadBBs( - std::map &ToPreserve) { +void BinaryFunction::markUnreachable() { + std::stack Stack; + BinaryBasicBlock *Entry = *layout_begin(); + + for (auto *BB : layout()) { + BB->markValid(false); + } + + Stack.push(Entry); + Entry->markValid(true); + + // Treat landing pads as roots. + for (auto *BB : BasicBlocks) { + if (BB->isLandingPad()) { + Stack.push(BB); + BB->markValid(true); + } + } + + // Determine reachable BBs from the entry point + while (!Stack.empty()) { + auto BB = Stack.top(); + Stack.pop(); + for (auto Succ : BB->successors()) { + if (Succ->isValid()) + continue; + Succ->markValid(true); + Stack.push(Succ); + } + } +} + +// Any unnecessary fallthrough jumps revealed after calling eraseInvalidBBs +// will be cleaned up by fixBranches(). +std::pair BinaryFunction::eraseInvalidBBs() { BasicBlockOrderType NewLayout; unsigned Count = 0; - assert(ToPreserve[BasicBlocksLayout.front()] == true && + uint64_t Bytes = 0; + assert(BasicBlocksLayout.front()->isValid() && "unable to remove an entry basic block"); for (auto I = BasicBlocksLayout.begin(), E = BasicBlocksLayout.end(); I != E; ++I) { - if (ToPreserve[*I]) + if ((*I)->isValid()) { NewLayout.push_back(*I); - else + } else { ++Count; + Bytes += BC.computeCodeSize((*I)->begin(), (*I)->end()); + } } BasicBlocksLayout = std::move(NewLayout); - return Count; + + BasicBlockListType NewBasicBlocks; + for (auto I = BasicBlocks.begin(), E = BasicBlocks.end(); I != E; ++I) { + if ((*I)->isValid()) { + NewBasicBlocks.push_back(*I); + } else { + DeletedBasicBlocks.push_back(*I); + } + } + BasicBlocks = std::move(NewBasicBlocks); + + assert(BasicBlocks.size() == BasicBlocksLayout.size()); + + // Update CFG state if needed + if (Count > 0) { + updateBBIndices(0); + recomputeLandingPads(0, BasicBlocks.size()); + BBCFIState = annotateCFIState(); + fixCFIState(); + } + + return std::make_pair(Count, Bytes); } void BinaryFunction::dump(std::string Annotation, @@ -1124,9 +1186,9 @@ bool BinaryFunction::buildCFG() { for (auto &Branch : TakenBranches) { DEBUG(dbgs() << "registering branch [0x" << Twine::utohexstr(Branch.first) << "] -> [0x" << Twine::utohexstr(Branch.second) << "]\n"); - BinaryBasicBlock *FromBB = getBasicBlockContainingOffset(Branch.first); + auto *FromBB = getBasicBlockContainingOffset(Branch.first); assert(FromBB && "cannot find BB containing FROM branch"); - BinaryBasicBlock *ToBB = getBasicBlockAtOffset(Branch.second); + auto *ToBB = getBasicBlockAtOffset(Branch.second); assert(ToBB && "cannot find BB containing TO branch"); if (BranchDataOrErr.getError()) { @@ -1175,7 +1237,7 @@ bool BinaryFunction::buildCFG() { DEBUG(dbgs() << "registering fallthrough [0x" << Twine::utohexstr(Branch.first) << "] -> [0x" << Twine::utohexstr(Branch.second) << "]\n"); - BinaryBasicBlock *FromBB = getBasicBlockContainingOffset(Branch.first); + auto *FromBB = getBasicBlockContainingOffset(Branch.first); assert(FromBB && "cannot find BB containing FROM branch"); // Try to find the destination basic block. If the jump instruction was // followed by a no-op then the destination offset recorded in FTBranches @@ -1183,7 +1245,7 @@ bool BinaryFunction::buildCFG() { // after the no-op due to ingoring no-ops when creating basic blocks. // So we have to skip any no-ops when trying to find the destination // basic block. - BinaryBasicBlock *ToBB = getBasicBlockAtOffset(Branch.second); + auto *ToBB = getBasicBlockAtOffset(Branch.second); if (ToBB == nullptr) { auto I = Instructions.find(Branch.second), E = Instructions.end(); while (ToBB == nullptr && I != E && MIA->isNoop(I->second)) { @@ -1290,7 +1352,7 @@ bool BinaryFunction::buildCFG() { inferFallThroughCounts(); // Update CFI information for each BB - annotateCFIState(); + BBCFIState = annotateCFIState(); // Convert conditional tail call branches to conditional branches that jump // to a tail call. @@ -1587,7 +1649,9 @@ void BinaryFunction::removeConditionalTailCalls() { std::vector> TailCallBBs; TailCallBBs.emplace_back(createBasicBlock(NextBB->getOffset(), TCLabel)); TailCallBBs[0]->addInstruction(TailCallInst); - insertBasicBlocks(BB, std::move(TailCallBBs), /* UpdateCFIState */ false); + insertBasicBlocks(BB, std::move(TailCallBBs), + /* UpdateLayout */ false, + /* UpdateCFIState */ false); TailCallBB = BasicBlocks[InsertIdx]; // Add the correct CFI state for the new block. @@ -1646,17 +1710,19 @@ uint64_t BinaryFunction::getFunctionScore() { return FunctionScore; } -void BinaryFunction::annotateCFIState() { +BinaryFunction::CFIStateVector +BinaryFunction::annotateCFIState(const MCInst *Stop) { assert(!BasicBlocks.empty() && "basic block list should not be empty"); uint32_t State = 0; uint32_t HighestState = 0; std::stack StateStack; + CFIStateVector CFIState; for (auto CI = BasicBlocks.begin(), CE = BasicBlocks.end(); CI != CE; ++CI) { BinaryBasicBlock *CurBB = *CI; // Annotate this BB entry - BBCFIState.emplace_back(State); + CFIState.emplace_back(State); // While building the CFG, we want to save the CFI state before a tail call // instruction, so that we can correctly remove condtional tail calls @@ -1671,6 +1737,10 @@ void BinaryFunction::annotateCFIState() { if (SaveState && Idx == TCI->second.Index) TCI->second.CFIStateBefore = State; ++Idx; + if (&Instr == Stop) { + CFIState.emplace_back(State); + return CFIState; + } continue; } ++HighestState; @@ -1684,13 +1754,19 @@ void BinaryFunction::annotateCFIState() { State = HighestState; } ++Idx; + if (&Instr == Stop) { + CFIState.emplace_back(State); + return CFIState; + } } } // Store the state after the last BB - BBCFIState.emplace_back(State); + CFIState.emplace_back(State); assert(StateStack.empty() && "Corrupt CFI stack"); + + return CFIState; } bool BinaryFunction::fixCFIState() { @@ -2593,7 +2669,8 @@ std::size_t BinaryFunction::hash() const { void BinaryFunction::insertBasicBlocks( BinaryBasicBlock *Start, std::vector> &&NewBBs, - bool UpdateCFIState) { + const bool UpdateLayout, + const bool UpdateCFIState) { const auto StartIndex = getIndex(Start); const auto NumNewBlocks = NewBBs.size(); @@ -2607,28 +2684,40 @@ void BinaryFunction::insertBasicBlocks( BasicBlocks[I++] = BB.release(); } - // Recompute indices and offsets for all basic blocks after Start. - uint64_t Offset = Start->getOffset(); - for (auto I = StartIndex; I < BasicBlocks.size(); ++I) { - auto *BB = BasicBlocks[I]; - BB->setOffset(Offset); - Offset += BC.computeCodeSize(BB->begin(), BB->end()); - BB->setIndex(I); - } - - if (UpdateCFIState) { - // Recompute CFI state for all BBs. - BBCFIState.clear(); - annotateCFIState(); - } + updateBBIndices(StartIndex); recomputeLandingPads(StartIndex, NumNewBlocks + 1); // Make sure the basic blocks are sorted properly. assert(std::is_sorted(begin(), end())); + + if (UpdateLayout) { + updateLayout(Start, NumNewBlocks); + } + + if (UpdateCFIState) { + updateCFIState(Start, NumNewBlocks); + } +} + +void BinaryFunction::updateBBIndices(const unsigned StartIndex) { + for (auto I = StartIndex; I < BasicBlocks.size(); ++I) { + BasicBlocks[I]->Index = I; + } +} + +void BinaryFunction::updateCFIState(BinaryBasicBlock *Start, + const unsigned NumNewBlocks) { + assert(TailCallTerminatedBlocks.empty()); + auto PartialCFIState = annotateCFIState(&(*Start->rbegin())); + const auto StartIndex = getIndex(Start); + BBCFIState.insert(BBCFIState.begin() + StartIndex + 1, + NumNewBlocks, + PartialCFIState.back()); + assert(BBCFIState.size() == BasicBlocks.size() + 1); + fixCFIState(); } -// TODO: Which of these methods is better? void BinaryFunction::updateLayout(BinaryBasicBlock* Start, const unsigned NumNewBlocks) { // Insert new blocks in the layout immediately after Start. @@ -2651,6 +2740,9 @@ BinaryFunction::~BinaryFunction() { for (auto BB : BasicBlocks) { delete BB; } + for (auto BB : DeletedBasicBlocks) { + delete BB; + } } void BinaryFunction::emitJumpTables(MCStreamer *Streamer) { diff --git a/bolt/BinaryFunction.h b/bolt/BinaryFunction.h index d4b8ac9a4ddd..cf22936cf7a3 100644 --- a/bolt/BinaryFunction.h +++ b/bolt/BinaryFunction.h @@ -320,6 +320,9 @@ private: return *this; } + /// Update the indices of all the basic blocks starting at StartIndex. + void updateBBIndices(const unsigned StartIndex); + /// Helper function that compares an instruction of this function to the /// given instruction of the given function. The functions should have /// identical CFG. @@ -510,15 +513,29 @@ private: // Blocks are kept sorted in the layout order. If we need to change the // layout (if BasicBlocksLayout stores a different order than BasicBlocks), // the terminating instructions need to be modified. - using BasicBlockListType = std::vector; + using BasicBlockListType = std::vector; BasicBlockListType BasicBlocks; + BasicBlockListType DeletedBasicBlocks; BasicBlockOrderType BasicBlocksLayout; + /// BasicBlockOffsets are used during CFG construction to map from code + /// offsets to BinaryBasicBlocks. Any modifications made to the CFG + /// after initial construction are not reflected in this data structure. + using BasicBlockOffset = std::pair; + struct CompareBasicBlockOffsets { + bool operator()(const BasicBlockOffset &A, + const BasicBlockOffset &B) const { + return A.first < B.first; + } + }; + std::vector BasicBlockOffsets; + // At each basic block entry we attach a CFI state to detect if reordering // corrupts the CFI state for a block. The CFI state is simply the index in // FrameInstructions for the CFI responsible for creating this state. // This vector is indexed by BB index. - std::vector BBCFIState; + using CFIStateVector = std::vector; + CFIStateVector BBCFIState; /// Symbol in the output. MCSymbol *OutputSymbol; @@ -882,14 +899,26 @@ public: auto BB = BasicBlocks.back(); BB->setIndex(BasicBlocks.size() - 1); - assert(CurrentState == State::CFG || std::is_sorted(begin(), end())); + if (CurrentState == State::Disassembled) { + BasicBlockOffsets.emplace_back(std::make_pair(Offset, BB)); + } + + assert(CurrentState == State::CFG || + (std::is_sorted(BasicBlockOffsets.begin(), + BasicBlockOffsets.end(), + CompareBasicBlockOffsets()) && + std::is_sorted(begin(), end()))); return BB; } + /// Mark all blocks that are unreachable from a root (entry point + /// or landing pad) as invalid. + void markUnreachable(); + /// Rebuilds BBs layout, ignoring dead BBs. Returns the number of removed - /// BBs. - unsigned eraseDeadBBs(std::map &ToPreserve); + /// BBs and the removed number of bytes of code. + std::pair eraseInvalidBBs(); /// Get the relative order between two basic blocks in the original /// layout. The result is > 0 if B occurs before A and < 0 if B @@ -916,7 +945,8 @@ public: void insertBasicBlocks( BinaryBasicBlock *Start, std::vector> &&NewBBs, - bool UpdateCFIState = true); + const bool UpdateLayout = true, + const bool UpdateCFIState = true); /// Update the basic block layout for this function. The BBs from /// [Start->Index, Start->Index + NumNewBlocks) are inserted into the @@ -935,6 +965,10 @@ public: } } + /// Recompute the CFI state for NumNewBlocks following Start after inserting + /// new blocks into the CFG. This must be called after updateLayout. + void updateCFIState(BinaryBasicBlock *Start, const unsigned NumNewBlocks); + /// Determine direction of the branch based on the current layout. /// Callee is responsible of updating basic block indices prior to using /// this function (e.g. by calling BinaryFunction::updateLayoutIndices()). @@ -1218,7 +1252,9 @@ public: /// fix this. /// The CFI state is simply the index in FrameInstructions for the /// MCCFIInstruction object responsible for this state. - void annotateCFIState(); + /// If Stop is not null, the annotation will exit early once the scan finishes + /// with the Stop instruction. + CFIStateVector annotateCFIState(const MCInst *Stop = nullptr); /// After reordering, this function checks the state of CFI and fixes it if it /// is corrupted. If it is unable to fix it, it returns false. diff --git a/bolt/BinaryPassManager.cpp b/bolt/BinaryPassManager.cpp index ed8bb11b5e28..646be20ea9b9 100644 --- a/bolt/BinaryPassManager.cpp +++ b/bolt/BinaryPassManager.cpp @@ -22,6 +22,7 @@ extern llvm::cl::opt DynoStatsAll; static cl::opt EliminateUnreachable("eliminate-unreachable", cl::desc("eliminate unreachable code"), + cl::init(true), cl::ZeroOrMore); static cl::opt @@ -140,8 +141,6 @@ cl::opt BinaryFunctionPassManager::AlwaysOn( cl::init(true), cl::ReallyHidden); -bool BinaryFunctionPassManager::NagUser = false; - void BinaryFunctionPassManager::runPasses() { for (const auto &OptPassPair : Passes) { if (!OptPassPair.first) @@ -196,10 +195,6 @@ void BinaryFunctionPassManager::runAllPasses( Manager.registerPass(llvm::make_unique(PrintInline), opts::InlineSmallFunctions); - Manager.registerPass( - llvm::make_unique(PrintUCE, Manager.NagUser), - opts::EliminateUnreachable); - Manager.registerPass( llvm::make_unique(PrintOptimizeBodyless), opts::OptimizeBodylessFunctions); @@ -208,17 +203,13 @@ void BinaryFunctionPassManager::runAllPasses( llvm::make_unique(PrintSimplifyROLoads), opts::SimplifyRODataLoads); - Manager.registerPass( - llvm::make_unique(PrintUCE, Manager.NagUser), - opts::EliminateUnreachable); - Manager.registerPass(llvm::make_unique(PrintReordered)); Manager.registerPass(llvm::make_unique(PrintPeepholes), opts::Peepholes); Manager.registerPass( - llvm::make_unique(PrintUCE, Manager.NagUser), + llvm::make_unique(PrintUCE), opts::EliminateUnreachable); // This pass syncs local branches with CFG. If any of the following diff --git a/bolt/BinaryPassManager.h b/bolt/BinaryPassManager.h index fb6d34e176a7..0360077a75b2 100644 --- a/bolt/BinaryPassManager.h +++ b/bolt/BinaryPassManager.h @@ -29,7 +29,6 @@ namespace bolt { class BinaryFunctionPassManager { private: static cl::opt AlwaysOn; - static bool NagUser; BinaryContext &BC; std::map &BFs; std::set &LargeFunctions; diff --git a/bolt/BinaryPasses.cpp b/bolt/BinaryPasses.cpp index db34d0eb1d1e..03715527aa31 100644 --- a/bolt/BinaryPasses.cpp +++ b/bolt/BinaryPasses.cpp @@ -780,42 +780,18 @@ void InlineSmallFunctions::runOnFunctions( } void EliminateUnreachableBlocks::runOnFunction(BinaryFunction& Function) { - // FIXME: this wouldn't work with C++ exceptions until we implement - // support for those as there will be "invisible" edges - // in the graph. if (Function.layout_size() > 0) { - if (NagUser) { - if (opts::Verbosity >= 1) { - errs() - << "BOLT-WARNING: Using -eliminate-unreachable is experimental and " - "unsafe for exceptions\n"; - } - NagUser = false; - } - - if (Function.hasEHRanges()) return; - - std::stack Stack; - std::map Reachable; - BinaryBasicBlock *Entry = *Function.layout_begin(); - Stack.push(Entry); - Reachable[Entry] = true; - // Determine reachable BBs from the entry point - while (!Stack.empty()) { - auto BB = Stack.top(); - Stack.pop(); - for (auto Succ : BB->successors()) { - if (Reachable[Succ]) - continue; - Reachable[Succ] = true; - Stack.push(Succ); - } - } - - auto Count = Function.eraseDeadBBs(Reachable); + unsigned Count; + uint64_t Bytes; + Function.markUnreachable(); + std::tie(Count, Bytes) = Function.eraseInvalidBBs(); + DeletedBlocks += Count; + DeletedBytes += Bytes; if (Count) { - DEBUG(dbgs() << "BOLT: Removed " << Count - << " dead basic block(s) in function " << Function << '\n'); + Modified.insert(&Function); + DEBUG(dbgs() << "BOLT-INFO: Removed " << Count + << " dead basic block(s) accounting for " << Bytes + << " bytes in function " << Function << '\n'); } } } @@ -831,6 +807,8 @@ void EliminateUnreachableBlocks::runOnFunctions( runOnFunction(Function); } } + outs() << "BOLT-INFO: UCE removed " << DeletedBlocks << " blocks and " + << DeletedBytes << " bytes of code.\n"; } bool ReorderBasicBlocks::shouldPrint(const BinaryFunction &BF) const { @@ -909,10 +887,8 @@ bool SimplifyConditionalTailCalls::fixTailCalls(BinaryContext &BC, auto &MIA = BC.MIA; uint64_t NumLocalCTCCandidates = 0; uint64_t NumLocalCTCs = 0; - std::map ToPreserve; - for (auto *BB : BF.layout()) { - ToPreserve[BB] = true; + for (auto *BB : BF.layout()) { // Locate BB with a single direct tail-call instruction. if (BB->getNumNonPseudos() != 1) continue; @@ -964,12 +940,11 @@ bool SimplifyConditionalTailCalls::fixTailCalls(BinaryContext &BC, } // Remove the block from CFG if all predecessors were removed. - if (BB->pred_size() == 0 && !BB->isLandingPad()) - ToPreserve[BB] = false; + BB->markValid(BB->pred_size() != 0 || BB->isLandingPad()); } // Clean-up unreachable tail-call blocks. - BF.eraseDeadBBs(ToPreserve); + BF.eraseInvalidBBs(); DEBUG(dbgs() << "BOLT: created " << NumLocalCTCs << " conditional tail calls from a total of " << NumLocalCTCCandidates @@ -1067,7 +1042,7 @@ void Peepholes::fixDoubleJumps(BinaryContext &BC, continue; const auto *SuccSym = BC.MIA->getTargetSymbol(*Inst); - auto *Succ = BB.getSuccessor(SuccSym); + auto *Succ = BB.getSuccessor(); if ((!Succ || &BB == Succ) && !IsTailCall) continue; diff --git a/bolt/BinaryPasses.h b/bolt/BinaryPasses.h index 660d967dcfdb..2ad88d501062 100644 --- a/bolt/BinaryPasses.h +++ b/bolt/BinaryPasses.h @@ -160,15 +160,20 @@ public: /// Detect and eliminate unreachable basic blocks. We could have those /// filled with nops and they are used for alignment. class EliminateUnreachableBlocks : public BinaryFunctionPass { - bool& NagUser; + std::unordered_set Modified; + unsigned DeletedBlocks{0}; + uint64_t DeletedBytes{0}; void runOnFunction(BinaryFunction& Function); public: - EliminateUnreachableBlocks(const cl::opt &PrintPass, bool &NagUser) - : BinaryFunctionPass(PrintPass), NagUser(NagUser) { } + EliminateUnreachableBlocks(const cl::opt &PrintPass) + : BinaryFunctionPass(PrintPass) { } const char *getName() const override { return "eliminate-unreachable"; } + bool shouldPrint(const BinaryFunction &BF) const override { + return BinaryFunctionPass::shouldPrint(BF) && Modified.count(&BF) > 0; + } void runOnFunctions(BinaryContext&, std::map &BFs, std::set &LargeFunctions) override; diff --git a/bolt/DebugData.cpp b/bolt/DebugData.cpp index 5938c793f05e..079a3be1e1aa 100644 --- a/bolt/DebugData.cpp +++ b/bolt/DebugData.cpp @@ -57,6 +57,7 @@ void BasicBlockOffsetRanges::addAddressRange(BinaryFunction &Function, std::min(BBAddress + Function.getBasicBlockOriginalSize(&BB), EndAddress); + assert(BB.isValid() && "Attempting to record debug info for a deleted BB."); AddressRanges.emplace_back( BBAddressRange{ &BB, @@ -70,6 +71,8 @@ std::vector BasicBlockOffsetRanges::getAbsoluteAddressRanges() const { std::vector AbsoluteRanges; for (const auto &BBAddressRange : AddressRanges) { + if (!BBAddressRange.BasicBlock->isValid()) + continue; auto BBOutputAddressRange = BBAddressRange.BasicBlock->getOutputAddressRange(); uint64_t NewRangeBegin = BBOutputAddressRange.first +