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

[release/9.0] Backport "JIT: Run single EH region repair pass after layout" #108715

Merged
merged 5 commits into from
Oct 10, 2024
Merged

[release/9.0] Backport "JIT: Run single EH region repair pass after layout" #108715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
27d8ca1
Add EH region ends pass
amanasifkhalid Oct 8, 2024
85646f5
Remove old EH fixup logic
amanasifkhalid Oct 8, 2024
1bb52ec
Style
amanasifkhalid Oct 8, 2024
d254188
Use null EH clause pointers as set indicators
amanasifkhalid Oct 8, 2024
2ef148c
Merge branch 'release/9.0' into backport-108634
jeffschwMSFT Oct 10, 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
5 changes: 2 additions & 3 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2829,9 +2829,6 @@ class Compiler
EHblkDsc* ehIsBlockHndLast(BasicBlock* block);
bool ehIsBlockEHLast(BasicBlock* block);

template <typename GetTryLast, typename SetTryLast>
void ehUpdateTryLasts(GetTryLast getTryLast, SetTryLast setTryLast);

bool ehBlockHasExnFlowDsc(BasicBlock* block);

// Return the region index of the most nested EH region this block is in.
Expand Down Expand Up @@ -2938,6 +2935,8 @@ class Compiler

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

void fgFindEHRegionEnds();

void fgSkipRmvdBlocks(EHblkDsc* handlerTab);

void fgAllocEHTable();
Expand Down
178 changes: 6 additions & 172 deletions src/coreclr/jit/fgopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3357,6 +3357,11 @@ bool Compiler::fgReorderBlocks(bool useProfile)
fgDoReversePostOrderLayout();
fgMoveColdBlocks();

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

// Renumber blocks to facilitate LSRA's order of block visitation
// TODO: Consider removing this, and using traversal order in lSRA
//
Expand Down Expand Up @@ -4702,22 +4707,6 @@ void Compiler::fgDoReversePostOrderLayout()
return;
}

// The RPO will scramble the EH regions, requiring us to correct their state.
// To do so, we will need to determine the new end blocks of each region.
//
struct EHLayoutInfo
{
BasicBlock* tryRegionEnd;
BasicBlock* hndRegionEnd;
bool tryRegionInMainBody;

// Default constructor provided so we can call ArrayStack::Emplace
//
EHLayoutInfo() = default;
};

ArrayStack<EHLayoutInfo> regions(getAllocator(CMK_ArrayStack), compHndBBtabCount);

// The RPO will break up call-finally pairs, so save them before re-ordering
//
struct CallFinallyPair
Expand All @@ -4738,9 +4727,6 @@ void Compiler::fgDoReversePostOrderLayout()

for (EHblkDsc* const HBtab : EHClauses(this))
{
// Default-initialize a EHLayoutInfo for each EH clause
regions.Emplace();

if (HBtab->HasFinallyHandler())
{
for (BasicBlock* const pred : HBtab->ebdHndBeg->PredBlocks())
Expand Down Expand Up @@ -4787,89 +4773,6 @@ void Compiler::fgDoReversePostOrderLayout()
}

fgMoveHotJumps</* hasEH */ true>();

// The RPO won't change the entry blocks of any EH regions, but reordering can change the last block in a region
// (for example, by pushing throw blocks unreachable via normal flow to the end of the region).
// First, determine the new EH region ends.
//

for (BasicBlock* const block : Blocks(fgFirstBB, fgLastBBInMainFunction()))
{
if (block->hasTryIndex())
{
EHLayoutInfo& layoutInfo = regions.BottomRef(block->getTryIndex());
layoutInfo.tryRegionEnd = block;
layoutInfo.tryRegionInMainBody = true;
}

if (block->hasHndIndex())
{
regions.BottomRef(block->getHndIndex()).hndRegionEnd = block;
}
}

for (BasicBlock* const block : Blocks(fgFirstFuncletBB))
{
if (block->hasHndIndex())
{
regions.BottomRef(block->getHndIndex()).hndRegionEnd = block;
}

if (block->hasTryIndex())
{
EHLayoutInfo& layoutInfo = regions.BottomRef(block->getTryIndex());

if (!layoutInfo.tryRegionInMainBody)
{
layoutInfo.tryRegionEnd = block;
}
}
}

// Now, update the EH descriptors, starting with the try regions
//
auto getTryLast = [&regions](const unsigned index) -> BasicBlock* {
return regions.BottomRef(index).tryRegionEnd;
};

auto setTryLast = [&regions](const unsigned index, BasicBlock* const block) {
regions.BottomRef(index).tryRegionEnd = block;
};

ehUpdateTryLasts<decltype(getTryLast), decltype(setTryLast)>(getTryLast, setTryLast);

// Now, do the handler regions
//
unsigned XTnum = 0;
for (EHblkDsc* const HBtab : EHClauses(this))
{
// The end of each handler region should have been visited by iterating the blocklist above
//
BasicBlock* const hndEnd = regions.BottomRef(XTnum++).hndRegionEnd;
assert(hndEnd != nullptr);

// Update the end pointer of this handler region to the new last block
//
HBtab->ebdHndLast = hndEnd;
const unsigned enclosingHndIndex = HBtab->ebdEnclosingHndIndex;

// If this handler region is nested in another one, we might need to update its enclosing region's end block
//
if (enclosingHndIndex != EHblkDsc::NO_ENCLOSING_INDEX)
{
BasicBlock* const enclosingHndEnd = regions.BottomRef(enclosingHndIndex).hndRegionEnd;
assert(enclosingHndEnd != nullptr);

// If the enclosing region ends right before the nested region begins,
// extend the enclosing region's last block to the end of the nested region.
//
BasicBlock* const hndBeg = HBtab->HasFilter() ? HBtab->ebdFilter : HBtab->ebdHndBeg;
if (enclosingHndEnd->NextIs(hndBeg))
{
regions.BottomRef(enclosingHndIndex).hndRegionEnd = hndEnd;
}
}
}
}

