diff --git a/bolt/include/bolt/Core/BinaryBasicBlock.h b/bolt/include/bolt/Core/BinaryBasicBlock.h --- a/bolt/include/bolt/Core/BinaryBasicBlock.h +++ b/bolt/include/bolt/Core/BinaryBasicBlock.h @@ -681,7 +681,7 @@ bool isCold() const { assert(Fragment.get() < 2 && "Function is split into more than two (hot/cold)-fragments"); - return Fragment == FragmentNum::cold(); + return isSplit(); } void setIsCold(const bool Flag) { diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h --- a/bolt/include/bolt/Core/BinaryFunction.h +++ b/bolt/include/bolt/Core/BinaryFunction.h @@ -1078,8 +1078,8 @@ /// Return MC symbol associated with the function. /// All references to the function should use this symbol. - MCSymbol *getSymbol(const FragmentNum Fragment = FragmentNum::hot()) { - if (Fragment == FragmentNum::hot()) + MCSymbol *getSymbol(const FragmentNum Fragment = FragmentNum::main()) { + if (Fragment == FragmentNum::main()) return Symbols[0]; size_t ColdSymbolIndex = Fragment.get() - 1; @@ -1316,34 +1316,29 @@ } /// Return internal section name for this function. - StringRef getCodeSectionName() const { return StringRef(CodeSectionName); } + SmallString<32> + getCodeSectionName(const FragmentNum Fragment = FragmentNum::main()) const { + if (Fragment == FragmentNum::main()) + return SmallString<32>(CodeSectionName); + if (Fragment == FragmentNum::cold()) + return SmallString<32>(ColdCodeSectionName); + return formatv("{0}.{1}", ColdCodeSectionName, Fragment.get() - 1); + } /// Assign a code section name to the function. - void setCodeSectionName(StringRef Name) { - CodeSectionName = std::string(Name); + void setCodeSectionName(const StringRef Name) { + CodeSectionName = Name.str(); } /// Get output code section. - ErrorOr getCodeSection() const { - return BC.getUniqueSectionByName(getCodeSectionName()); - } - - /// Return cold code section name for the function. - std::string getColdCodeSectionName(FragmentNum Fragment) const { - std::string Result = ColdCodeSectionName; - if (Fragment != FragmentNum::cold()) - Result.append(".").append(std::to_string(Fragment.get() - 1)); - return Result; + ErrorOr + getCodeSection(const FragmentNum Fragment = FragmentNum::main()) const { + return BC.getUniqueSectionByName(getCodeSectionName(Fragment)); } /// Assign a section name for the cold part of the function. void setColdCodeSectionName(StringRef Name) { - ColdCodeSectionName = std::string(Name); - } - - /// Get output code section for cold code of this function. - ErrorOr getColdCodeSection(FragmentNum Fragment) const { - return BC.getUniqueSectionByName(getColdCodeSectionName(Fragment)); + ColdCodeSectionName = Name.str(); } /// Return true iif the function will halt execution on entry. @@ -1877,11 +1872,9 @@ if (ColdCallSites.empty()) return nullptr; - SmallString<8> SymbolSuffix; - if (Fragment != FragmentNum::cold()) - SymbolSuffix = formatv(".{0}", Fragment.get()); - ColdLSDASymbol = BC.Ctx->getOrCreateSymbol(formatv( - "GCC_cold_except_table{0:x-}{1}", getFunctionNumber(), SymbolSuffix)); + ColdLSDASymbol = + BC.Ctx->getOrCreateSymbol(formatv("GCC_cold_except_table{0:x-}.{1}", + getFunctionNumber(), Fragment.get())); return ColdLSDASymbol; } 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 @@ -163,7 +163,7 @@ /// Get the fragment that contains all entry blocks and other blocks that /// cannot be split. FunctionFragment getMainFragment() const { - return getFragment(FragmentNum::hot()); + return getFragment(FragmentNum::main()); } /// Get the fragment that contains all entry blocks and other blocks that @@ -183,7 +183,8 @@ void insertBasicBlocks(BinaryBasicBlock *InsertAfter, ArrayRef NewBlocks); - /// Erase all blocks from the layout that are in ToErase. + /// Erase all blocks from the layout that are in ToErase. If this method + /// erases all blocks of a fragment, it will be removed as well. void eraseBasicBlocks(const DenseSet ToErase); /// Make sure fragments' and basic blocks' indices match the current layout. diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp --- a/bolt/lib/Core/BinaryContext.cpp +++ b/bolt/lib/Core/BinaryContext.cpp @@ -2207,7 +2207,7 @@ ColdStartEndLabels.emplace_back(FragStartLabel, FragEndLabel); MCSectionELF *const ColdSection = LocalCtx->getELFSection( - BF.getColdCodeSectionName(F.getFragmentNum()), ELF::SHT_PROGBITS, + BF.getCodeSectionName(F.getFragmentNum()), ELF::SHT_PROGBITS, ELF::SHF_EXECINSTR | ELF::SHF_ALLOC); ColdSection->setHasInstructions(true); Streamer->switchSection(ColdSection); 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 @@ -282,9 +282,8 @@ if (Function.getState() == BinaryFunction::State::Empty) return false; - MCSection *Section = BC.getCodeSection( - F.isSplitFragment() ? Function.getColdCodeSectionName(F.getFragmentNum()) - : Function.getCodeSectionName()); + MCSection *Section = + BC.getCodeSection(Function.getCodeSectionName(F.getFragmentNum())); Streamer.switchSection(Section); Section->setHasInstructions(true); BC.Ctx->addGenDwarfSection(Section); @@ -402,7 +401,7 @@ const FunctionFragment &F, bool EmitCodeOnly) { if (!EmitCodeOnly && F.isSplitFragment() && BF.hasConstantIsland()) { - assert(F.getFragmentNum() == FragmentNum::cold() && + assert(BF.getLayout().isHotColdSplit() && "Constant island support only with hot/cold split"); BF.duplicateConstantIslands(); } @@ -902,7 +901,7 @@ // Corresponding FDE start. const MCSymbol *StartSymbol = - EmitColdPart ? BF.getSymbol(FragmentNum::cold()) : BF.getSymbol(); + BF.getSymbol(EmitColdPart ? FragmentNum::cold() : FragmentNum::main()); // Emit the LSDA header. 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 @@ -4042,10 +4042,6 @@ } const uint64_t BaseAddress = getCodeSection()->getOutputAddress(); - ErrorOr ColdSection = - getColdCodeSection(FragmentNum::cold()); - const uint64_t ColdBaseAddress = - isSplit() ? ColdSection->getOutputAddress() : 0; if (BC.HasRelocations || isInjected()) { const uint64_t StartOffset = Layout.getSymbolOffset(*getSymbol()); const uint64_t EndOffset = Layout.getSymbolOffset(*getFunctionEndLabel()); @@ -4057,20 +4053,27 @@ setOutputDataAddress(BaseAddress + DataOffset); } if (isSplit()) { - const MCSymbol *ColdStartSymbol = getSymbol(FragmentNum::cold()); - assert(ColdStartSymbol && ColdStartSymbol->isDefined() && - "split function should have defined cold symbol"); - const MCSymbol *ColdEndSymbol = getFunctionEndLabel(FragmentNum::cold()); - assert(ColdEndSymbol && ColdEndSymbol->isDefined() && - "split function should have defined cold end symbol"); - const uint64_t ColdStartOffset = Layout.getSymbolOffset(*ColdStartSymbol); - const uint64_t ColdEndOffset = Layout.getSymbolOffset(*ColdEndSymbol); - cold().setAddress(ColdBaseAddress + ColdStartOffset); - cold().setImageSize(ColdEndOffset - ColdStartOffset); - if (hasConstantIsland()) { - const uint64_t DataOffset = - Layout.getSymbolOffset(*getFunctionColdConstantIslandLabel()); - setOutputColdDataAddress(ColdBaseAddress + DataOffset); + for (const FunctionFragment F : getLayout().getSplitFragments()) { + ErrorOr ColdSection = + getCodeSection(F.getFragmentNum()); + const uint64_t ColdBaseAddress = ColdSection->getOutputAddress(); + + const MCSymbol *ColdStartSymbol = getSymbol(F.getFragmentNum()); + assert(ColdStartSymbol && ColdStartSymbol->isDefined() && + "split function should have defined cold symbol"); + const MCSymbol *ColdEndSymbol = getFunctionEndLabel(F.getFragmentNum()); + assert(ColdEndSymbol && ColdEndSymbol->isDefined() && + "split function should have defined cold end symbol"); + const uint64_t ColdStartOffset = + Layout.getSymbolOffset(*ColdStartSymbol); + const uint64_t ColdEndOffset = Layout.getSymbolOffset(*ColdEndSymbol); + cold().setAddress(ColdBaseAddress + ColdStartOffset); + cold().setImageSize(ColdEndOffset - ColdStartOffset); + if (hasConstantIsland()) { + const uint64_t DataOffset = + Layout.getSymbolOffset(*getFunctionColdConstantIslandLabel()); + setOutputColdDataAddress(ColdBaseAddress + DataOffset); + } } } } else { @@ -4093,29 +4096,38 @@ return; BinaryBasicBlock *PrevBB = nullptr; - for (BinaryBasicBlock *BB : this->Layout.blocks()) { - assert(BB->getLabel()->isDefined() && "symbol should be defined"); - const uint64_t BBBaseAddress = BB->isCold() ? ColdBaseAddress : BaseAddress; - if (!BC.HasRelocations) { - if (BB->isCold()) { - assert(BBBaseAddress == cold().getAddress()); - } else { - assert(BBBaseAddress == getOutputAddress()); - } + for (const FunctionFragment F : getLayout().fragments()) { + uint64_t ColdBaseAddress = 0; + if (isSimple() && F.isSplitFragment()) { + ErrorOr ColdSection = getCodeSection(F.getFragmentNum()); + ColdBaseAddress = ColdSection->getOutputAddress(); } - const uint64_t BBOffset = Layout.getSymbolOffset(*BB->getLabel()); - const uint64_t BBAddress = BBBaseAddress + BBOffset; - BB->setOutputStartAddress(BBAddress); - - if (PrevBB) { - uint64_t PrevBBEndAddress = BBAddress; - if (BB->isCold() != PrevBB->isCold()) - PrevBBEndAddress = getOutputAddress() + getOutputSize(); - PrevBB->setOutputEndAddress(PrevBBEndAddress); - } - PrevBB = BB; - BB->updateOutputValues(Layout); + for (BinaryBasicBlock *const BB : F) { + assert(BB->getLabel()->isDefined() && "symbol should be defined"); + const uint64_t BBBaseAddress = + BB->isCold() ? ColdBaseAddress : BaseAddress; + if (!BC.HasRelocations) { + if (BB->isCold()) { + assert(BBBaseAddress == cold().getAddress()); + } else { + assert(BBBaseAddress == getOutputAddress()); + } + } + const uint64_t BBOffset = Layout.getSymbolOffset(*BB->getLabel()); + const uint64_t BBAddress = BBBaseAddress + BBOffset; + BB->setOutputStartAddress(BBAddress); + + if (PrevBB) { + uint64_t PrevBBEndAddress = BBAddress; + if (BB->isCold() != PrevBB->isCold()) + PrevBBEndAddress = getOutputAddress() + getOutputSize(); + PrevBB->setOutputEndAddress(PrevBBEndAddress); + } + PrevBB = BB; + + BB->updateOutputValues(Layout); + } } PrevBB->setOutputEndAddress(PrevBB->isCold() ? cold().getAddress() + cold().getImageSize() 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 @@ -61,19 +61,23 @@ NewFragments.emplace_back(0); for (const FunctionFragment F : fragments()) { unsigned ErasedBlocks = count_if(F, IsErased); + // Only add the fragment if it is non-empty after removing blocks. unsigned NewFragment = NewFragments.back() + F.size() - ErasedBlocks; NewFragments.emplace_back(NewFragment); } Blocks.erase(std::remove_if(Blocks.begin(), Blocks.end(), IsErased), Blocks.end()); Fragments = std::move(NewFragments); + updateLayoutIndices(); } void FunctionLayout::updateLayoutIndices() const { unsigned BlockIndex = 0; for (const FunctionFragment F : fragments()) { - for (BinaryBasicBlock *const BB : F) + for (BinaryBasicBlock *const BB : F) { BB->setLayoutIndex(BlockIndex++); + BB->setFragmentNum(F.getFragmentNum()); + } } } diff --git a/bolt/lib/Passes/SplitFunctions.cpp b/bolt/lib/Passes/SplitFunctions.cpp --- a/bolt/lib/Passes/SplitFunctions.cpp +++ b/bolt/lib/Passes/SplitFunctions.cpp @@ -345,7 +345,7 @@ PreSplitLayout = mergeEHTrampolines(BF, PreSplitLayout, Trampolines); for (BinaryBasicBlock &BB : BF) - BB.setFragmentNum(FragmentNum::hot()); + BB.setFragmentNum(FragmentNum::main()); BF.getLayout().update(PreSplitLayout); } else { SplitBytesHot += HotSize; diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp --- a/bolt/lib/Rewrite/RewriteInstance.cpp +++ b/bolt/lib/Rewrite/RewriteInstance.cpp @@ -3141,9 +3141,9 @@ Function->CodeSectionName = std::string(*Function->getOriginSectionName()); for (const FunctionFragment F : Function->getLayout().getSplitFragments()) { if (ErrorOr ColdSection = - Function->getColdCodeSection(F.getFragmentNum())) + Function->getCodeSection(F.getFragmentNum())) BC->deregisterSection(*ColdSection); - Function->ColdCodeSectionName = std::string(getBOLTTextSectionName()); + Function->setColdCodeSectionName(getBOLTTextSectionName()); } } @@ -3682,7 +3682,7 @@ for (const FunctionFragment F : Function.getLayout().getSplitFragments()) { ErrorOr ColdSection = - Function.getColdCodeSection(F.getFragmentNum()); + Function.getCodeSection(F.getFragmentNum()); assert(ColdSection && "cannot find section for cold part"); // Cold fragments are aligned at 16 bytes. NextAvailableAddress = alignTo(NextAvailableAddress, 16); @@ -4475,12 +4475,12 @@ for (const FunctionFragment F : Function.getLayout().getSplitFragments()) { ELFSymTy NewColdSym = FunctionSymbol; - SmallString<256> Buf = formatv( + SmallString<256> SymbolName = formatv( "{0}.cold.{1}", cantFail(FunctionSymbol.getName(StringSection)), F.getFragmentNum().get() - 1); - NewColdSym.st_name = AddToStrTab(Buf); + NewColdSym.st_name = AddToStrTab(SymbolName); NewColdSym.st_shndx = - Function.getColdCodeSection(F.getFragmentNum())->getIndex(); + Function.getCodeSection(F.getFragmentNum())->getIndex(); NewColdSym.st_value = Function.cold().getAddress(); NewColdSym.st_size = Function.cold().getImageSize(); NewColdSym.setBindingAndType(ELF::STB_LOCAL, ELF::STT_FUNC); @@ -4612,7 +4612,7 @@ NewSymbol.st_shndx = OutputAddress >= Function->cold().getAddress() && OutputAddress < Function->cold().getImageSize() - ? Function->getColdCodeSection(FragmentNum::cold())->getIndex() + ? Function->getCodeSection(FragmentNum::cold())->getIndex() : Function->getCodeSection()->getIndex(); } else { // Check if the symbol belongs to moved data object and update it. @@ -4710,6 +4710,9 @@ Symbols.emplace_back(NewSymbol); if (Function->isSplit()) { + assert(Function->getLayout().isHotColdSplit() && + "Adding symbols based on cold fragment when there are more than " + "2 fragments"); ELFSymTy NewColdSym = NewSymbol; NewColdSym.setType(ELF::STT_NOTYPE); SmallVector Buf;