[BOLT] Replace uses of layout with basic block list

[FPar]

Summary

As we are moving towards support for multiple fragments, loops that
iterate over all basic blocks of a function, but do not depend on the
order of basic blocks in the final layout, should iterate over binary
functions directly, rather than the layout.

Eventually, all loops using the layout list should either iterate over
the function, or be aware of multiple layouts. This patch replaces
references to binary function's block layout with the binary function
itself where only little code changes are necessary.

[FPar]

Remove some substitutions

[maksfb]

Maybe instead of for (const BinaryBasicBlock *const BB : BasicBlocks) we can have for (const BinaryBasicBlock &BB : blocks())?

Other than that, looks good to me. Thanks.

bolt/lib/Core/BinaryFunction.cpp

3521

nit: remove curly braces.

3525

Likewise.

[FPar]

Replace iterations over blocks with *this

[FPar]

Use blocks() instead of *this to iterate

[FPar]

In D129585#3650403, @maksfb wrote:

Maybe instead of for (const BinaryBasicBlock *const BB : BasicBlocks) we can have for (const BinaryBasicBlock &BB : blocks())?

Done.

[maksfb]

LGTM. Please address a couple if nits before commit.

bolt/lib/Core/BinaryFunction.cpp

3514

nit:

3518

nit:

bolt/include/bolt/Core/BinaryFunction.h

public:
   uint64_t getSize() const { return Size; }
 
   /// Return the maximum size the body of the function could have.
   uint64_t getMaxSize() const { return MaxSize; }
 
   /// Return the number of emitted instructions for this function.
   uint32_t getNumNonPseudos() const {
     uint32_t N = 0;
-    for (BinaryBasicBlock *const &BB : layout())
-      N += BB->getNumNonPseudos();
+    for (const BinaryBasicBlock &BB : blocks())
+      N += BB.getNumNonPseudos();
     return N;
   }
 
   /// Return MC symbol associated with the function.
   /// All references to the function should use this symbol.
   MCSymbol *getSymbol() { return Symbols[0]; }
 
   /// Return MC symbol associated with the function (const version).

   /// This is a very rough estimate, as with C++ exceptions there are
   /// blocks we don't move, and it makes no attempt at estimating the size
   /// of the added/removed branch instructions.
   /// Note that this size is optimistic and the actual size may increase
   /// after relaxation.
   size_t estimateHotSize(const bool UseSplitSize = true) const {
     size_t Estimate = 0;
     if (UseSplitSize && isSplit()) {
-      for (const BinaryBasicBlock *BB : BasicBlocksLayout)
-        if (!BB->isCold())
-          Estimate += BC.computeCodeSize(BB->begin(), BB->end());
+      for (const BinaryBasicBlock &BB : blocks())
+        if (!BB.isCold())
+          Estimate += BC.computeCodeSize(BB.begin(), BB.end());
     } else {
-      for (const BinaryBasicBlock *BB : BasicBlocksLayout)
-        if (BB->getKnownExecutionCount() != 0)
-          Estimate += BC.computeCodeSize(BB->begin(), BB->end());
+      for (const BinaryBasicBlock &BB : blocks())
+        if (BB.getKnownExecutionCount() != 0)
+          Estimate += BC.computeCodeSize(BB.begin(), BB.end());
     }
     return Estimate;
   }
 
   size_t estimateColdSize() const {
     if (!isSplit())
       return estimateSize();
     size_t Estimate = 0;
-    for (const BinaryBasicBlock *BB : BasicBlocksLayout)
-      if (BB->isCold())
-        Estimate += BC.computeCodeSize(BB->begin(), BB->end());
+    for (const BinaryBasicBlock &BB : blocks())
+      if (BB.isCold())
+        Estimate += BC.computeCodeSize(BB.begin(), BB.end());
     return Estimate;
   }
 
   size_t estimateSize() const {
     size_t Estimate = 0;
-    for (const BinaryBasicBlock *BB : BasicBlocksLayout)
-      Estimate += BC.computeCodeSize(BB->begin(), BB->end());
+    for (const BinaryBasicBlock &BB : blocks())
+      Estimate += BC.computeCodeSize(BB.begin(), BB.end());
     return Estimate;
   }
 
   /// Return output address ranges for a function.
   DebugAddressRangesVector getOutputAddressRanges() const;
 
   /// Given an address corresponding to an instruction in the input binary,
   /// return an address of this instruction in output binary.

bolt/lib/Core/BinaryFunction.cpp

