[PATCH] D146649: [BOLT][NFC] Simplify instrumentFunction

Amir Ayupov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 22 11:14:35 PDT 2023


Amir created this revision.
Amir added a reviewer: bolt.
Herald added a reviewer: rafauler.
Herald added subscribers: treapster, ayermolo.
Herald added a reviewer: maksfb.
Herald added a project: All.
Amir requested review of this revision.
Herald added subscribers: llvm-commits, yota9.
Herald added a project: LLVM.

Add `BinaryBasicBlock::insertInstructions` helper also used in D141234 <https://reviews.llvm.org/D141234>.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146649

Files:
  bolt/include/bolt/Core/BinaryBasicBlock.h
  bolt/lib/Passes/Instrumentation.cpp


Index: bolt/lib/Passes/Instrumentation.cpp
===================================================================
--- bolt/lib/Passes/Instrumentation.cpp
+++ bolt/lib/Passes/Instrumentation.cpp
@@ -183,11 +183,8 @@
 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,
@@ -364,8 +361,7 @@
     }
   }
 
-  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;
@@ -392,9 +388,8 @@
           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,
@@ -451,10 +446,9 @@
       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
@@ -480,24 +474,16 @@
   } // 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();
Index: bolt/include/bolt/Core/BinaryBasicBlock.h
===================================================================
--- bolt/include/bolt/Core/BinaryBasicBlock.h
+++ bolt/include/bolt/Core/BinaryBasicBlock.h
@@ -769,6 +769,11 @@
     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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D146649.507436.patch
Type: text/x-patch
Size: 4021 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230322/891c2dd1/attachment.bin>


More information about the llvm-commits mailing list