[BOLT][NFC] Simplify instrumentFunction

[Amir]

Summary

Add BinaryBasicBlock::insertInstructions helper also used in D141234.

bolt/include/bolt/Core/BinaryBasicBlock.h

iterator insertInstruction(iterator At, MCInst &&NewInst) {
     return Instructions.emplace(At, std::move(NewInst));
   }
 
   iterator insertInstruction(iterator At, MCInst &NewInst) {
     adjustNumPseudos(NewInst, 1);
     return Instructions.insert(At, NewInst);
   }
 
+  iterator insertInstructions(iterator At, InstructionListType &NewInsts) {
+    adjustNumPseudos(NewInsts.begin(), NewInsts.end(), 1);
+    return Instructions.insert(At, NewInsts.begin(), NewInsts.end());
+  }
+
   /// Helper to retrieve any terminators in \p BB before \p Pos. This is used
   /// to skip CFI instructions and to retrieve the first terminator instruction
   /// in basic blocks with two terminators (conditional jump and unconditional
   /// jump).
   MCInst *getTerminatorBefore(MCInst *Pos);
 
   /// Used to identify whether an instruction is before a terminator and whether
   /// moving it to the end of the BB would render it dead code.

bolt/lib/Passes/Instrumentation.cpp

Instrumentation::createInstrumentationSnippet(BinaryContext &BC, bool IsLeaf) {
   Summary->Counters.emplace_back(Label);
   return BC.MIB->createInstrIncMemory(Label, BC.Ctx.get(), IsLeaf);
 }
 
 // Helper instruction sequence insertion function
 static BinaryBasicBlock::iterator
 insertInstructions(InstructionListType &Instrs, BinaryBasicBlock &BB,
                    BinaryBasicBlock::iterator Iter) {
-  for (MCInst &NewInst : Instrs) {
-    Iter = BB.insertInstruction(Iter, NewInst);
-    ++Iter;
-  }
-  return Iter;
+  Iter = BB.insertInstructions(Iter, Instrs);
+  return Iter + Instrs.size();
 }
 
 void Instrumentation::instrumentLeafNode(BinaryBasicBlock &BB,
                                          BinaryBasicBlock::iterator Iter,
                                          bool IsLeaf,
                                          FunctionDescription &FuncDesc,
                                          uint32_t Node) {
   createLeafNodeDescription(FuncDesc, Node);
for (const MCInst &Inst : BB) {
         if (BC.MIB->isInvoke(Inst))
           InvokeBlocks.insert(&BB);
         if (!BC.MIB->isTailCall(Inst))
           IsLeafFunction = false;
       }
     }
   }
 
-  for (auto BBI = Function.begin(), BBE = Function.end(); BBI != BBE; ++BBI) {
-    BinaryBasicBlock &BB = *BBI;
+  for (BinaryBasicBlock &BB : Function) {
     bool HasUnconditionalBranch = false;
     bool HasJumpTable = false;
     bool IsInvokeBlock = InvokeBlocks.count(&BB) > 0;
 
     for (auto I = BB.begin(); I != BB.end(); ++I) {
       const MCInst &Inst = *I;
       if (!BC.MIB->getOffset(Inst))
         continue;
for (auto I = BB.begin(); I != BB.end(); ++I) {
       const uint32_t FromOffset = *BC.MIB->getOffset(Inst);
       const MCSymbol *Target = BC.MIB->getTargetSymbol(Inst);
       BinaryBasicBlock *TargetBB = Function.getBasicBlockForLabel(Target);
       uint32_t ToOffset = TargetBB ? TargetBB->getInputOffset() : 0;
       BinaryFunction *TargetFunc =
           TargetBB ? &Function : BC.getFunctionForSymbol(Target);
       if (TargetFunc && BC.MIB->isCall(Inst)) {
         if (opts::InstrumentCalls) {
-          const BinaryBasicBlock *ForeignBB =
-              TargetFunc->getBasicBlockForLabel(Target);
-          if (ForeignBB)
+          if (const BinaryBasicBlock *ForeignBB =
+                  TargetFunc->getBasicBlockForLabel(Target))
             ToOffset = ForeignBB->getInputOffset();
           instrumentOneTarget(SplitWorklist, SplitInstrs, I, Function, BB,
                               FromOffset, *TargetFunc, TargetBB, ToOffset,
                               IsLeafFunction, IsInvokeBlock, FuncDesc,
                               BBToID[&BB]);
         }
         continue;
       }
for (BinaryBasicBlock &BB : Function) {
     } // End of instructions loop
 
     // Instrument fallthroughs (when the direct jump instruction is missing)
     if (!HasUnconditionalBranch && !HasJumpTable && BB.succ_size() > 0 &&
         BB.size() > 0) {
       BinaryBasicBlock *FTBB = BB.getFallthrough();
       assert(FTBB && "expected valid fall-through basic block");
       auto I = BB.begin();
-      auto LastInstr = BB.end();
-      --LastInstr;
-      while (LastInstr != I && BC.MIB->isPseudo(*LastInstr))
-        --LastInstr;
+      const MCInst *LastInstr = BB.getLastNonPseudoInstr();
+      if (LastInstr == nullptr)
+        LastInstr = &BB.front();
       uint32_t FromOffset = 0;
       // The last instruction in the BB should have an annotation, except
       // if it was branching to the end of the function as a result of
       // __builtin_unreachable(), in which case it was deleted by fixBranches.
       // Ignore this case. FIXME: force fixBranches() to preserve the offset.
       if (!BC.MIB->getOffset(*LastInstr))
         continue;
       FromOffset = *BC.MIB->getOffset(*LastInstr);
if (!HasUnconditionalBranch && !HasJumpTable && BB.succ_size() > 0 &&
       instrumentOneTarget(SplitWorklist, SplitInstrs, I, Function, BB,
                           FromOffset, Function, FTBB, FTBB->getInputOffset(),
                           IsLeafFunction, IsInvokeBlock, FuncDesc, BBToID[&BB],
                           BBToID[FTBB]);
     }
   } // End of BBs loop
 
   // Instrument spanning tree leaves
-  if (!opts::ConservativeInstrumentation) {
-    for (auto BBI = Function.begin(), BBE = Function.end(); BBI != BBE; ++BBI) {
-      BinaryBasicBlock &BB = *BBI;
+  if (!opts::ConservativeInstrumentation)
+    for (BinaryBasicBlock &BB : Function)
       if (STOutSet[&BB].size() == 0)
         instrumentLeafNode(BB, BB.begin(), IsLeafFunction, *FuncDesc,
                            BBToID[&BB]);
-    }
-  }
 
   // Consume list of critical edges: split them and add instrumentation to the
   // newly created BBs
-  auto Iter = SplitInstrs.begin();
-  for (std::pair<BinaryBasicBlock *, BinaryBasicBlock *> &BBPair :
-       SplitWorklist) {
-    BinaryBasicBlock *NewBB = Function.splitEdge(BBPair.first, BBPair.second);
-    NewBB->addInstructions(Iter->begin(), Iter->end());
-    ++Iter;
-  }
+  for (const auto &[Edge, Insts] : llvm::zip(SplitWorklist, SplitInstrs))
+    Function.splitEdge(Edge.first, Edge.second)->addInstructions(Insts);
 
   // Unused now
   FuncDesc->EdgesSet.clear();
 }
 
 void Instrumentation::runOnFunctions(BinaryContext &BC) {
   if (!BC.isX86())
     return;