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

JIT: Simplify block insertion when cloning finally regions #107585

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+56 −90
Open

JIT: Simplify block insertion when cloning finally regions #107585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2833,7 +2833,7 @@ class Compiler
bool ehIsBlockEHLast(BasicBlock* block);

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

bool ehBlockHasExnFlowDsc(BasicBlock* block);

Expand All @@ -2858,6 +2858,10 @@ class Compiler
// Update the 'last' pointers in the EH table to reflect new or deleted blocks in an EH region.
void ehUpdateLastBlocks(BasicBlock* oldLast, BasicBlock* newLast);

// Update the end pointer of the try region containing 'oldTryLast',
// as well as the end pointers of any parent try regions, to 'newTryLast'.
void ehUpdateLastTryBlocks(BasicBlock* oldTryLast, BasicBlock* newTryLast);

// For a finally handler, find the region index that the BBJ_CALLFINALLY lives in that calls the handler,
// or NO_ENCLOSING_INDEX if the BBJ_CALLFINALLY lives in the main function body. Normally, the index
// is the same index as the handler (and the BBJ_CALLFINALLY lives in the 'try' region), but for AMD64 the
Expand Down
14 changes: 3 additions & 11 deletions src/coreclr/jit/fgbasic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6736,23 +6736,15 @@ BasicBlock* Compiler::fgNewBBinRegionWorker(BBKinds jumpKind,
//
BasicBlock* Compiler::fgNewBBatTryRegionEnd(BBKinds jumpKind, unsigned tryIndex)
{
BasicBlock* const oldTryLast = ehGetDsc(tryIndex)->ebdTryLast;
EHblkDsc* HBtab = ehGetDsc(tryIndex);
BasicBlock* const oldTryLast = HBtab->ebdTryLast;
BasicBlock* const newBlock = fgNewBBafter(jumpKind, oldTryLast, /* extendRegion */ false);
newBlock->setTryIndex(tryIndex);
newBlock->clearHndIndex();

// Update this try region's (and all parent try regions') last block pointer
//
for (unsigned XTnum = tryIndex; XTnum < compHndBBtabCount; XTnum++)
{
EHblkDsc* const HBtab = ehGetDsc(XTnum);
if (HBtab->ebdTryLast == oldTryLast)
{
assert((XTnum == tryIndex) || (ehGetDsc(tryIndex)->ebdEnclosingTryIndex != EHblkDsc::NO_ENCLOSING_INDEX));
fgSetTryEnd(HBtab, newBlock);
}
}

ehUpdateLastTryBlocks(oldTryLast, newBlock);
return newBlock;
}

Expand Down
50 changes: 18 additions & 32 deletions src/coreclr/jit/fgehopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1009,44 +1009,18 @@ PhaseStatus Compiler::fgCloneFinally()
// Clone the finally body, and splice it into the flow graph
// within the parent region of the try.
//
const unsigned finallyTryIndex = firstBlock->bbTryIndex;
BasicBlock* insertAfter = nullptr;
BlockToBlockMap blockMap(getAllocator());
unsigned cloneBBCount = 0;
weight_t const originalWeight = firstBlock->hasProfileWeight() ? firstBlock->bbWeight : BB_ZERO_WEIGHT;
bool reachedEnd = false;

for (BasicBlock* block = firstBlock; block != nextBlock; block = block->Next())
for (BasicBlock* block = firstBlock; !reachedEnd; block = block->Next())
{
BasicBlock* newBlock;

if (block == firstBlock)
{
// Put first cloned finally block into the appropriate
// region, somewhere within or after the range of
// callfinallys, depending on the EH implementation.
const unsigned hndIndex = 0;
BasicBlock* const nearBlk = cloneInsertAfter;
newBlock = fgNewBBinRegion(BBJ_ALWAYS, finallyTryIndex, hndIndex, nearBlk);

// If the clone ends up just after the finally, adjust
// the stopping point for finally traversal.
if (newBlock->NextIs(nextBlock))
{
assert(newBlock->PrevIs(lastBlock));
nextBlock = newBlock;
}
}
else
{
// Put subsequent blocks in the same region...
const bool extendRegion = true;
newBlock = fgNewBBafter(BBJ_ALWAYS, insertAfter, extendRegion);
}

BasicBlock* newBlock = fgNewBBafter(BBJ_ALWAYS, cloneInsertAfter, /* extendRegion */ false);
cloneInsertAfter = newBlock;
cloneBBCount++;
assert(cloneBBCount <= regionBBCount);

insertAfter = newBlock;
blockMap.Set(block, newBlock);

BasicBlock::CloneBlockState(this, newBlock, block);
Expand All @@ -1062,10 +1036,11 @@ PhaseStatus Compiler::fgCloneFinally()
{
// Mark the block as the end of the cloned finally.
newBlock->SetFlags(BBF_CLONED_FINALLY_END);
reachedEnd = true;
}

// Make sure clone block state hasn't munged the try region.
assert(newBlock->bbTryIndex == finallyTryIndex);
assert(newBlock->bbTryIndex == firstBlock->bbTryIndex);

// Cloned handler block is no longer within the handler.
newBlock->clearHndIndex();
Expand All @@ -1075,6 +1050,17 @@ PhaseStatus Compiler::fgCloneFinally()
assert(!newBlock->HasInitializedTarget());
}

// If the cloned blocks were inserted at the end of a try region, update the region's end pointer
if (firstBlock->hasTryIndex())
{
BasicBlock* const oldLastTry = ehGetDsc(firstBlock->getTryIndex())->ebdTryLast;
if (!oldLastTry->IsLast() && BasicBlock::sameTryRegion(oldLastTry, oldLastTry->Next()))
{
assert(oldLastTry->NextIs(blockMap[firstBlock]));
ehUpdateLastTryBlocks(oldLastTry, blockMap[lastBlock]);
}
}

// We should have cloned all the finally region blocks.
assert(cloneBBCount == regionBBCount);

Expand All @@ -1084,7 +1070,7 @@ PhaseStatus Compiler::fgCloneFinally()
// Redirect any branches within the newly-cloned
// finally, and any finally returns to jump to the return
// point.
for (BasicBlock* block = firstBlock; block != nextBlock; block = block->Next())
for (BasicBlock* block = firstBlock; !lastBlock->NextIs(block); block = block->Next())
{
BasicBlock* newBlock = blockMap[block];
// Jump kind/target should not be set yet
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/jit/fgopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4828,7 +4828,7 @@ void Compiler::fgDoReversePostOrderLayout()
regions.BottomRef(index).tryRegionEnd = block;
};

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

// Now, do the handler regions
//
Expand Down Expand Up @@ -5108,11 +5108,11 @@ void Compiler::fgMoveColdBlocks()
tryRegionEnds[index] = block;
};

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

