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

Jump to bottom

JIT: Break up try regions in Compiler::fgMoveColdBlocks, and fix contiguity later #108914

Merged
merged 4 commits into from
Oct 18, 2024
+84 −199
Merged

JIT: Break up try regions in Compiler::fgMoveColdBlocks, and fix contiguity later #108914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4232610
Wip
amanasifkhalid Oct 15, 2024
57de512
Clean replay
amanasifkhalid Oct 16, 2024
e9b417d
Remove dead code
amanasifkhalid Oct 16, 2024
41a9a69
Fix comment
amanasifkhalid Oct 18, 2024
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
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5260,7 +5260,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl

if (compHndBBtabCount != 0)
{
fgFindEHRegionEnds();
fgRebuildEHRegions();
}

return PhaseStatus::MODIFIED_EVERYTHING;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2936,7 +2936,7 @@ class Compiler

void fgSetHndEnd(EHblkDsc* handlerTab, BasicBlock* newHndLast);

void fgFindEHRegionEnds();
void fgRebuildEHRegions();

void fgSkipRmvdBlocks(EHblkDsc* handlerTab);

Expand Down
214 changes: 34 additions & 180 deletions src/coreclr/jit/fgopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3348,7 +3348,7 @@ bool Compiler::fgReorderBlocks(bool useProfile)

if (compHndBBtabCount != 0)
{
fgFindEHRegionEnds();
fgRebuildEHRegions();
}

// Renumber blocks to facilitate LSRA's order of block visitation
Expand Down Expand Up @@ -4769,6 +4769,9 @@ void Compiler::fgDoReversePostOrderLayout()
//
// Notes:
// Exception handlers are assumed to be cold, so we won't move blocks within them.
// On platforms that don't use funclets, we should use Compiler::fgRelocateEHRegions to move cold handlers.
// Note that Compiler::fgMoveColdBlocks will break up EH regions to facilitate intermediate transformations.
// To reestablish contiguity of EH regions, callers need to follow this with Compiler::fgRebuildEHRegions.
//
void Compiler::fgMoveColdBlocks()
{
Expand All @@ -4783,212 +4786,63 @@ void Compiler::fgMoveColdBlocks()
}
#endif // DEBUG

