diff --git a/bolt/include/bolt/Core/FunctionLayout.h b/bolt/include/bolt/Core/FunctionLayout.h --- a/bolt/include/bolt/Core/FunctionLayout.h +++ b/bolt/include/bolt/Core/FunctionLayout.h @@ -23,6 +23,7 @@ #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/iterator.h" +#include "llvm/ADT/iterator_range.h" namespace llvm { namespace bolt { @@ -100,7 +101,10 @@ const FunctionLayout *Layout; FragmentIterator(FragmentNum Num, const FunctionLayout *Layout) - : Num(Num), Layout(Layout) {} + : Num(Num), Layout(Layout) { + assert(Num.get() <= Layout->fragment_size() && + "Initializing iterator out of bounds"); + } public: bool operator==(const FragmentIterator &Other) const { @@ -108,15 +112,20 @@ } FunctionFragment operator*() const { + assert(Num.get() < Layout->fragment_size() && + "Dereferencing end() iterator (or past it)"); return FunctionFragment(Num, *Layout); } FragmentIterator &operator++() { + assert(Num.get() < Layout->fragment_size() && + "Incrementing iterator past end()"); Num = FragmentNum(Num.get() + 1); return *this; } FragmentIterator &operator--() { + assert(Num.get() > 0 && "Decrementing iterator past begin()"); Num = FragmentNum(Num.get() - 1); return *this; } @@ -133,8 +142,9 @@ BasicBlockListType Blocks; /// List of indices dividing block list into fragments. To simplify iteration, /// we have `Fragments.back()` equals `Blocks.size()`. Hence, - /// `Fragments.size()` equals `this->size() + 1`. - FragmentListType Fragments = {0}; + /// `Fragments.size()` equals `this->size() + 1`. Always contains at least one + /// fragment. + FragmentListType Fragments = {0, 0}; BasicBlockListType PreviousBlocks; FragmentListType PreviousFragments; @@ -188,14 +198,23 @@ /// layout after basic block reordering. uint64_t getEditDistance() const; - size_t size() const { return Fragments.size() - 1; } - bool empty() const { return Fragments.size() == 1; } - const_iterator begin() const { return {FragmentNum(0), this}; } - const_iterator end() const { return {FragmentNum(size()), this}; } - FunctionFragment front() const { return *begin(); } - FunctionFragment back() const { return *std::prev(end()); } - FunctionFragment operator[](const FragmentNum Num) const { - return getFragment(Num); + /// True if the function is split into at most 2 fragments. Mostly used for + /// checking whether a function can be processed in places that do not support + /// multiple fragments yet. + bool isHotColdSplit() const { return fragment_size() <= 2; } + + size_t fragment_size() const { + assert(Fragments.size() >= 2 && + "Layout should have at least one fragment."); + return Fragments.size() - 1; + } + bool fragment_empty() const { return Fragments.size() == 1; } + const_iterator fragment_begin() const { return {FragmentNum(0), this}; } + const_iterator fragment_end() const { + return {FragmentNum(fragment_size()), this}; + } + iterator_range fragments() const { + return {fragment_begin(), fragment_end()}; } size_t block_size() const { return Blocks.size(); } diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp --- a/bolt/lib/Core/BinaryEmitter.cpp +++ b/bolt/lib/Core/BinaryEmitter.cpp @@ -15,6 +15,7 @@ #include "bolt/Core/BinaryContext.h" #include "bolt/Core/BinaryFunction.h" #include "bolt/Core/DebugData.h" +#include "bolt/Core/FunctionLayout.h" #include "bolt/Utils/CommandLineOpts.h" #include "bolt/Utils/Utils.h" #include "llvm/DebugInfo/DWARF/DWARFCompileUnit.h" @@ -396,12 +397,12 @@ if (!EmitCodeOnly && EmitColdPart && BF.hasConstantIsland()) BF.duplicateConstantIslands(); + const FunctionFragment FF = BF.getLayout().getFragment( + EmitColdPart ? FragmentNum::cold() : FragmentNum::hot()); + // Track the first emitted instruction with debug info. bool FirstInstr = true; - for (BinaryBasicBlock *BB : BF.getLayout().blocks()) { - if (EmitColdPart != BB->isCold()) - continue; - + for (BinaryBasicBlock *const BB : FF) { if ((opts::AlignBlocks || opts::PreserveBlocksAlignment) && BB->getAlignment() > 1) Streamer.emitCodeAlignment(BB->getAlignment(), &*BC.STI, diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp --- a/bolt/lib/Core/BinaryFunction.cpp +++ b/bolt/lib/Core/BinaryFunction.cpp @@ -503,10 +503,10 @@ } StringRef SplitPointMsg = ""; - for (const FunctionFragment F : Layout) { + for (const FunctionFragment FF : Layout.fragments()) { OS << SplitPointMsg; SplitPointMsg = "------- HOT-COLD SPLIT POINT -------\n\n"; - for (const BinaryBasicBlock *BB : F) { + for (const BinaryBasicBlock *BB : FF) { OS << BB->getName() << " (" << BB->size() << " instructions, align : " << BB->getAlignment() << ")\n"; @@ -2782,12 +2782,12 @@ const char *Sep = ""; (void)Sep; - for (const FunctionFragment F : Layout) { + for (const FunctionFragment FF : Layout.fragments()) { // Hot-cold border: at start of each region (with a different FDE) we need // to reset the CFI state. int32_t State = 0; - for (BinaryBasicBlock *BB : F) { + for (BinaryBasicBlock *BB : FF) { const int32_t CFIStateAtExit = BB->getCFIStateAtExit(); // We need to recover the correct state if it doesn't match expected diff --git a/bolt/lib/Core/FunctionLayout.cpp b/bolt/lib/Core/FunctionLayout.cpp --- a/bolt/lib/Core/FunctionLayout.cpp +++ b/bolt/lib/Core/FunctionLayout.cpp @@ -20,7 +20,7 @@ FunctionFragment FunctionLayout::addFragment() { Fragments.emplace_back(Blocks.size()); - return back(); + return getFragment(FragmentNum(Blocks.size() - 1)); } FunctionFragment FunctionLayout::getFragment(FragmentNum Num) const { @@ -33,21 +33,14 @@ } void FunctionLayout::addBasicBlock(BinaryBasicBlock *BB) { - if (empty()) - addFragment(); - BB->setLayoutIndex(Blocks.size()); Blocks.emplace_back(BB); ++Fragments.back(); - assert(Fragments.back() == Blocks.size()); } void FunctionLayout::insertBasicBlocks(BinaryBasicBlock *InsertAfter, ArrayRef NewBlocks) { - if (empty()) - addFragment(); - const block_iterator InsertBeforePos = InsertAfter ? std::next(findBasicBlockPos(InsertAfter)) : Blocks.begin(); Blocks.insert(InsertBeforePos, NewBlocks.begin(), NewBlocks.end()); @@ -66,9 +59,9 @@ }; FragmentListType NewFragments; NewFragments.emplace_back(0); - for (const FunctionFragment F : *this) { - unsigned ErasedBlocks = count_if(F, IsErased); - unsigned NewFragment = NewFragments.back() + F.size() - ErasedBlocks; + for (const FunctionFragment FF : fragments()) { + unsigned ErasedBlocks = count_if(FF, IsErased); + unsigned NewFragment = NewFragments.back() + FF.size() - ErasedBlocks; NewFragments.emplace_back(NewFragment); } llvm::erase_if(Blocks, IsErased); @@ -77,8 +70,8 @@ void FunctionLayout::updateLayoutIndices() const { unsigned BlockIndex = 0; - for (const FunctionFragment F : *this) { - for (BinaryBasicBlock *const BB : F) + for (const FunctionFragment FF : fragments()) { + for (BinaryBasicBlock *const BB : FF) BB->setLayoutIndex(BlockIndex++); } } @@ -88,7 +81,7 @@ PreviousFragments = std::move(Fragments); Blocks.clear(); - Fragments = {0}; + Fragments = {0, 0}; if (NewLayout.empty()) return; @@ -99,12 +92,12 @@ for (const auto &BB : enumerate(Blocks)) { unsigned FragmentNum = BB.value()->getFragmentNum().get(); - assert(FragmentNum + 1 >= size() && + assert(FragmentNum >= fragment_size() - 1 && "Blocks must be arranged such that fragments are monotonically " "increasing."); // Add empty fragments if necessary - for (unsigned I = size(); I <= FragmentNum; ++I) { + for (unsigned I = fragment_size(); I <= FragmentNum; ++I) { addFragment(); Fragments[I] = BB.index(); } @@ -123,9 +116,9 @@ void FunctionLayout::clear() { Blocks = {}; - Fragments = {0}; + Fragments = {0, 0}; PreviousBlocks = {}; - PreviousFragments = {0}; + PreviousFragments = {0, 0}; } BinaryBasicBlock *FunctionLayout::getBasicBlockAfter(const BinaryBasicBlock *BB, @@ -147,7 +140,7 @@ bool FunctionLayout::isSplit() const { unsigned NonEmptyFragCount = llvm::count_if( - *this, [](const FunctionFragment &F) { return !F.empty(); }); + fragments(), [](const FunctionFragment &FF) { return !FF.empty(); }); return NonEmptyFragCount >= 2; }