//-------------------------------------------------------------
// ehUpdateTryLasts: Iterates EH descriptors, updating each try region's
// ehSetTryLasts: Iterates EH descriptors, updating each try region's
// end block as determined by getTryLast.
//
// Type parameters:
Expand All @@ -5126,7 +5126,7 @@ void Compiler::fgMoveColdBlocks()
// 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)
void Compiler::ehSetTryLasts(GetTryLast getTryLast, SetTryLast setTryLast)
{
unsigned XTnum = 0;
for (EHblkDsc* const HBtab : EHClauses(this))
Expand Down
68 changes: 26 additions & 42 deletions src/coreclr/jit/jiteh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,26 @@ void Compiler::ehUpdateLastBlocks(BasicBlock* oldLast, BasicBlock* newLast)
}
}

//-----------------------------------------------------------------------------
// ehUpdateLastTryBlocks: Update the end pointer of the try region containing 'oldTryLast',
// as well as the end pointers of any parent try regions, to 'newTryLast'.
//
// Arguments:
// oldTryLast - The previous end block of the try region(s) to be updated
// newTryLast - The new end block
//
void Compiler::ehUpdateLastTryBlocks(BasicBlock* oldTryLast, BasicBlock* newTryLast)
{
assert(oldTryLast->hasTryIndex() && BasicBlock::sameTryRegion(oldTryLast, newTryLast));
unsigned XTnum = oldTryLast->getTryIndex();
for (EHblkDsc* HBtab = ehGetDsc(XTnum); (XTnum < compHndBBtabCount) && (HBtab->ebdTryLast == oldTryLast);
XTnum++, HBtab++)
{
assert((XTnum == oldTryLast->getTryIndex()) || (ehGetDsc(XTnum - 1)->ebdEnclosingTryIndex == XTnum));
fgSetTryEnd(HBtab, newTryLast);
}
}