auto moveColdMainBlocks = [this]() {
// Find the last block in the main body that isn't part of an EH region
//
BasicBlock* lastMainBB;
for (lastMainBB = this->fgLastBBInMainFunction(); lastMainBB != nullptr; lastMainBB = lastMainBB->Prev())
{
if (!lastMainBB->hasTryIndex() && !lastMainBB->hasHndIndex())
{
break;
}
}

// Nothing to do if there are two or fewer non-EH blocks
//
if ((lastMainBB == nullptr) || lastMainBB->IsFirst() || lastMainBB->PrevIs(fgFirstBB))
{
return;
}

// Search the main method body for rarely-run blocks to move
//
BasicBlock* prev;
for (BasicBlock* block = lastMainBB->Prev(); block != fgFirstBB; block = prev)
auto moveBlock = [this](BasicBlock* block, BasicBlock* insertionPoint) {
assert(block != insertionPoint);
// Don't move handler blocks.
// Also, leave try entries behind as a breadcrumb for where to reinsert try blocks.
if (!bbIsTryBeg(block) && !block->hasHndIndex())
{
prev = block->Prev();

// We only want to move cold blocks.
// Also, don't consider blocks in EH regions for now; only move blocks in the main method body.
// Finally, don't move block if it is the beginning of a call-finally pair,
// as we want to keep these pairs contiguous
// (if we encounter the end of a pair below, we'll move the whole pair).
//
if (!block->isBBWeightCold(this) || block->hasTryIndex() || block->hasHndIndex() ||
block->isBBCallFinallyPair())
{
continue;
}

this->fgUnlinkBlock(block);
this->fgInsertBBafter(lastMainBB, block);

// If block is the end of a call-finally pair, prev is the beginning of the pair.
// Move prev to before block to keep the pair contiguous.
//
if (block->KindIs(BBJ_CALLFINALLYRET))
{
BasicBlock* const callFinally = prev;
prev = prev->Prev();
assert(callFinally->KindIs(BBJ_CALLFINALLY));
assert(!callFinally->HasFlag(BBF_RETLESS_CALL));
this->fgUnlinkBlock(callFinally);
this->fgInsertBBafter(lastMainBB, callFinally);
}
}

// We have moved all cold main blocks before lastMainBB to after lastMainBB.
// If lastMainBB itself is cold, move it to the end of the method to restore its relative ordering.
//
if (lastMainBB->isBBWeightCold(this))
{
BasicBlock* const newLastMainBB = this->fgLastBBInMainFunction();
if (lastMainBB != newLastMainBB)
if (block->isBBCallFinallyPair())
{
BasicBlock* const prev = lastMainBB->Prev();
this->fgUnlinkBlock(lastMainBB);
this->fgInsertBBafter(newLastMainBB, lastMainBB);

// Call-finally check
//
if (lastMainBB->KindIs(BBJ_CALLFINALLYRET))
BasicBlock* const callFinallyRet = block->Next();
if (callFinallyRet != insertionPoint)
{
assert(prev->KindIs(BBJ_CALLFINALLY));
assert(!prev->HasFlag(BBF_RETLESS_CALL));
assert(prev != newLastMainBB);
this->fgUnlinkBlock(prev);
this->fgInsertBBafter(newLastMainBB, prev);
fgUnlinkRange(block, callFinallyRet);
fgMoveBlocksAfter(block, callFinallyRet, insertionPoint);
}
}
else
{
fgUnlinkBlock(block);
fgInsertBBafter(insertionPoint, block);
}
}
};

moveColdMainBlocks();

// No EH regions
//
if (compHndBBtabCount == 0)
BasicBlock* const lastMainBB = fgLastBBInMainFunction();
if (lastMainBB->IsFirst())
{
return;
}

// We assume exception handlers are cold, so we won't bother moving blocks within them.
// We will move blocks only within try regions.
// First, determine where each try region ends, without considering nested regions.
// We will use these end blocks as insertion points.
//
BasicBlock** const tryRegionEnds = new (this, CMK_Generic) BasicBlock* [compHndBBtabCount] {};

for (BasicBlock* const block : Blocks(fgFirstBB, fgLastBBInMainFunction()))
{
if (block->hasTryIndex())
{
tryRegionEnds[block->getTryIndex()] = block;
}
}

// Search all try regions in the main method body for cold blocks to move
// Search the main method body for rarely-run blocks to move
//
BasicBlock* prev;
for (BasicBlock* block = fgLastBBInMainFunction(); block != fgFirstBB; block = prev)
for (BasicBlock *block = lastMainBB->Prev(), *prev; !block->IsFirst(); block = prev)
{
prev = block->Prev();

// Only consider cold blocks in try regions.
// If we have such a block that is also part of an exception handler, don't bother moving it.
// Finally, don't move block if it is the beginning of a call-finally pair,
// We only want to move cold blocks.
// Also, don't move block if it is the end of a call-finally pair,
// as we want to keep these pairs contiguous
// (if we encounter the end of a pair below, we'll move the whole pair).
//
if (!block->isBBWeightCold(this) || !block->hasTryIndex() || block->hasHndIndex() ||
block->isBBCallFinallyPair())
{
continue;
}

const unsigned tryIndex = block->getTryIndex();
EHblkDsc* const HBtab = ehGetDsc(tryIndex);

// Don't move the beginning of a try region.
// Also, if this try region's entry is cold, don't bother moving its blocks.
//
if ((HBtab->ebdTryBeg == block) || HBtab->ebdTryBeg->isBBWeightCold(this))
{
continue;
}

BasicBlock* const insertionPoint = tryRegionEnds[tryIndex];
assert(insertionPoint != nullptr);

// Don't move the end of this try region
// (if we encounter the beginning of a pair, we'll move the whole pair).
//
if (block == insertionPoint)
if (!block->isBBWeightCold(this) || block->isBBCallFinallyPairTail())
{
continue;
}

fgUnlinkBlock(block);
fgInsertBBafter(insertionPoint, block);

// Keep call-finally pairs contiguous
//
if (block->KindIs(BBJ_CALLFINALLYRET))
{
BasicBlock* const callFinally = prev;
prev = prev->Prev();
assert(callFinally->KindIs(BBJ_CALLFINALLY));
assert(!callFinally->HasFlag(BBF_RETLESS_CALL));
fgUnlinkBlock(callFinally);
fgInsertBBafter(insertionPoint, callFinally);
}
moveBlock(block, lastMainBB);
}

// Find the new try region ends
// We have moved all cold main blocks before lastMainBB to after lastMainBB.
// If lastMainBB itself is cold, move it to the end of the method to restore its relative ordering.
//
for (unsigned XTnum = 0; XTnum < compHndBBtabCount; XTnum++)
if (lastMainBB->isBBWeightCold(this) && !lastMainBB->isBBCallFinallyPairTail())
{
BasicBlock* const tryEnd = tryRegionEnds[XTnum];

// This try region isn't in the main method body
//
if (tryEnd == nullptr)
{
continue;
}

// If we moved cold blocks to the end of this try region,
// search for the new end block
//
BasicBlock* newTryEnd = tryEnd;
for (BasicBlock* const block : Blocks(tryEnd, fgLastBBInMainFunction()))
BasicBlock* const newLastMainBB = fgLastBBInMainFunction();
if (lastMainBB != newLastMainBB)
{
if (!BasicBlock::sameTryRegion(tryEnd, block))
{
break;
}

newTryEnd = block;
}

// We moved cold blocks to the end of this try region, but the old end block is cold, too.
// Move the old end block to the end of the region to preserve its relative ordering.
//
if ((tryEnd != newTryEnd) && !tryEnd->hasHndIndex() && tryEnd->isBBWeightCold(this))
{
BasicBlock* const prev = tryEnd->Prev();
fgUnlinkBlock(tryEnd);
fgInsertBBafter(newTryEnd, tryEnd);

// Keep call-finally pairs contiguous
//
if (tryEnd->KindIs(BBJ_CALLFINALLYRET))
{
assert(prev->KindIs(BBJ_CALLFINALLY));
assert(!prev->HasFlag(BBF_RETLESS_CALL));
fgUnlinkBlock(prev);
fgInsertBBafter(newTryEnd, prev);
}
moveBlock(lastMainBB, newLastMainBB);
}
}
}
Expand Down
65 changes: 48 additions & 17 deletions src/coreclr/jit/jiteh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1254,22 +1254,62 @@ void Compiler::fgSetHndEnd(EHblkDsc* handlerTab, BasicBlock* newHndLast)
}

