Skip to content

Commit

Permalink
[release/9.0] Backport "JIT: Run single EH region repair pass after l…
Browse files Browse the repository at this point in the history 
…ayout" (#108715)

* Add EH region ends pass

* Remove old EH fixup logic

* Style

* Use null EH clause pointers as set indicators

---------

Co-authored-by: Jeff Schwartz <[email protected]>
  • Loading branch information
@amanasifkhalid @jeffschwMSFT
amanasifkhalid and jeffschwMSFT authored Oct 10, 2024
1 parent 05adda5 commit 8ea49ab
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 175 deletions.
5 changes: 2 additions & 3 deletions src/coreclr/jit/compiler.h
View file
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
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
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

0 comments on commit 8ea49ab

Please sign in to comment.