diff --git a/bolt/BinaryBasicBlock.cpp b/bolt/BinaryBasicBlock.cpp index 2ac456551773..1930369e1bf0 100644 --- a/bolt/BinaryBasicBlock.cpp +++ b/bolt/BinaryBasicBlock.cpp @@ -39,6 +39,15 @@ MCInst *BinaryBasicBlock::findFirstNonPseudoInstruction() { return nullptr; } +MCInst *BinaryBasicBlock::findLastNonPseudoInstruction() { + auto &BC = Function->getBinaryContext(); + for (auto Itr = Instructions.rbegin(); Itr != Instructions.rend(); ++Itr) { + if (!BC.MII->get(Itr->getOpcode()).isPseudo()) + return &*Itr; + } + return nullptr; +} + BinaryBasicBlock *BinaryBasicBlock::getSuccessor(const MCSymbol *Label) const { if (!Label && succ_size() == 1) return *succ_begin(); @@ -68,6 +77,24 @@ void BinaryBasicBlock::addSuccessor(BinaryBasicBlock *Succ, Succ->Predecessors.push_back(this); } +void BinaryBasicBlock::replaceSuccessor(BinaryBasicBlock *Succ, + BinaryBasicBlock *NewSucc, + uint64_t Count, + uint64_t MispredictedCount) { + auto I = succ_begin(); + auto BI = BranchInfo.begin(); + for (; I != succ_end(); ++I) { + assert(BI != BranchInfo.end() && "missing BranchInfo entry"); + if (*I == Succ) + break; + ++BI; + } + assert(I != succ_end() && "no such successor!"); + + *I = NewSucc; + *BI = BinaryBranchInfo{Count, MispredictedCount}; +} + void BinaryBasicBlock::removeSuccessor(BinaryBasicBlock *Succ) { Succ->removePredecessor(this); auto I = succ_begin(); @@ -117,12 +144,20 @@ bool BinaryBasicBlock::swapConditionalSuccessors() { } void BinaryBasicBlock::addBranchInstruction(const BinaryBasicBlock *Successor) { + assert(isSuccessor(Successor)); auto &BC = Function->getBinaryContext(); MCInst NewInst; BC.MIA->createUncondBranch(NewInst, Successor->getLabel(), BC.Ctx.get()); Instructions.emplace_back(std::move(NewInst)); } +void BinaryBasicBlock::addTailCallInstruction(const MCSymbol *Target) { + auto &BC = Function->getBinaryContext(); + MCInst NewInst; + BC.MIA->createTailCall(NewInst, Target, BC.Ctx.get()); + Instructions.emplace_back(std::move(NewInst)); +} + uint32_t BinaryBasicBlock::getNumPseudos() const { #ifndef NDEBUG auto &BC = Function->getBinaryContext(); diff --git a/bolt/BinaryBasicBlock.h b/bolt/BinaryBasicBlock.h index 78c8313cb84a..fba243966f19 100644 --- a/bolt/BinaryBasicBlock.h +++ b/bolt/BinaryBasicBlock.h @@ -308,6 +308,15 @@ public: return Successors[Condition == true ? 0 : 1]; } + /// Find the fallthrough successor for a block, or nullptr if there is + /// none. + const BinaryBasicBlock* getFallthrough() const { + if (succ_size() == 2) + return getConditionalSuccessor(false); + else + return getSuccessor(); + } + const BinaryBranchInfo &getBranchInfo(bool Condition) const { assert(BranchInfo.size() == 2 && "could only be called for blocks with 2 successors"); @@ -324,6 +333,10 @@ public: /// basic block to the end of this basic block. void addBranchInstruction(const BinaryBasicBlock *Successor); + /// Add an instruction with tail call control transfer to \p Target + /// to the end of this basic block. + void addTailCallInstruction(const MCSymbol *Target); + /// Get landing pad with given label. Returns nullptr if no such /// landing pad is found. BinaryBasicBlock *getLandingPad(const MCSymbol *Label) const; @@ -333,8 +346,6 @@ public: return Label->getName(); } - MCInst *findFirstNonPseudoInstruction(); - /// Add instruction at the end of this basic block. /// Returns the index of the instruction in the Instructions vector of the BB. uint32_t addInstruction(MCInst &&Inst) { @@ -372,6 +383,14 @@ public: return size() - getNumPseudos(); } + /// Return a pointer to the first non-pseudo instruction in this basic + /// block. Returns nullptr if none exists. + MCInst *findFirstNonPseudoInstruction(); + + /// Return a pointer to the last non-pseudo instruction in this basic + /// block. Returns nullptr if none exists. + MCInst *findLastNonPseudoInstruction(); + /// Set minimum alignment for the basic block. void setAlignment(uint64_t Align) { Alignment = Align; @@ -419,6 +438,13 @@ public: } } + /// Replace Succ with NewSucc. This routine is helpful for preserving + /// the order of conditional successors when editing the CFG. + void replaceSuccessor(BinaryBasicBlock *Succ, + BinaryBasicBlock *NewSucc, + uint64_t Count = 0, + uint64_t MispredictedCount = 0); + /// Adds block to landing pad list. void addLandingPad(BinaryBasicBlock *LPBlock); @@ -434,6 +460,12 @@ public: } } + /// Test if BB is a successor of this block. + bool isSuccessor(const BinaryBasicBlock *BB) const { + auto Itr = std::find(Successors.begin(), Successors.end(), BB); + return Itr != Successors.end(); + } + /// Return the information about the number of times this basic block was /// executed. /// diff --git a/bolt/BinaryPassManager.cpp b/bolt/BinaryPassManager.cpp index c66d7b2af158..f653c26140a3 100644 --- a/bolt/BinaryPassManager.cpp +++ b/bolt/BinaryPassManager.cpp @@ -207,6 +207,10 @@ void BinaryFunctionPassManager::runAllPasses( Manager.registerPass(llvm::make_unique(PrintPeepholes), opts::Peepholes); + Manager.registerPass( + llvm::make_unique(PrintUCE, Manager.NagUser), + opts::EliminateUnreachable); + // This pass syncs local branches with CFG. If any of the following // passes breaks the sync - they either need to re-run the pass or // fix branches consistency internally. diff --git a/bolt/BinaryPasses.cpp b/bolt/BinaryPasses.cpp index 6c97c41cd04d..d67bda059d8d 100644 --- a/bolt/BinaryPasses.cpp +++ b/bolt/BinaryPasses.cpp @@ -956,6 +956,81 @@ void Peepholes::shortenInstructions(BinaryContext &BC, } } +// This peephole fixes jump instructions that jump to another basic +// block with a single jump instruction, e.g. +// +// B0: ... +// jmp B1 (or jcc B1) +// +// B1: jmp B2 +// +// -> +// +// B0: ... +// jmp B2 (or jcc B2) +// +void Peepholes::fixDoubleJumps(BinaryContext &BC, + BinaryFunction &Function) { + for (auto &BB : Function) { + auto checkAndPatch = [&](BinaryBasicBlock *Pred, + BinaryBasicBlock *Succ, + const MCSymbol *SuccSym) { + // Ignore infinite loop jumps or fallthrough tail jumps. + if (Pred == Succ || Succ == &BB) + return; + + if (Succ) { + Pred->replaceSuccessor(&BB, Succ, BinaryBasicBlock::COUNT_NO_PROFILE); + } else { + // Succ will be null in the tail call case. In this case we + // need to explicitly add a tail call instruction. + auto *Branch = Pred->findLastNonPseudoInstruction(); + if (Branch && BC.MIA->isUnconditionalBranch(*Branch)) { + Pred->removeSuccessor(&BB); + Pred->eraseInstruction(Branch); + Pred->addTailCallInstruction(SuccSym); + } else { + return; + } + } + + ++NumDoubleJumps; + DEBUG(dbgs() << "Removed double jump in " << Function << " from " + << Pred->getName() << " -> " << BB.getName() << " to " + << Pred->getName() << " -> " << SuccSym->getName() + << (!Succ ? " (tail)\n" : "\n")); + }; + + if (BB.getNumNonPseudos() != 1 || BB.isLandingPad()) + continue; + + auto *Inst = BB.findFirstNonPseudoInstruction(); + const bool IsTailCall = BC.MIA->isTailCall(*Inst); + + if (!BC.MIA->isUnconditionalBranch(*Inst) && !IsTailCall) + continue; + + const auto *SuccSym = BC.MIA->getTargetSymbol(*Inst); + auto *Succ = BB.getSuccessor(SuccSym); + + if ((!Succ || &BB == Succ) && !IsTailCall) + continue; + + std::vector Preds{BB.pred_begin(), BB.pred_end()}; + + for (auto *Pred : Preds) { + if (Pred->isLandingPad()) + continue; + + if (Pred->getSuccessor() == &BB || + (Pred->getConditionalSuccessor(true) == &BB && !IsTailCall) || + Pred->getConditionalSuccessor(false) == &BB) { + checkAndPatch(Pred, Succ, SuccSym); + } + } + } +} + void Peepholes::runOnFunctions(BinaryContext &BC, std::map &BFs, std::set &LargeFunctions) { @@ -963,8 +1038,10 @@ void Peepholes::runOnFunctions(BinaryContext &BC, auto &Function = It.second; if (shouldOptimize(Function)) { shortenInstructions(BC, Function); + fixDoubleJumps(BC, Function); } } + outs() << "BOLT-INFO: " << NumDoubleJumps << " double jumps patched.\n"; } bool SimplifyRODataLoads::simplifyRODataLoads( diff --git a/bolt/BinaryPasses.h b/bolt/BinaryPasses.h index 9bc2987a3921..41602213bb99 100644 --- a/bolt/BinaryPasses.h +++ b/bolt/BinaryPasses.h @@ -256,7 +256,9 @@ class SimplifyConditionalTailCalls : public BinaryFunctionPass { /// Perform simple peephole optimizations. class Peepholes : public BinaryFunctionPass { + uint64_t NumDoubleJumps{0}; void shortenInstructions(BinaryContext &BC, BinaryFunction &Function); + void fixDoubleJumps(BinaryContext &BC, BinaryFunction &Function); public: explicit Peepholes(const cl::opt &PrintPass) : BinaryFunctionPass(PrintPass) { }