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 @@ -133,9 +133,9 @@ /// CFI state at the entry to this basic block. int32_t CFIState{-1}; - /// In cases where the parent function has been split, IsCold == true means - /// this BB will be allocated outside its parent function. - bool IsCold{false}; + /// In cases where the parent function has been split, FragmentNum > 0 means + /// this BB will be allocated in a fragment outside its parent function. + FragmentNum Fragment; /// Indicates if the block could be outlined. bool CanOutline{true}; @@ -672,13 +672,21 @@ void markValid(const bool Valid) { IsValid = Valid; } - FragmentNum getFragmentNum() const { - return IsCold ? FragmentNum::cold() : FragmentNum::hot(); - } + FragmentNum getFragmentNum() const { return Fragment; } + + void setFragmentNum(const FragmentNum Value) { Fragment = Value; } - bool isCold() const { return IsCold; } + bool isSplit() const { return Fragment != FragmentNum::main(); } - void setIsCold(const bool Flag) { IsCold = Flag; } + bool isCold() const { + assert(Fragment.get() < 2 && + "Function is split into more than two (hot/cold)-fragments"); + return Fragment == FragmentNum::cold(); + } + + void setIsCold(const bool Flag) { + Fragment = Flag ? FragmentNum::cold() : FragmentNum::hot(); + } /// Return true if the block can be outlined. At the moment we disallow /// outlining of blocks that can potentially throw exceptions or are 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 @@ -570,11 +570,8 @@ SmallVector ColdSymbols; - /// Symbol at the end of the function. - mutable MCSymbol *FunctionEndLabel{nullptr}; - - /// Symbol at the end of the cold part of split function. - mutable MCSymbol *FunctionColdEndLabel{nullptr}; + /// Symbol at the end of each fragment of a split function. + mutable SmallVector FunctionEndLabels; /// Unique number associated with the function. uint64_t FunctionNumber; @@ -1152,22 +1149,25 @@ bool forEachEntryPoint(EntryPointCallbackTy Callback) const; /// Return MC symbol associated with the end of the function. - MCSymbol *getFunctionEndLabel() const { + MCSymbol * + getFunctionEndLabel(const FragmentNum Fragment = FragmentNum::main()) const { assert(BC.Ctx && "cannot be called with empty context"); - if (!FunctionEndLabel) { - std::unique_lock Lock(BC.CtxMutex); - FunctionEndLabel = BC.Ctx->createNamedTempSymbol("func_end"); + + size_t LabelIndex = Fragment.get(); + if (LabelIndex >= FunctionEndLabels.size()) { + FunctionEndLabels.resize(LabelIndex + 1); } - return FunctionEndLabel; - } - /// Return MC symbol associated with the end of the cold part of the function. - MCSymbol *getFunctionColdEndLabel() const { - if (!FunctionColdEndLabel) { + MCSymbol *&FunctionEndLabel = FunctionEndLabels[LabelIndex]; + if (!FunctionEndLabel) { std::unique_lock Lock(BC.CtxMutex); - FunctionColdEndLabel = BC.Ctx->createNamedTempSymbol("func_cold_end"); + if (Fragment == FragmentNum::main()) + FunctionEndLabel = BC.Ctx->createNamedTempSymbol("func_end"); + else + FunctionEndLabel = BC.Ctx->createNamedTempSymbol( + formatv("func_cold_end.{0}", Fragment.get() - 1)); } - return FunctionColdEndLabel; + return FunctionEndLabel; } /// Return a label used to identify where the constant island was emitted @@ -1872,14 +1872,17 @@ } /// Return symbol pointing to function's LSDA for the cold part. - MCSymbol *getColdLSDASymbol() { + MCSymbol *getColdLSDASymbol(const FragmentNum Fragment) { if (ColdLSDASymbol) return ColdLSDASymbol; if (ColdCallSites.empty()) return nullptr; - ColdLSDASymbol = BC.Ctx->getOrCreateSymbol( - Twine("GCC_cold_except_table") + Twine::utohexstr(getFunctionNumber())); + 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)); 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 @@ -46,6 +46,7 @@ return !(*this == Other); } + static constexpr FragmentNum main() { return FragmentNum(0); } static constexpr FragmentNum hot() { return FragmentNum(0); } static constexpr FragmentNum cold() { return FragmentNum(1); } }; diff --git a/bolt/lib/Core/BinaryBasicBlock.cpp b/bolt/lib/Core/BinaryBasicBlock.cpp --- a/bolt/lib/Core/BinaryBasicBlock.cpp +++ b/bolt/lib/Core/BinaryBasicBlock.cpp @@ -102,7 +102,7 @@ if (Valid) { for (const MCSymbol *Sym : UniqueSyms) { Valid &= (Sym == Function->getFunctionEndLabel() || - Sym == Function->getFunctionColdEndLabel()); + Sym == Function->getFunctionEndLabel(getFragmentNum())); if (!Valid) { errs() << "BOLT-WARNING: Jump table contains illegal entry: " << Sym->getName() << "\n"; 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 @@ -331,14 +331,13 @@ // Emit CFI start if (Function.hasCFI()) { - assert(Function.getLayout().isHotColdSplit() && - "Exceptions supported only with hot/cold splitting."); Streamer.emitCFIStartProc(/*IsSimple=*/false); if (Function.getPersonalityFunction() != nullptr) Streamer.emitCFIPersonality(Function.getPersonalityFunction(), Function.getPersonalityEncoding()); - MCSymbol *LSDASymbol = FF.isSplitFragment() ? Function.getColdLSDASymbol() - : Function.getLSDASymbol(); + MCSymbol *LSDASymbol = FF.isSplitFragment() + ? Function.getColdLSDASymbol(FF.getFragmentNum()) + : Function.getLSDASymbol(); if (LSDASymbol) Streamer.emitCFILsda(LSDASymbol, BC.LSDAEncoding); else @@ -383,9 +382,7 @@ if (Function.hasCFI()) Streamer.emitCFIEndProc(); - MCSymbol *EndSymbol = FF.isSplitFragment() - ? Function.getFunctionColdEndLabel() - : Function.getFunctionEndLabel(); + MCSymbol *EndSymbol = Function.getFunctionEndLabel(FF.getFragmentNum()); Streamer.emitLabel(EndSymbol); if (MAI->hasDotTypeDotSizeDirective()) { @@ -903,8 +900,9 @@ Streamer.emitValueToAlignment(TTypeAlignment); // Emit the LSDA label. - MCSymbol *LSDASymbol = - EmitColdPart ? BF.getColdLSDASymbol() : BF.getLSDASymbol(); + MCSymbol *LSDASymbol = EmitColdPart + ? BF.getColdLSDASymbol(FragmentNum::cold()) + : BF.getLSDASymbol(); assert(LSDASymbol && "no LSDA symbol set"); Streamer.emitLabel(LSDASymbol); 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 @@ -1576,8 +1576,8 @@ if (BC.HasRelocations) { for (std::pair &LI : Labels) BC.UndefinedSymbols.insert(LI.second); - if (FunctionEndLabel) - BC.UndefinedSymbols.insert(FunctionEndLabel); + if (!FunctionEndLabels.empty() && FunctionEndLabels.front()) + BC.UndefinedSymbols.insert(FunctionEndLabels.front()); } clearList(Relocations); @@ -2847,8 +2847,8 @@ if (BC.HasRelocations) { for (std::pair &LI : Labels) BC.UndefinedSymbols.insert(LI.second); - if (FunctionEndLabel) - BC.UndefinedSymbols.insert(FunctionEndLabel); + if (!FunctionEndLabels.empty() && FunctionEndLabels.front()) + BC.UndefinedSymbols.insert(FunctionEndLabels.front()); } } @@ -4057,7 +4057,7 @@ const MCSymbol *ColdStartSymbol = getSymbol(FragmentNum::cold()); assert(ColdStartSymbol && ColdStartSymbol->isDefined() && "split function should have defined cold symbol"); - const MCSymbol *ColdEndSymbol = getFunctionColdEndLabel(); + 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); diff --git a/bolt/lib/Core/Exceptions.cpp b/bolt/lib/Core/Exceptions.cpp --- a/bolt/lib/Core/Exceptions.cpp +++ b/bolt/lib/Core/Exceptions.cpp @@ -367,115 +367,97 @@ uint64_t Action; }; - // If previous call can throw, this is its exception handler. - EHInfo PreviousEH = {nullptr, 0}; - - // Marker for the beginning of exceptions range. - const MCSymbol *StartRange = nullptr; - - // Indicates whether the start range is located in a cold part. - bool IsStartInCold = false; - - // Have we crossed hot/cold border for split functions? - bool SeenCold = false; - - // Sites to update - either regular or cold. - CallSitesType *Sites = &CallSites; + for (const FunctionFragment FF : getLayout().fragments()) { + // Sites to update - either regular or cold. + CallSitesType &Sites = FF.isMainFragment() ? CallSites : ColdCallSites; + + // If previous call can throw, this is its exception handler. + EHInfo PreviousEH = {nullptr, 0}; + + // Marker for the beginning of exceptions range. + const MCSymbol *StartRange = nullptr; + + for (BinaryBasicBlock *const BB : FF) { + for (auto II = BB->begin(); II != BB->end(); ++II) { + if (!BC.MIB->isCall(*II)) + continue; + + // Instruction can throw an exception that should be handled. + const bool Throws = BC.MIB->isInvoke(*II); + + // Ignore the call if it's a continuation of a no-throw gap. + if (!Throws && !StartRange) + continue; + + assert(getLayout().isHotColdSplit() && + "Exceptions only supported for hot/cold splitting"); + + // Extract exception handling information from the instruction. + const MCSymbol *LP = nullptr; + uint64_t Action = 0; + if (const Optional EHInfo = + BC.MIB->getEHInfo(*II)) + std::tie(LP, Action) = *EHInfo; + + // No action if the exception handler has not changed. + if (Throws && StartRange && PreviousEH.LP == LP && + PreviousEH.Action == Action) + continue; + + // Same symbol is used for the beginning and the end of the range. + const MCSymbol *EHSymbol; + MCInst EHLabel; + { + std::unique_lock Lock(BC.CtxMutex); + EHSymbol = BC.Ctx->createNamedTempSymbol("EH"); + BC.MIB->createEHLabel(EHLabel, EHSymbol, BC.Ctx.get()); + } - for (BinaryBasicBlock *BB : getLayout().blocks()) { + II = std::next(BB->insertPseudoInstr(II, EHLabel)); + + // At this point we could be in one of the following states: + // + // I. Exception handler has changed and we need to close previous range + // and start a new one. + // + // II. Start a new exception range after the gap. + // + // III. Close current exception range and start a new gap. + const MCSymbol *EndRange; + if (StartRange) { + // I, III: + EndRange = EHSymbol; + } else { + // II: + StartRange = EHSymbol; + EndRange = nullptr; + } - if (BB->isCold() && !SeenCold) { - SeenCold = true; + // Close the previous range. + if (EndRange) { + Sites.emplace_back( + CallSite{StartRange, EndRange, PreviousEH.LP, PreviousEH.Action}); + } - // Close the range (if any) and change the target call sites. - if (StartRange) { - Sites->emplace_back(CallSite{StartRange, getFunctionEndLabel(), - PreviousEH.LP, PreviousEH.Action}); + if (Throws) { + // I, II: + StartRange = EHSymbol; + PreviousEH = EHInfo{LP, Action}; + } else { + StartRange = nullptr; + } } - Sites = &ColdCallSites; - - // Reset the range. - StartRange = nullptr; - PreviousEH = {nullptr, 0}; } - for (auto II = BB->begin(); II != BB->end(); ++II) { - if (!BC.MIB->isCall(*II)) - continue; - - // Instruction can throw an exception that should be handled. - const bool Throws = BC.MIB->isInvoke(*II); - - // Ignore the call if it's a continuation of a no-throw gap. - if (!Throws && !StartRange) - continue; - - // Extract exception handling information from the instruction. - const MCSymbol *LP = nullptr; - uint64_t Action = 0; - if (const Optional EHInfo = BC.MIB->getEHInfo(*II)) - std::tie(LP, Action) = *EHInfo; - - // No action if the exception handler has not changed. - if (Throws && StartRange && PreviousEH.LP == LP && - PreviousEH.Action == Action) - continue; - - // Same symbol is used for the beginning and the end of the range. - const MCSymbol *EHSymbol; - MCInst EHLabel; - { - std::unique_lock Lock(BC.CtxMutex); - EHSymbol = BC.Ctx->createNamedTempSymbol("EH"); - BC.MIB->createEHLabel(EHLabel, EHSymbol, BC.Ctx.get()); - } - - II = std::next(BB->insertPseudoInstr(II, EHLabel)); - - // At this point we could be in one of the following states: - // - // I. Exception handler has changed and we need to close previous range - // and start a new one. - // - // II. Start a new exception range after the gap. - // - // III. Close current exception range and start a new gap. - const MCSymbol *EndRange; - if (StartRange) { - // I, III: - EndRange = EHSymbol; - } else { - // II: - StartRange = EHSymbol; - IsStartInCold = SeenCold; - EndRange = nullptr; - } - - // Close the previous range. - if (EndRange) { - Sites->emplace_back( - CallSite{StartRange, EndRange, PreviousEH.LP, PreviousEH.Action}); - } - - if (Throws) { - // I, II: - StartRange = EHSymbol; - IsStartInCold = SeenCold; - PreviousEH = EHInfo{LP, Action}; - } else { - StartRange = nullptr; - } + // Check if we need to close the range. + if (StartRange) { + assert((FF.isMainFragment() || &Sites == &ColdCallSites) && + "sites mismatch"); + const MCSymbol *EndRange = getFunctionEndLabel(FF.getFragmentNum()); + Sites.emplace_back( + CallSite{StartRange, EndRange, PreviousEH.LP, PreviousEH.Action}); } } - - // Check if we need to close the range. - if (StartRange) { - assert((!isSplit() || Sites == &ColdCallSites) && "sites mismatch"); - const MCSymbol *EndRange = - IsStartInCold ? getFunctionColdEndLabel() : getFunctionEndLabel(); - Sites->emplace_back( - CallSite{StartRange, EndRange, PreviousEH.LP, PreviousEH.Action}); - } } const uint8_t DWARF_CFI_PRIMARY_OPCODE_MASK = 0xc0; diff --git a/bolt/lib/Passes/Aligner.cpp b/bolt/lib/Passes/Aligner.cpp --- a/bolt/lib/Passes/Aligner.cpp +++ b/bolt/lib/Passes/Aligner.cpp @@ -84,7 +84,7 @@ size_t ColdSize = 0; for (const BinaryBasicBlock &BB : Function) - if (BB.isCold()) + if (BB.isSplit()) ColdSize += BC.computeCodeSize(BB.begin(), BB.end(), Emitter); else HotSize += BC.computeCodeSize(BB.begin(), BB.end(), Emitter); diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp --- a/bolt/lib/Passes/BinaryPasses.cpp +++ b/bolt/lib/Passes/BinaryPasses.cpp @@ -592,41 +592,39 @@ for (auto &It : BC.getBinaryFunctions()) { BinaryFunction &BF = It.second; - int64_t CurrentGnuArgsSize = 0; - // Have we crossed hot/cold border for split functions? - bool SeenCold = false; - - for (BinaryBasicBlock *BB : BF.getLayout().blocks()) { - if (BB->isCold() && !SeenCold) { - SeenCold = true; - CurrentGnuArgsSize = 0; - } - - // First convert GnuArgsSize annotations into CFIs. This may change instr - // pointers, so do it before recording ptrs for preserved annotations - if (BF.usesGnuArgsSize()) { - for (auto II = BB->begin(); II != BB->end(); ++II) { - if (!BC.MIB->isInvoke(*II)) - continue; - const int64_t NewGnuArgsSize = BC.MIB->getGnuArgsSize(*II); - assert(NewGnuArgsSize >= 0 && "expected non-negative GNU_args_size"); - if (NewGnuArgsSize != CurrentGnuArgsSize) { - auto InsertII = BF.addCFIInstruction( - BB, II, - MCCFIInstruction::createGnuArgsSize(nullptr, NewGnuArgsSize)); - CurrentGnuArgsSize = NewGnuArgsSize; - II = std::next(InsertII); + for (const FunctionFragment FF : BF.getLayout().fragments()) { + int64_t CurrentGnuArgsSize = 0; + + for (BinaryBasicBlock *const BB : FF) { + // First convert GnuArgsSize annotations into CFIs. This may change + // instr pointers, so do it before recording ptrs for preserved + // annotations + if (BF.usesGnuArgsSize()) { + for (auto II = BB->begin(); II != BB->end(); ++II) { + if (!BC.MIB->isInvoke(*II)) + continue; + const int64_t NewGnuArgsSize = BC.MIB->getGnuArgsSize(*II); + assert(NewGnuArgsSize >= 0 && + "expected non-negative GNU_args_size"); + if (NewGnuArgsSize != CurrentGnuArgsSize) { + auto InsertII = BF.addCFIInstruction( + BB, II, + MCCFIInstruction::createGnuArgsSize(nullptr, NewGnuArgsSize)); + CurrentGnuArgsSize = NewGnuArgsSize; + II = std::next(InsertII); + } } } - } - // Now record preserved annotations separately and then strip annotations. - for (auto II = BB->begin(); II != BB->end(); ++II) { - if (BF.requiresAddressTranslation() && BC.MIB->getOffset(*II)) - PreservedOffsetAnnotations.emplace_back(&(*II), - *BC.MIB->getOffset(*II)); - BC.MIB->stripAnnotations(*II); + // Now record preserved annotations separately and then strip + // annotations. + for (auto II = BB->begin(); II != BB->end(); ++II) { + if (BF.requiresAddressTranslation() && BC.MIB->getOffset(*II)) + PreservedOffsetAnnotations.emplace_back(&(*II), + *BC.MIB->getOffset(*II)); + BC.MIB->stripAnnotations(*II); + } } } } diff --git a/bolt/lib/Passes/IndirectCallPromotion.cpp b/bolt/lib/Passes/IndirectCallPromotion.cpp --- a/bolt/lib/Passes/IndirectCallPromotion.cpp +++ b/bolt/lib/Passes/IndirectCallPromotion.cpp @@ -257,9 +257,9 @@ MCSymbol *Entry = JT->Entries[I]; assert(BF.getBasicBlockForLabel(Entry) || Entry == BF.getFunctionEndLabel() || - Entry == BF.getFunctionColdEndLabel()); + Entry == BF.getFunctionEndLabel(FragmentNum::cold())); if (Entry == BF.getFunctionEndLabel() || - Entry == BF.getFunctionColdEndLabel()) + Entry == BF.getFunctionEndLabel(FragmentNum::cold())) continue; const Location To(Entry); const BinaryBasicBlock::BinaryBranchInfo &BI = BB.getBranchInfo(Entry);