unsigned Compiler::ehGetCallFinallyRegionIndex(unsigned finallyIndex, bool* inTryRegion)
{
assert(finallyIndex != EHblkDsc::NO_ENCLOSING_INDEX);
Expand Down Expand Up @@ -1239,16 +1259,8 @@ EHblkDsc* Compiler::ehInitTryBlockRange(BasicBlock* blk, BasicBlock** tryBeg, Ba
void Compiler::fgSetTryBeg(EHblkDsc* handlerTab, BasicBlock* newTryBeg)
{
assert(newTryBeg != nullptr);

// Check if we are going to change the existing value of endTryLast
//
if (handlerTab->ebdTryBeg != newTryBeg)
{
// Update the EH table with the newTryLast block
handlerTab->ebdTryBeg = newTryBeg;

JITDUMP("EH#%u: New first block of try: " FMT_BB "\n", ehGetIndex(handlerTab), handlerTab->ebdTryBeg->bbNum);
}
handlerTab->ebdTryBeg = newTryBeg;
JITDUMP("EH#%u: New first block of try: " FMT_BB "\n", ehGetIndex(handlerTab), newTryBeg->bbNum);
}

/*****************************************************************************
Expand All @@ -1258,22 +1270,8 @@ void Compiler::fgSetTryBeg(EHblkDsc* handlerTab, BasicBlock* newTryBeg)
void Compiler::fgSetTryEnd(EHblkDsc* handlerTab, BasicBlock* newTryLast)
{
assert(newTryLast != nullptr);

//
// Check if we are going to change the existing value of endTryLast
//
if (handlerTab->ebdTryLast != newTryLast)
{
// Update the EH table with the newTryLast block
handlerTab->ebdTryLast = newTryLast;

#ifdef DEBUG
if (verbose)
{
printf("EH#%u: New last block of try: " FMT_BB "\n", ehGetIndex(handlerTab), newTryLast->bbNum);
}
#endif // DEBUG
}
handlerTab->ebdTryLast = newTryLast;
JITDUMP("EH#%u: New last block of try: " FMT_BB "\n", ehGetIndex(handlerTab), newTryLast->bbNum);
}

/*****************************************************************************
Expand All @@ -1284,22 +1282,8 @@ void Compiler::fgSetTryEnd(EHblkDsc* handlerTab, BasicBlock* newTryLast)
void Compiler::fgSetHndEnd(EHblkDsc* handlerTab, BasicBlock* newHndLast)
{
assert(newHndLast != nullptr);

//
// Check if we are going to change the existing value of endHndLast
//
if (handlerTab->ebdHndLast != newHndLast)
{
// Update the EH table with the newHndLast block
handlerTab->ebdHndLast = newHndLast;

#ifdef DEBUG
if (verbose)
{
printf("EH#%u: New last block of handler: " FMT_BB "\n", ehGetIndex(handlerTab), newHndLast->bbNum);
}
#endif // DEBUG
}
handlerTab->ebdHndLast = newHndLast;
JITDUMP("EH#%u: New last block of handler: " FMT_BB "\n", ehGetIndex(handlerTab), newHndLast->bbNum);
}

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