From f5359e68c776f0657087c3b4b7744f31ea3b1c49 Mon Sep 17 00:00:00 2001 From: Aman Khalid Date: Thu, 23 Jun 2022 15:23:32 -0700 Subject: [PATCH 1/2] Enable hot/cold splitting of EH funclets --- src/coreclr/jit/compiler.h | 4 ++ src/coreclr/jit/emitarm64.cpp | 16 +++----- src/coreclr/jit/flowgraph.cpp | 66 +++++++++++++++++++++++++-------- src/coreclr/jit/unwind.cpp | 17 +++++---- src/coreclr/jit/unwindamd64.cpp | 7 +--- src/coreclr/jit/unwindarm.cpp | 16 -------- 6 files changed, 70 insertions(+), 56 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index b5bfdf926f2126..cf3c99d7f48d95 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5257,6 +5257,10 @@ class Compiler bool fgReorderBlocks(bool useProfile); +#ifdef FEATURE_EH_FUNCLETS + bool fgFuncletsAreCold(); +#endif // FEATURE_EH_FUNCLETS + PhaseStatus fgDetermineFirstColdBlock(); bool fgIsForwardBranch(BasicBlock* bJump, BasicBlock* bSrc = nullptr); diff --git a/src/coreclr/jit/emitarm64.cpp b/src/coreclr/jit/emitarm64.cpp index b0bf1769d1bd09..287a14c6d94350 100644 --- a/src/coreclr/jit/emitarm64.cpp +++ b/src/coreclr/jit/emitarm64.cpp @@ -8434,12 +8434,9 @@ void emitter::emitIns_J(instruction ins, BasicBlock* dst, int instrCount) /* Figure out the encoding format of the instruction */ - bool idjShort = false; switch (ins) { case INS_bl_local: - idjShort = true; - FALLTHROUGH; case INS_b: // Unconditional jump is a single form. // Assume is long in case we cross hot/cold sections. @@ -8473,7 +8470,7 @@ void emitter::emitIns_J(instruction ins, BasicBlock* dst, int instrCount) id->idIns(ins); id->idInsFmt(fmt); - id->idjShort = idjShort; + id->idjShort = false; #ifdef DEBUG // Mark the finally call @@ -8489,15 +8486,14 @@ void emitter::emitIns_J(instruction ins, BasicBlock* dst, int instrCount) // Skip unconditional jump that has a single form. // The target needs to be relocated. - if (!idjShort) - { - id->idjKeepLong = emitComp->fgInDifferentRegions(emitComp->compCurBB, dst); + id->idjKeepLong = emitComp->fgInDifferentRegions(emitComp->compCurBB, dst); #ifdef DEBUG - if (emitComp->opts.compLongAddress) // Force long branches - id->idjKeepLong = 1; -#endif // DEBUG + if (emitComp->opts.compLongAddress) // Force long branches + { + id->idjKeepLong = true; } +#endif // DEBUG } else { diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 33d6fab81e5aef..e5c4189d8d166f 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -3354,6 +3354,34 @@ void Compiler::fgCreateFunclets() #endif // DEBUG } +/*------------------------------------------------------------------------- + * + * Walk the EH funclet blocks of a function to determine if the funclet + * section is cold. If any of the funclets are hot, then it may not be + * beneficial to split at fgFirstFuncletBB, moving all funclets to the + * cold section. + */ + +bool Compiler::fgFuncletsAreCold() +{ + for (BasicBlock* block = fgFirstFuncletBB; block != nullptr; block = block->bbNext) + { +#if HANDLER_ENTRY_MUST_BE_IN_HOT_SECTION + if (bbIsHandlerBeg(block)) + { + return false; + } +#endif // HANDLER_ENTRY_MUST_BE_IN_HOT_SECTION + + if (!block->isRunRarely()) + { + return false; + } + } + + return true; +} + #endif // defined(FEATURE_EH_FUNCLETS) /*------------------------------------------------------------------------- @@ -3394,17 +3422,6 @@ PhaseStatus Compiler::fgDetermineFirstColdBlock() } #endif // DEBUG -#if defined(FEATURE_EH_FUNCLETS) - // TODO-CQ: handle hot/cold splitting in functions with EH (including synchronized methods - // that create EH in methods without explicit EH clauses). - - if (compHndBBtabCount > 0) - { - JITDUMP("No procedure splitting will be done for this method with EH (implementation limitation)\n"); - return PhaseStatus::MODIFIED_NOTHING; - } -#endif // FEATURE_EH_FUNCLETS - BasicBlock* firstColdBlock = nullptr; BasicBlock* prevToFirstColdBlock = nullptr; BasicBlock* block; @@ -3413,8 +3430,8 @@ PhaseStatus Compiler::fgDetermineFirstColdBlock() bool forceSplit = false; #ifdef DEBUG - // If stress-splitting, split right after the first block; don't handle functions with EH - forceSplit = JitConfig.JitStressProcedureSplitting() && (compHndBBtabCount == 0); + // If stress-splitting, split right after the first block + forceSplit = JitConfig.JitStressProcedureSplitting(); #endif if (forceSplit) @@ -3449,12 +3466,29 @@ PhaseStatus Compiler::fgDetermineFirstColdBlock() prevToFirstColdBlock = nullptr; } } - else // (firstColdBlock == NULL) + else // (firstColdBlock == NULL) -- we don't have a candidate for first cold block { - // We don't have a candidate for first cold block + +#ifdef FEATURE_EH_FUNCLETS + // + // If a function has exception handling and we haven't found the first cold block yet, + // consider splitting at the first funclet; do not consider splitting between funclets, + // as this may break unwind info. + // + if (block == fgFirstFuncletBB) + { + if (fgFuncletsAreCold()) + { + firstColdBlock = block; + prevToFirstColdBlock = lblk; + } + + break; + } +#endif // FEATURE_EH_FUNCLETS // Is this a cold block? - if (!blockMustBeInHotSection && (block->isRunRarely() == true)) + if (!blockMustBeInHotSection && block->isRunRarely()) { // // If the last block that was hot was a BBJ_COND diff --git a/src/coreclr/jit/unwind.cpp b/src/coreclr/jit/unwind.cpp index bf19631a44d098..5cae88b36e3699 100644 --- a/src/coreclr/jit/unwind.cpp +++ b/src/coreclr/jit/unwind.cpp @@ -66,13 +66,18 @@ void Compiler::unwindGetFuncLocations(FuncInfoDsc* func, if (fgFirstColdBlock != nullptr) { - // The hot section only goes up to the cold section - assert(fgFirstFuncletBB == nullptr); - #ifdef DEBUG + // If fake-splitting, "trick" VM by pretending entire function is hot. if (JitConfig.JitFakeProcedureSplitting()) { - *ppEndLoc = nullptr; // If fake-splitting, "trick" VM by pretending entire function is hot. + if (fgFirstFuncletBB != nullptr) + { + *ppEndLoc = new (this, CMK_UnwindInfo) emitLocation(ehEmitCookie(fgFirstFuncletBB)); + } + else + { + *ppEndLoc = nullptr; + } } else #endif // DEBUG @@ -94,7 +99,6 @@ void Compiler::unwindGetFuncLocations(FuncInfoDsc* func, } else { - assert(fgFirstFuncletBB == nullptr); // TODO-CQ: support hot/cold splitting in functions with EH assert(fgFirstColdBlock != nullptr); // There better be a cold section! *ppStartLoc = new (this, CMK_UnwindInfo) emitLocation(ehEmitCookie(fgFirstColdBlock)); @@ -103,8 +107,6 @@ void Compiler::unwindGetFuncLocations(FuncInfoDsc* func, } else { - assert(getHotSectionData); // TODO-CQ: support funclets in cold section - EHblkDsc* HBtab = ehGetDsc(func->funEHIndex); if (func->funKind == FUNC_FILTER) @@ -308,7 +310,6 @@ void Compiler::unwindEmitFuncCFI(FuncInfoDsc* func, void* pHotCode, void* pColdC if (pColdCode != nullptr) { assert(fgFirstColdBlock != nullptr); - assert(func->funKind == FUNC_ROOT); // No splitting of funclets. unwindCodeBytes = 0; pUnwindBlock = nullptr; diff --git a/src/coreclr/jit/unwindamd64.cpp b/src/coreclr/jit/unwindamd64.cpp index 88cefbe31ed5e9..ff9a5094cb0ae1 100644 --- a/src/coreclr/jit/unwindamd64.cpp +++ b/src/coreclr/jit/unwindamd64.cpp @@ -661,11 +661,7 @@ void Compiler::unwindReserveFunc(FuncInfoDsc* func) if (fgFirstColdBlock != nullptr) { #ifdef DEBUG - if (JitConfig.JitFakeProcedureSplitting()) - { - assert(func->funKind == FUNC_ROOT); // No splitting of funclets. - } - else + if (!JitConfig.JitFakeProcedureSplitting()) #endif // DEBUG { unwindReserveFuncHelper(func, false); @@ -814,7 +810,6 @@ void Compiler::unwindEmitFuncHelper(FuncInfoDsc* func, void* pHotCode, void* pCo else { assert(fgFirstColdBlock != nullptr); - assert(func->funKind == FUNC_ROOT); // No splitting of funclets. if (func->coldStartLoc == nullptr) { diff --git a/src/coreclr/jit/unwindarm.cpp b/src/coreclr/jit/unwindarm.cpp index 8a14c6edbb8324..aaa6dd3171522e 100644 --- a/src/coreclr/jit/unwindarm.cpp +++ b/src/coreclr/jit/unwindarm.cpp @@ -593,8 +593,6 @@ void Compiler::unwindReserveFunc(FuncInfoDsc* func) if (funcHasColdSection) { - assert(!isFunclet); // TODO-CQ: support hot/cold splitting with EH - emitLocation* startLoc; emitLocation* endLoc; unwindGetFuncLocations(func, false, &startLoc, &endLoc); @@ -1609,20 +1607,6 @@ void UnwindFragmentInfo::Allocate( UNATIVE_OFFSET endOffset; UNATIVE_OFFSET codeSize; -// We don't support hot/cold splitting with EH, so if there is cold code, this -// better not be a funclet! -// TODO-CQ: support funclets in cold code -#ifdef DEBUG - if (JitConfig.JitFakeProcedureSplitting() && (pColdCode != NULL)) - { - noway_assert(isHotCode && (funKind == CORJIT_FUNC_ROOT)); - } - else -#endif // DEBUG - { - noway_assert(isHotCode || (funKind == CORJIT_FUNC_ROOT)); - } - // Compute the final size, and start and end offsets of the fragment startOffset = GetStartOffset(); From 957dab58c5f80b91b5e77330788ea631601e5384 Mon Sep 17 00:00:00 2001 From: Aman Khalid Date: Fri, 24 Jun 2022 10:07:50 -0700 Subject: [PATCH 2/2] Removed condition blocking splitting; fixed style --- src/coreclr/jit/flowgraph.cpp | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index e5c4189d8d166f..4e28b56c3995d0 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -3354,25 +3354,19 @@ void Compiler::fgCreateFunclets() #endif // DEBUG } -/*------------------------------------------------------------------------- - * - * Walk the EH funclet blocks of a function to determine if the funclet - * section is cold. If any of the funclets are hot, then it may not be - * beneficial to split at fgFirstFuncletBB, moving all funclets to the - * cold section. - */ - +//------------------------------------------------------------------------ +// fgFuncletsAreCold: Determine if EH funclets can be moved to cold section. +// +// Notes: +// Walk the EH funclet blocks of a function to determine if the funclet +// section is cold. If any of the funclets are hot, then it may not be +// beneficial to split at fgFirstFuncletBB and move all funclets to +// the cold section. +// bool Compiler::fgFuncletsAreCold() { for (BasicBlock* block = fgFirstFuncletBB; block != nullptr; block = block->bbNext) { -#if HANDLER_ENTRY_MUST_BE_IN_HOT_SECTION - if (bbIsHandlerBeg(block)) - { - return false; - } -#endif // HANDLER_ENTRY_MUST_BE_IN_HOT_SECTION - if (!block->isRunRarely()) { return false;