//-------------------------------------------------------------
// fgFindEHRegionEnds: Walk the block list, and set each try/handler region's end block.
// fgRebuildEHRegions: After reordering blocks, make EH regions contiguous
// while maintaining relative block order, and update each region's end pointer.
//
void Compiler::fgFindEHRegionEnds()
void Compiler::fgRebuildEHRegions()
{
assert(compHndBBtabCount != 0);
unsigned unsetTryEnds = compHndBBtabCount;
unsigned unsetHndEnds = compHndBBtabCount;

// Null out each clause's end pointers.
// A non-null end pointer indicates we already updated the clause.
// Null out try/handler end pointers to signify the given clause hasn't been visited yet.
for (EHblkDsc* const HBtab : EHClauses(this))
{
HBtab->ebdTryLast = nullptr;
HBtab->ebdHndLast = nullptr;
}

// Walk the main method body, and move try blocks to re-establish contiguity.
for (BasicBlock *block = fgFirstBB, *next; block != fgFirstFuncletBB; block = next)
{
next = block->Next();
EHblkDsc* HBtab = ehGetBlockTryDsc(block);
if (HBtab != nullptr)
{
// Move this block up to the previous block in the same try region.
BasicBlock* const insertionPoint = HBtab->ebdTryLast;
if ((insertionPoint != nullptr) && !insertionPoint->NextIs(block))
{
assert(block != HBtab->ebdTryLast);
fgUnlinkBlock(block);
fgInsertBBafter(HBtab->ebdTryLast, block);
}

// Update this try region's (and all parent try regions') end pointer with the last block visited
for (unsigned XTnum = block->getTryIndex(); XTnum != EHblkDsc::NO_ENCLOSING_INDEX;
XTnum = ehGetEnclosingTryIndex(XTnum))
{
HBtab = ehGetDsc(XTnum);
if (HBtab->ebdTryLast == nullptr)
{
assert(HBtab->ebdTryBeg == block);
assert(unsetTryEnds != 0);
unsetTryEnds--;
HBtab->ebdTryLast = block;
}
else if (HBtab->ebdTryLast->NextIs(block))
{
HBtab->ebdTryLast = block;
}
}
}
}

// The above logic rebuilt the try regions in the main method body.
// Now, resolve the regions in the funclet section, if there is one.
assert((unsetTryEnds == 0) || (fgFirstFuncletBB != nullptr));

// Updates the try region's (and all of its parent regions') end block to 'block,'
// if the try region's end block hasn't been updated yet.
auto setTryEnd = [this, &unsetTryEnds](BasicBlock* block) {
Expand Down Expand Up @@ -1310,19 +1350,7 @@ void Compiler::fgFindEHRegionEnds()
}
};

// Iterate backwards through the main method body, and update each try region's end block
for (BasicBlock* block = fgLastBBInMainFunction(); (unsetTryEnds != 0) && (block != nullptr); block = block->Prev())
{
if (block->hasTryIndex())
{
setTryEnd(block);
}
}

// If we don't have a funclet section, then all of the try regions should have been updated above
assert((unsetTryEnds == 0) || (fgFirstFuncletBB != nullptr));

// If we do have a funclet section, update the ends of any try regions nested in funclets
// If we have a funclet section, update the ends of any try regions nested in funclets
for (BasicBlock* block = fgLastBB; (unsetTryEnds != 0) && (block != fgLastBBInMainFunction());
block = block->Prev())
{
Expand All @@ -1340,6 +1368,9 @@ void Compiler::fgFindEHRegionEnds()
setHndEnd(block);
}
}

assert(unsetTryEnds == 0);
assert(unsetHndEnds == 0);
}

/*****************************************************************************
Expand Down
Loading