//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -5052,7 +4955,7 @@ void Compiler::fgMoveColdBlocks()
}
}

// Before updating EH descriptors, find the new try region ends
// Find the new try region ends
//
for (unsigned XTnum = 0; XTnum < compHndBBtabCount; XTnum++)
{
Expand Down Expand Up @@ -5098,75 +5001,6 @@ void Compiler::fgMoveColdBlocks()
fgInsertBBafter(newTryEnd, prev);
}
}
else
{
// Otherwise, just update the try region end
//
tryRegionEnds[XTnum] = newTryEnd;
}
}

// Now, update EH descriptors
//
auto getTryLast = [tryRegionEnds](const unsigned index) -> BasicBlock* {
return tryRegionEnds[index];
};

auto setTryLast = [tryRegionEnds](const unsigned index, BasicBlock* const block) {
tryRegionEnds[index] = block;
};

ehUpdateTryLasts<decltype(getTryLast), decltype(setTryLast)>(getTryLast, setTryLast);
}

//-------------------------------------------------------------
// ehUpdateTryLasts: Iterates EH descriptors, updating each try region's
// end block as determined by getTryLast.
//
// Type parameters:
// GetTryLast - Functor type that takes an EH index,
// and returns the corresponding region's new try end block
// SetTryLast - Functor type that takes an EH index and a BasicBlock*,
// and updates some internal state tracking the new try end block of each EH region
//
// Parameters:
// getTryLast - Functor to get new try end block for an EH region
// setTryLast - Functor to update the new try end block for an EH region
//
template <typename GetTryLast, typename SetTryLast>
void Compiler::ehUpdateTryLasts(GetTryLast getTryLast, SetTryLast setTryLast)
{
unsigned XTnum = 0;
for (EHblkDsc* const HBtab : EHClauses(this))
{
BasicBlock* const tryEnd = getTryLast(XTnum++);

if (tryEnd == nullptr)
{
continue;
}

// Update the end pointer of this try region to the new last block
//
HBtab->ebdTryLast = tryEnd;
const unsigned enclosingTryIndex = HBtab->ebdEnclosingTryIndex;

// If this try region is nested in another one, we might need to update its enclosing region's end block
//
if (enclosingTryIndex != EHblkDsc::NO_ENCLOSING_INDEX)
{
BasicBlock* const enclosingTryEnd = getTryLast(enclosingTryIndex);

// If multiple EH descriptors map to the same try region,
// then the enclosing region's last block might be null in the table, so set it here.
// Similarly, if the enclosing region ends right before the nested region begins,
// extend the enclosing region's last block to the end of the nested region.
//
if ((enclosingTryEnd == nullptr) || enclosingTryEnd->NextIs(HBtab->ebdTryBeg))
{
setTryLast(enclosingTryIndex, tryEnd);
}
}
}
}

Expand Down
89 changes: 89 additions & 0 deletions src/coreclr/jit/jiteh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1302,6 +1302,95 @@ void Compiler::fgSetHndEnd(EHblkDsc* handlerTab, BasicBlock* newHndLast)
}
}

//-------------------------------------------------------------
// fgFindEHRegionEnds: Walk the block list, and set each try/handler region's end block.
//
void Compiler::fgFindEHRegionEnds()
{
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.
for (EHblkDsc* const HBtab : EHClauses(this))
{
HBtab->ebdTryLast = nullptr;
HBtab->ebdHndLast = 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) {
for (unsigned tryIndex = block->getTryIndex(); tryIndex != EHblkDsc::NO_ENCLOSING_INDEX;
tryIndex = ehGetEnclosingTryIndex(tryIndex))
{
EHblkDsc* const HBtab = ehGetDsc(tryIndex);
if (HBtab->ebdTryLast == nullptr)
{
assert(unsetTryEnds != 0);
HBtab->ebdTryLast = block;
unsetTryEnds--;
}
else
{
break;
}
}
};

// Updates the handler region's (and all of its parent regions') end block to 'block,'
// if the handler region's end block hasn't been updated yet.
auto setHndEnd = [this, &unsetHndEnds](BasicBlock* block) {
for (unsigned hndIndex = block->getHndIndex(); hndIndex != EHblkDsc::NO_ENCLOSING_INDEX;
hndIndex = ehGetEnclosingHndIndex(hndIndex))
{
EHblkDsc* const HBtab = ehGetDsc(hndIndex);
if (HBtab->ebdHndLast == nullptr)
{
assert(unsetHndEnds != 0);
HBtab->ebdHndLast = block;
unsetHndEnds--;
}
else
{
break;
}
}
};

// 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
for (BasicBlock* block = fgLastBB; (unsetTryEnds != 0) && (block != fgLastBBInMainFunction());
block = block->Prev())
{
if (block->hasTryIndex())
{
setTryEnd(block);
}
}

// Finally, update the handler regions' ends
for (BasicBlock* block = fgLastBB; (unsetHndEnds != 0) && (block != nullptr); block = block->Prev())
{
if (block->hasHndIndex())
{
setHndEnd(block);
}
}
}

/*****************************************************************************
*
* Given a EH handler table entry update the ebdTryLast and ebdHndLast pointers
Expand Down
Loading