BinaryFunction::getBasicBlockContainingOffset(uint64_t Offset) {
   --I;
   BinaryBasicBlock *BB = I->second;
   return (Offset < BB->getOffset() + BB->getOriginalSize()) ? BB : nullptr;
 }
 
 void BinaryFunction::markUnreachableBlocks() {
   std::stack<BinaryBasicBlock *> Stack;
 
-  for (BinaryBasicBlock *BB : layout())
-    BB->markValid(false);
+  for (BinaryBasicBlock &BB : blocks())
+    BB.markValid(false);
 
   // Add all entries and landing pads as roots.
   for (BinaryBasicBlock *BB : BasicBlocks) {
     if (isEntryPoint(*BB) || BB->isLandingPad()) {
       Stack.push(BB);
       BB->markValid(true);
       continue;
     }
for (uint64_t PossibleDestination : ExternallyReferencedOffsets)
         BB.addSuccessor(SuccBB);
   };
 
   uint64_t NumIndirectJumps = 0;
   MCInst *LastIndirectJump = nullptr;
   BinaryBasicBlock *LastIndirectJumpBB = nullptr;
   uint64_t LastJT = 0;
   uint16_t LastJTIndexReg = BC.MIB->getNoRegister();
-  for (BinaryBasicBlock *BB : layout()) {
-    for (MCInst &Instr : *BB) {
+  for (BinaryBasicBlock &BB : blocks()) {
+    for (MCInst &Instr : BB) {
       if (!BC.MIB->isIndirectBranch(Instr))
         continue;
 
       // If there's an indirect branch in a single-block function -
       // it must be a tail call.
-      if (layout_size() == 1) {
+      if (BasicBlocks.size() == 1) {
         BC.MIB->convertJmpToTailCall(Instr);
         return true;
       }
 
       ++NumIndirectJumps;
 
       if (opts::StrictMode && !hasInternalReference()) {
         BC.MIB->convertJmpToTailCall(Instr);
         break;
       }
 
       // Validate the tail call or jump table assumptions now that we know
       // basic block boundaries.
       if (BC.MIB->isTailCall(Instr) || BC.MIB->getJumpTable(Instr)) {
         const unsigned PtrSize = BC.AsmInfo->getCodePointerSize();
         MCInst *MemLocInstr;
         unsigned BaseRegNum, IndexRegNum;
         int64_t DispValue;
         const MCExpr *DispExpr;
         MCInst *PCRelBaseInstr;
         IndirectBranchType Type = BC.MIB->analyzeIndirectBranch(
-            Instr, BB->begin(), BB->end(), PtrSize, MemLocInstr, BaseRegNum,
+            Instr, BB.begin(), BB.end(), PtrSize, MemLocInstr, BaseRegNum,
             IndexRegNum, DispValue, DispExpr, PCRelBaseInstr);
         if (Type != IndirectBranchType::UNKNOWN || MemLocInstr != nullptr)
           continue;
 
         if (!opts::StrictMode)
           return false;
 
         if (BC.MIB->isTailCall(Instr)) {
           BC.MIB->convertTailCallToJmp(Instr);
         } else {
           LastIndirectJump = &Instr;
-          LastIndirectJumpBB = BB;
+          LastIndirectJumpBB = &BB;
           LastJT = BC.MIB->getJumpTable(Instr);
           LastJTIndexReg = BC.MIB->getJumpTableIndexReg(Instr);
           BC.MIB->unsetJumpTable(Instr);
 
           JumpTable *JT = BC.getJumpTableContainingAddress(LastJT);
           if (JT->Type == JumpTable::JTT_NORMAL) {
             // Invalidating the jump table may also invalidate other jump table
             // boundaries. Until we have/need a support for this, mark the
             // function as non-simple.
             LLVM_DEBUG(dbgs() << "BOLT-DEBUG: rejected jump table reference"
                               << JT->getName() << " in " << *this << '\n');
             return false;
           }
         }
 
-        addUnknownControlFlow(*BB);
+        addUnknownControlFlow(BB);
         continue;
       }
 
       // If this block contains an epilogue code and has an indirect branch,
       // then most likely it's a tail call. Otherwise, we cannot tell for sure
       // what it is and conservatively reject the function's CFG.
       bool IsEpilogue = false;
-      for (const MCInst &Instr : *BB) {
+      for (const MCInst &Instr : BB) {
         if (BC.MIB->isLeave(Instr) || BC.MIB->isPop(Instr)) {
           IsEpilogue = true;
           break;
         }
       }
       if (IsEpilogue) {
         BC.MIB->convertJmpToTailCall(Instr);
-        BB->removeAllSuccessors();
+        BB.removeAllSuccessors();
         continue;
       }
 
       if (opts::Verbosity >= 2) {
         outs() << "BOLT-INFO: rejected potential indirect tail call in "
-               << "function " << *this << " in basic block " << BB->getName()
+               << "function " << *this << " in basic block " << BB.getName()
                << ".\n";
-        LLVM_DEBUG(BC.printInstructions(dbgs(), BB->begin(), BB->end(),
-                                        BB->getOffset(), this, true));
+        LLVM_DEBUG(BC.printInstructions(dbgs(), BB.begin(), BB.end(),
+                                        BB.getOffset(), this, true));
       }
 
       if (!opts::StrictMode)
         return false;
 
-      addUnknownControlFlow(*BB);
+      addUnknownControlFlow(BB);
     }
   }
 
   if (HasInternalLabelReference)
     return false;
 
   // If there's only one jump table, and one indirect jump, and no other
   // references, then we should be able to derive the jump table even if we
void BinaryFunction::postProcessCFG() {
 
   // The final cleanup of intermediate structures.
   clearList(IgnoredBranches);
 
   // Remove "Offset" annotations, unless we need an address-translation table
   // later. This has no cost, since annotations are allocated by a bumpptr
   // allocator and won't be released anyway until late in the pipeline.
   if (!requiresAddressTranslation() && !opts::Instrument) {
-    for (BinaryBasicBlock *BB : layout())
-      for (MCInst &Inst : *BB)
+    for (BinaryBasicBlock &BB : blocks())
+      for (MCInst &Inst : BB)
         BC.MIB->clearOffset(Inst);
   }
 
   assert((!isSimple() || validateCFG()) &&
          "invalid CFG detected after post-processing");
 }
 
 void BinaryFunction::calculateMacroOpFusionStats() {
   if (!getBinaryContext().isX86())
     return;
-  for (BinaryBasicBlock *BB : layout()) {
-    auto II = BB->getMacroOpFusionPair();
-    if (II == BB->end())
+  for (const BinaryBasicBlock &BB : blocks()) {
+    auto II = BB.getMacroOpFusionPair();
+    if (II == BB.end())
       continue;
 
     // Check offset of the second instruction.
     // FIXME: arch-specific.
     const uint32_t Offset = BC.MIB->getOffsetWithDefault(*std::next(II), 0);
     if (!Offset || (getAddress() + Offset) % 64)
       continue;
 
     LLVM_DEBUG(dbgs() << "\nmissed macro-op fusion at address 0x"
                       << Twine::utohexstr(getAddress() + Offset)
                       << " in function " << *this << "; executed "
-                      << BB->getKnownExecutionCount() << " times.\n");
+                      << BB.getKnownExecutionCount() << " times.\n");
     ++BC.MissedMacroFusionPairs;
-    BC.MissedMacroFusionExecCount += BB->getKnownExecutionCount();
+    BC.MissedMacroFusionExecCount += BB.getKnownExecutionCount();
   }
 }
 
 void BinaryFunction::removeTagsFromProfile() {
   for (BinaryBasicBlock *BB : BasicBlocks) {
     if (BB->ExecutionCount == BinaryBasicBlock::COUNT_NO_PROFILE)
       BB->ExecutionCount = 0;
     for (BinaryBasicBlock::BinaryBranchInfo &BI : BB->branch_info()) {
if (FunctionScore != -1)
     return FunctionScore;
 
   if (!isSimple() || !hasValidProfile()) {
     FunctionScore = 0;
     return FunctionScore;
   }
 
   uint64_t TotalScore = 0ULL;
-  for (BinaryBasicBlock *BB : layout()) {
-    uint64_t BBExecCount = BB->getExecutionCount();
+  for (const BinaryBasicBlock &BB : blocks()) {
+    uint64_t BBExecCount = BB.getExecutionCount();
     if (BBExecCount == BinaryBasicBlock::COUNT_NO_PROFILE)
       continue;
-    TotalScore += BBExecCount * BB->getNumNonPseudos();
+    TotalScore += BBExecCount * BB.getNumNonPseudos();
   }
   FunctionScore = TotalScore;
   return FunctionScore;
 }
 
 void BinaryFunction::annotateCFIState() {
   assert(CurrentState == State::Disassembled && "unexpected function state");
   assert(!BasicBlocks.empty() && "basic block list should not be empty");
if (BB->getCFIState() < State) {
         return false;
     }
 
     State = CFIStateAtExit;
     LLVM_DEBUG(dbgs() << Sep << State; Sep = ", ");
   }
   LLVM_DEBUG(dbgs() << "\n");
 
-  for (BinaryBasicBlock *BB : BasicBlocksLayout) {
-    for (auto II = BB->begin(); II != BB->end();) {
+  for (BinaryBasicBlock &BB : blocks()) {
+    for (auto II = BB.begin(); II != BB.end();) {
       const MCCFIInstruction *CFI = getCFIFor(*II);
       if (CFI && (CFI->getOperation() == MCCFIInstruction::OpRememberState ||
                   CFI->getOperation() == MCCFIInstruction::OpRestoreState)) {
-        II = BB->eraseInstruction(II);
+        II = BB.eraseInstruction(II);
       } else {
         ++II;
       }
     }
   }
 
   return true;
 }
 
 bool BinaryFunction::requiresAddressTranslation() const {
   return opts::EnableBAT || hasSDTMarker() || hasPseudoProbe();
 }
 
 uint64_t BinaryFunction::getInstructionCount() const {
   uint64_t Count = 0;
-  for (BinaryBasicBlock *const &Block : BasicBlocksLayout)
-    Count += Block->getNumNonPseudos();
+  for (const BinaryBasicBlock &BB : blocks())
+    Count += BB.getNumNonPseudos();
   return Count;
 }
 
 bool BinaryFunction::hasLayoutChanged() const { return ModifiedLayout; }
 
 uint64_t BinaryFunction::getEditDistance() const {
   return ComputeEditDistance<BinaryBasicBlock *>(BasicBlocksPreviousLayout,
                                                  BasicBlocksLayout);
for (auto II = BB->begin(); II != BB->end();) {
       ++II;
     }
   }
 }
 
 void BinaryFunction::postProcessBranches() {
   if (!isSimple())
     return;
-  for (BinaryBasicBlock *BB : BasicBlocksLayout) {
-    auto LastInstrRI = BB->getLastNonPseudo();
-    if (BB->succ_size() == 1) {
-      if (LastInstrRI != BB->rend() &&
+  for (BinaryBasicBlock &BB : blocks()) {
+    auto LastInstrRI = BB.getLastNonPseudo();
+    if (BB.succ_size() == 1) {
+      if (LastInstrRI != BB.rend() &&
           BC.MIB->isConditionalBranch(*LastInstrRI)) {
         // __builtin_unreachable() could create a conditional branch that
         // falls-through into the next function - hence the block will have only
         // one valid successor. Such behaviour is undefined and thus we remove
         // the conditional branch while leaving a valid successor.
-        BB->eraseInstruction(std::prev(LastInstrRI.base()));
+        BB.eraseInstruction(std::prev(LastInstrRI.base()));
         LLVM_DEBUG(dbgs() << "BOLT-DEBUG: erasing conditional branch in "
-                          << BB->getName() << " in function " << *this << '\n');
+                          << BB.getName() << " in function " << *this << '\n');
       }
-    } else if (BB->succ_size() == 0) {
+    } else if (BB.succ_size() == 0) {
       // Ignore unreachable basic blocks.
-      if (BB->pred_size() == 0 || BB->isLandingPad())
+      if (BB.pred_size() == 0 || BB.isLandingPad())
         continue;
 
       // If it's the basic block that does not end up with a terminator - we
       // insert a return instruction unless it's a call instruction.
-      if (LastInstrRI == BB->rend()) {
+      if (LastInstrRI == BB.rend()) {
         LLVM_DEBUG(
             dbgs() << "BOLT-DEBUG: at least one instruction expected in BB "
-                   << BB->getName() << " in function " << *this << '\n');
+                   << BB.getName() << " in function " << *this << '\n');
         continue;
       }
       if (!BC.MIB->isTerminator(*LastInstrRI) &&
           !BC.MIB->isCall(*LastInstrRI)) {
         LLVM_DEBUG(dbgs() << "BOLT-DEBUG: adding return to basic block "
-                          << BB->getName() << " in function " << *this << '\n');
+                          << BB.getName() << " in function " << *this << '\n');
         MCInst ReturnInstr;
         BC.MIB->createReturn(ReturnInstr);
-        BB->addInstruction(ReturnInstr);
+        BB.addInstruction(ReturnInstr);
       }
     }
   }
   assert(validateCFG() && "invalid CFG");
 }
 
 MCSymbol *BinaryFunction::addEntryPointAtOffset(uint64_t Offset) {
   assert(Offset && "cannot add primary entry point");
if (!EntrySymbol)
       continue;
 
     Status = Callback(KV.first, EntrySymbol);
   }
 
   return Status;
 }
 
-BinaryFunction::BasicBlockOrderType BinaryFunction::dfs() const {
-  BasicBlockOrderType DFS;
+BinaryFunction::BasicBlockListType BinaryFunction::dfs() const {
+  BasicBlockListType DFS;
   unsigned Index = 0;
   std::stack<BinaryBasicBlock *> Stack;
 
   // Push entry points to the stack in reverse order.
   //
   // NB: we rely on the original order of entries to match.
-  for (auto BBI = layout_rbegin(); BBI != layout_rend(); ++BBI) {
-    BinaryBasicBlock *BB = *BBI;
-    if (isEntryPoint(*BB))
-      Stack.push(BB);
-    BB->setLayoutIndex(BinaryBasicBlock::InvalidIndex);
-  }
+  SmallVector<BinaryBasicBlock *> EntryPoints;
+  llvm::copy_if(BasicBlocks, std::back_inserter(EntryPoints),
+          [&](const BinaryBasicBlock *const BB) { return isEntryPoint(*BB); });
+  // Sort entry points by their offset to make sure we got them in the right
+  // order.
+  llvm::stable_sort(EntryPoints, [](const BinaryBasicBlock *const A,
+                              const BinaryBasicBlock *const B) {
+    return A->getOffset() < B->getOffset();
+  });
+  for (BinaryBasicBlock *const BB : reverse(EntryPoints))
+
+  for (BinaryBasicBlock &BB : blocks())
+    BB.setLayoutIndex(BinaryBasicBlock::InvalidIndex);
 
   while (!Stack.empty()) {
     BinaryBasicBlock *BB = Stack.top();
     Stack.pop();
 
     if (BB->getLayoutIndex() != BinaryBasicBlock::InvalidIndex)
       continue;
 
void BinaryFunction::adjustExecutionCount(uint64_t Count) {
 
   if (ExecutionCount < Count)
     Count = ExecutionCount;
 
   double AdjustmentRatio = ((double)ExecutionCount - Count) / ExecutionCount;
   if (AdjustmentRatio < 0.0)
     AdjustmentRatio = 0.0;
 
-  for (BinaryBasicBlock *&BB : layout())
-    BB->adjustExecutionCount(AdjustmentRatio);
+  for (BinaryBasicBlock &BB : blocks())
+    BB.adjustExecutionCount(AdjustmentRatio);
 
   ExecutionCount -= Count;
 }
 
 BinaryFunction::~BinaryFunction() {
   for (BinaryBasicBlock *BB : BasicBlocks)
     delete BB;
   for (BinaryBasicBlock *BB : DeletedBasicBlocks)

bolt/lib/Passes/ADRRelaxationPass.cpp

AdrPassOpt("adr-relaxation",
                cl::init(true), cl::cat(BoltCategory), cl::ReallyHidden);
 } // namespace opts
 
 namespace llvm {
 namespace bolt {
 
 void ADRRelaxationPass::runOnFunction(BinaryFunction &BF) {
   BinaryContext &BC = BF.getBinaryContext();
-  for (BinaryBasicBlock *BB : BF.layout()) {
-    for (auto It = BB->begin(); It != BB->end(); ++It) {
+  for (BinaryBasicBlock &BB : BF) {
+    for (auto It = BB.begin(); It != BB.end(); ++It) {
       MCInst &Inst = *It;
       if (!BC.MIB->isADR(Inst))
         continue;
 
       const MCSymbol *Symbol = BC.MIB->getTargetSymbol(Inst);
       if (!Symbol)
         continue;
 
       if (BF.hasIslandsInfo()) {
         BinaryFunction::IslandInfo &Islands = BF.getIslandInfo();
         if (Islands.Symbols.count(Symbol) || Islands.ProxySymbols.count(Symbol))
           continue;
       }
 
       BinaryFunction *TargetBF = BC.getFunctionForSymbol(Symbol);
       if (TargetBF && TargetBF == &BF)
         continue;
 
       MCPhysReg Reg;
       BC.MIB->getADRReg(Inst, Reg);
       int64_t Addend = BC.MIB->getTargetAddend(Inst);
       InstructionListType Addr =
           BC.MIB->materializeAddress(Symbol, BC.Ctx.get(), Reg, Addend);
-      It = BB->replaceInstruction(It, Addr);
+      It = BB.replaceInstruction(It, Addr);
     }
   }
 }
 
 void ADRRelaxationPass::runOnFunctions(BinaryContext &BC) {
   if (!opts::AdrPassOpt || !BC.HasRelocations)
     return;
 

bolt/lib/Passes/Aligner.cpp

 // the fuction by not more than the minimum over
 // -- the size of the function
 // -- the specified number of bytes
 void alignCompact(BinaryFunction &Function, const MCCodeEmitter *Emitter) {
   const BinaryContext &BC = Function.getBinaryContext();
   size_t HotSize = 0;
   size_t ColdSize = 0;
 
-  for (const BinaryBasicBlock *BB : Function.layout())
-    if (BB->isCold())
-      ColdSize += BC.computeCodeSize(BB->begin(), BB->end(), Emitter);
+  for (const BinaryBasicBlock &BB : Function)
+    if (BB.isCold())
+      ColdSize += BC.computeCodeSize(BB.begin(), BB.end(), Emitter);
     else
-      HotSize += BC.computeCodeSize(BB->begin(), BB->end(), Emitter);
+      HotSize += BC.computeCodeSize(BB.begin(), BB.end(), Emitter);
 
   Function.setAlignment(opts::AlignFunctions);
   if (HotSize > 0)
     Function.setMaxAlignmentBytes(
       std::min(size_t(opts::AlignFunctionsMaxBytes), HotSize));
 
   // using the same option, max-align-bytes, both for cold and hot parts of the
   // functions, as aligning cold functions typically does not affect performance

bolt/lib/Passes/BinaryPasses.cpp

 }
 
 void EliminateUnreachableBlocks::runOnFunction(BinaryFunction &Function) {
   if (Function.layout_size() > 0) {
     unsigned Count;
     uint64_t Bytes;
     Function.markUnreachableBlocks();
     LLVM_DEBUG({
-      for (BinaryBasicBlock *BB : Function.layout()) {
-        if (!BB->isValid()) {
-          dbgs() << "BOLT-INFO: UCE found unreachable block " << BB->getName()
+      for (BinaryBasicBlock &BB : Function) {
+        if (!BB.isValid()) {
+          dbgs() << "BOLT-INFO: UCE found unreachable block " << BB.getName()
                  << " in function " << Function << "\n";
           Function.dump();
         }
       }
     });
     std::tie(Count, Bytes) = Function.eraseInvalidBBs();
     DeletedBlocks += Count;
     DeletedBytes += Bytes;

bolt/lib/Passes/CacheMetrics.cpp

 /// Initialize and return a position map for binary basic blocks
 void extractBasicBlockInfo(
     const std::vector<BinaryFunction *> &BinaryFunctions,
     std::unordered_map<BinaryBasicBlock *, uint64_t> &BBAddr,
     std::unordered_map<BinaryBasicBlock *, uint64_t> &BBSize) {
 
   for (BinaryFunction *BF : BinaryFunctions) {
     const BinaryContext &BC = BF->getBinaryContext();
-    for (BinaryBasicBlock *BB : BF->layout()) {
+    for (BinaryBasicBlock &BB : *BF) {
       if (BF->isSimple() || BC.HasRelocations) {
         // Use addresses/sizes as in the output binary
-        BBAddr[BB] = BB->getOutputAddressRange().first;
-        BBSize[BB] = BB->getOutputSize();
+        BBAddr[&BB] = BB.getOutputAddressRange().first;
+        BBSize[&BB] = BB.getOutputSize();
       } else {
         // Output ranges should match the input if the body hasn't changed
-        BBAddr[BB] = BB->getInputAddressRange().first + BF->getAddress();
-        BBSize[BB] = BB->getOriginalSize();
+        BBAddr[&BB] = BB.getInputAddressRange().first + BF->getAddress();
+        BBSize[&BB] = BB.getOriginalSize();
       }
     }
   }
 }
 
 /// Calculate TSP metric, which quantifies the number of fallthrough jumps in
 /// the ordering of basic blocks
 double
 calcTSPScore(const std::vector<BinaryFunction *> &BinaryFunctions,
              const std::unordered_map<BinaryBasicBlock *, uint64_t> &BBAddr,
              const std::unordered_map<BinaryBasicBlock *, uint64_t> &BBSize) {
 
   double Score = 0;
   for (BinaryFunction *BF : BinaryFunctions) {
     if (!BF->hasProfile())
       continue;
-    for (BinaryBasicBlock *SrcBB : BF->layout()) {
-      auto BI = SrcBB->branch_info_begin();
-      for (BinaryBasicBlock *DstBB : SrcBB->successors()) {
-        if (SrcBB != DstBB && BI->Count != BinaryBasicBlock::COUNT_NO_PROFILE &&
-            BBAddr.at(SrcBB) + BBSize.at(SrcBB) == BBAddr.at(DstBB))
+    for (BinaryBasicBlock &SrcBB : *BF) {
+      auto BI = SrcBB.branch_info_begin();
+      for (BinaryBasicBlock *DstBB : SrcBB.successors()) {
+        if (&SrcBB != DstBB &&
+            BI->Count != BinaryBasicBlock::COUNT_NO_PROFILE &&
+            BBAddr.at(&SrcBB) + BBSize.at(&SrcBB) == BBAddr.at(DstBB))
           Score += BI->Count;
         ++BI;
       }
     }
   }
   return Score;
 }
 
 /// Calculate Ext-TSP metric, which quantifies the expected number of i-cache
 /// misses for a given ordering of basic blocks
 double calcExtTSPScore(
     const std::vector<BinaryFunction *> &BinaryFunctions,
     const std::unordered_map<BinaryBasicBlock *, uint64_t> &BBAddr,
     const std::unordered_map<BinaryBasicBlock *, uint64_t> &BBSize) {
 
   double Score = 0.0;
   for (BinaryFunction *BF : BinaryFunctions) {
     if (!BF->hasProfile())
       continue;
-    for (BinaryBasicBlock *SrcBB : BF->layout()) {
-      auto BI = SrcBB->branch_info_begin();
-      for (BinaryBasicBlock *DstBB : SrcBB->successors()) {
-        if (DstBB != SrcBB)
-          Score += CacheMetrics::extTSPScore(BBAddr.at(SrcBB), BBSize.at(SrcBB),
-                                             BBAddr.at(DstBB), BI->Count);
+    for (BinaryBasicBlock &SrcBB : *BF) {
+      auto BI = SrcBB.branch_info_begin();
+      for (BinaryBasicBlock *DstBB : SrcBB.successors()) {
+        if (DstBB != &SrcBB)
+          Score +=
+              CacheMetrics::extTSPScore(BBAddr.at(&SrcBB), BBSize.at(&SrcBB),
         ++BI;
       }
     }
   }
   return Score;
 }
 
 using Predecessors = std::vector<std::pair<BinaryFunction *, uint64_t>>;
void CacheMetrics::printAll(const std::vector<BinaryFunction *> &BFs) {
   size_t HotCodeMaxAddr = 0;
 
   for (BinaryFunction *BF : BFs) {
     NumFunctions++;
     if (BF->hasProfile())
       NumProfiledFunctions++;
     if (BF->hasValidIndex())
       NumHotFunctions++;
-    for (BinaryBasicBlock *BB : BF->layout()) {
+    for (const BinaryBasicBlock &BB : *BF) {
       NumBlocks++;
-      size_t BBAddrMin = BB->getOutputAddressRange().first;
-      size_t BBAddrMax = BB->getOutputAddressRange().second;
+      size_t BBAddrMin = BB.getOutputAddressRange().first;
+      size_t BBAddrMax = BB.getOutputAddressRange().second;
       TotalCodeMinAddr = std::min(TotalCodeMinAddr, BBAddrMin);
       TotalCodeMaxAddr = std::max(TotalCodeMaxAddr, BBAddrMax);
-      if (BF->hasValidIndex() && !BB->isCold()) {
+      if (BF->hasValidIndex() && !BB.isCold()) {
         NumHotBlocks++;
         HotCodeMinAddr = std::min(HotCodeMinAddr, BBAddrMin);
         HotCodeMaxAddr = std::max(HotCodeMaxAddr, BBAddrMax);
       }
     }
   }
 
   outs() << format("  There are %zu functions;", NumFunctions)

bolt/lib/Passes/IndirectCallPromotion.cpp

 } // namespace opts
 
 static bool verifyProfile(std::map<uint64_t, BinaryFunction> &BFs) {
   bool IsValid = true;
   for (auto &BFI : BFs) {
     BinaryFunction &BF = BFI.second;
     if (!BF.isSimple())
       continue;
-    for (BinaryBasicBlock *BB : BF.layout()) {
-      auto BI = BB->branch_info_begin();
-      for (BinaryBasicBlock *SuccBB : BB->successors()) {
+    for (const BinaryBasicBlock &BB : BF) {
+      auto BI = BB.branch_info_begin();
+      for (BinaryBasicBlock *SuccBB : BB.successors()) {
         if (BI->Count != BinaryBasicBlock::COUNT_NO_PROFILE && BI->Count > 0) {
-          if (BB->getKnownExecutionCount() == 0 ||
+          if (BB.getKnownExecutionCount() == 0 ||
               SuccBB->getKnownExecutionCount() == 0) {
             errs() << "BOLT-WARNING: profile verification failed after ICP for "
                       "function "
                    << BF << '\n';
             IsValid = false;
           }
         }
         ++BI;

bolt/lib/Passes/Inliner.cpp

if (!opts::mustConsider(BF)) {
 
     if (BF.isMultiEntry())
       return INL_NONE;
 
     if (BF.hasJumpTables())
       return INL_NONE;
 
     const MCPhysReg SPReg = BC.MIB->getStackPointer();
-    for (const BinaryBasicBlock *BB : BF.layout()) {
-      for (const MCInst &Inst : *BB) {
+    for (const BinaryBasicBlock &BB : BF) {
+      for (const MCInst &Inst : BB) {
         // Tail calls are marked as implicitly using the stack pointer and they
         // could be inlined.
         if (BC.MIB->isTailCall(Inst))
           break;
 
         if (BC.MIB->isCFI(Inst)) {
           HasCFI = true;
           continue;

bolt/lib/Passes/PLTCall.cpp

for (auto &It : BC.getBinaryFunctions()) {
     BinaryFunction &Function = It.second;
     if (!shouldOptimize(Function))
       continue;
 
     if (opts::PLT == OT_HOT &&
         Function.getExecutionCount() == BinaryFunction::COUNT_NO_PROFILE)
       continue;
 
-    for (BinaryBasicBlock *BB : Function.layout()) {
-      if (opts::PLT == OT_HOT && !BB->getKnownExecutionCount())
+    for (BinaryBasicBlock &BB : Function) {
+      if (opts::PLT == OT_HOT && !BB.getKnownExecutionCount())
         continue;
 
-      for (MCInst &Instr : *BB) {
+      for (MCInst &Instr : BB) {
         if (!BC.MIB->isCall(Instr))
           continue;
         const MCSymbol *CallSymbol = BC.MIB->getTargetSymbol(Instr);
         if (!CallSymbol)
           continue;
         const BinaryFunction *CalleeBF = BC.getFunctionForSymbol(CallSymbol);
         if (!CalleeBF || !CalleeBF->isPLTFunction())
           continue;

bolt/lib/Passes/SplitFunctions.cpp

 
 namespace {
 struct SplitCold {
   bool canSplit(const BinaryFunction &BF) {
     if (!BF.hasValidProfile())
       return false;
 
     bool AllCold = true;
-    for (BinaryBasicBlock *BB : BF.layout()) {
-      const uint64_t ExecCount = BB->getExecutionCount();
+    for (const BinaryBasicBlock &BB : BF) {
+      const uint64_t ExecCount = BB.getExecutionCount();
       if (ExecCount == BinaryBasicBlock::COUNT_NO_PROFILE)
         return false;
       if (ExecCount != 0)
         AllCold = false;
     }
 
     return !AllCold;
   }

bolt/lib/Passes/ThreeWayBranch.cpp

 
 using namespace llvm;
 
 namespace llvm {
 namespace bolt {
 
 bool ThreeWayBranch::shouldRunOnFunction(BinaryFunction &Function) {
   BinaryContext &BC = Function.getBinaryContext();
-  BinaryFunction::BasicBlockOrderType BlockLayout = Function.getLayout();
-  for (BinaryBasicBlock *BB : BlockLayout)
-    for (MCInst &Inst : *BB)
+  for (const BinaryBasicBlock &BB : Function)
+    for (const MCInst &Inst : BB)
       if (BC.MIB->isPacked(Inst))
         return false;
   return true;
 }
 
 void ThreeWayBranch::runOnFunction(BinaryFunction &Function) {
   BinaryContext &BC = Function.getBinaryContext();
   MCContext *Ctx = BC.Ctx.get();