BOLT: Remove double jumps peephole.

Summary:
Replace jumps to other unconditional jumps with the final
destination, e.g.

  B0: ...
      jmp B1  (or jcc B1)

  B1: jmp B2

  ->

  B0: ...
      jmp B2  (or jcc B1)

This peephole removes 8928 double jumps from a test binary.

Note: after filtering out double jumps found in EH code and infinite
loops, the number of double jumps patched is 49 (24 for a clang
compiled test).  The 24 in the clang build are all from external
libraries which have probably been compiled with gcc.  This peephole
is still useful for cleaning up after ICP though.

(cherry picked from FBD3815420)

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();

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.
   ///

bolt/BinaryPassManager.cpp

@@ -207,6 +207,10 @@ void BinaryFunctionPassManager::runAllPasses(
   Manager.registerPass(llvm::make_unique<Peepholes>(PrintPeepholes),
                        opts::Peepholes);
 
+  Manager.registerPass(
+    llvm::make_unique<EliminateUnreachableBlocks>(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.

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<BinaryBasicBlock *> 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<uint64_t, BinaryFunction> &BFs,
                                std::set<uint64_t> &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(

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<bool> &PrintPass)
     : BinaryFunctionPass(PrintPass) { }