Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable hot/cold splitting of EH funclets #71236

Merged
merged 3 commits into from
Jun 29, 2022
Merged

Enable hot/cold splitting of EH funclets #71236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f5359e6
Enable hot/cold splitting of EH funclets
Jun 23, 2022
957dab5
Removed condition blocking splitting; fixed style
Jun 24, 2022
d71e69f
Merge branch 'dotnet:main' into split-eh
amanasifkhalid Jun 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5256,6 +5256,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);
Expand Down
16 changes: 6 additions & 10 deletions src/coreclr/jit/emitarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When branching to a finally funclet on ARM64, we emit an INS_bl_local instruction -- previously, we could guarantee this jump would stay within the hot section, and thus would set idjShort = true. Now that EH funclets can be moved to the cold section, we can no longer assume this, and must set idjKeepLong based on if we're crossing hot/cold sections. Without changing this logic, we'll hit asserts in emitOutputLJ due to the branch being too short.


#ifdef DEBUG
// Mark the finally call
Expand All @@ -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
{
Expand Down
60 changes: 44 additions & 16 deletions src/coreclr/jit/flowgraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3361,6 +3361,28 @@ void Compiler::fgCreateFunclets()
#endif // DEBUG
}

//------------------------------------------------------------------------
// 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 (!block->isRunRarely())
{
return false;
}
}

return true;
}

#endif // defined(FEATURE_EH_FUNCLETS)

/*-------------------------------------------------------------------------
Expand Down Expand Up @@ -3401,17 +3423,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;
Expand All @@ -3420,8 +3431,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)
Expand Down Expand Up @@ -3456,12 +3467,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
Expand Down
17 changes: 9 additions & 8 deletions src/coreclr/jit/unwind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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));
Expand All @@ -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)
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 1 addition & 6 deletions src/coreclr/jit/unwindamd64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
{
Expand Down
16 changes: 0 additions & 16 deletions src/coreclr/jit/unwindarm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down