Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Introduce BBJ_EHFAULTRET #84467

Merged
merged 1 commit into from
Apr 7, 2023
Merged

Introduce BBJ_EHFAULTRET #84467

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
34 changes: 9 additions & 25 deletions src/coreclr/jit/block.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,10 @@ void BasicBlock::dspJumpKind()
printf(" (finret)");
break;

case BBJ_EHFAULTRET:
printf(" (falret)");
break;

case BBJ_EHFILTERRET:
printf(" (fltret)");
break;
Expand Down Expand Up @@ -1023,6 +1027,7 @@ bool BasicBlock::bbFallsThrough() const
{
case BBJ_THROW:
case BBJ_EHFINALLYRET:
case BBJ_EHFAULTRET:
case BBJ_EHFILTERRET:
case BBJ_EHCATCHRET:
case BBJ_RETURN:
Expand Down Expand Up @@ -1060,6 +1065,7 @@ unsigned BasicBlock::NumSucc() const
case BBJ_THROW:
case BBJ_RETURN:
case BBJ_EHFINALLYRET:
case BBJ_EHFAULTRET:
case BBJ_EHFILTERRET:
return 0;

Expand Down Expand Up @@ -1147,30 +1153,17 @@ unsigned BasicBlock::NumSucc(Compiler* comp)
{
case BBJ_THROW:
case BBJ_RETURN:
case BBJ_EHFAULTRET:
return 0;

case BBJ_EHFINALLYRET:
{
// We may call this method before we realize we have invalid IL. Tolerate.
//
if (!hasHndIndex())
{
return 0;
}

// The first block of the handler is labelled with the catch type.
BasicBlock* hndBeg = comp->fgFirstBlockOfHandler(this);
if (hndBeg->bbCatchTyp == BBCT_FINALLY)
{
return comp->fgNSuccsOfFinallyRet(this);
}
else
{
assert(hndBeg->bbCatchTyp == BBCT_FAULT); // We can only BBJ_EHFINALLYRET from FINALLY and FAULT.
// A FAULT block has no successors.
return 0;
}
}
return comp->fgNSuccsOfFinallyRet(this);

case BBJ_CALLFINALLY:
case BBJ_ALWAYS:
Expand Down Expand Up @@ -1615,16 +1608,7 @@ bool BasicBlock::hasEHBoundaryIn() const
//
bool BasicBlock::hasEHBoundaryOut() const
{
bool returnVal = false;
if (bbJumpKind == BBJ_EHFILTERRET)
{
returnVal = true;
}

if (bbJumpKind == BBJ_EHFINALLYRET)
{
returnVal = true;
}
bool returnVal = KindIs(BBJ_EHFILTERRET, BBJ_EHFINALLYRET, BBJ_EHFAULTRET);

#if FEATURE_EH_FUNCLETS
if (bbJumpKind == BBJ_EHCATCHRET)
Expand Down
24 changes: 22 additions & 2 deletions src/coreclr/jit/block.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ typedef BitVec_ValRet_T ASSERT_VALRET_TP;

enum BBjumpKinds : BYTE
{
BBJ_EHFINALLYRET,// block ends with 'endfinally' (for finally or fault)
BBJ_EHFINALLYRET,// block ends with 'endfinally' (for finally)
BBJ_EHFAULTRET, // block ends with 'endfinally' (IL alias for 'endfault') (for fault)
BBJ_EHFILTERRET, // block ends with 'endfilter'
BBJ_EHCATCHRET, // block ends with a leave out of a catch (only #if defined(FEATURE_EH_FUNCLETS))
BBJ_THROW, // block ends with 'throw'
Expand All @@ -74,6 +75,24 @@ enum BBjumpKinds : BYTE
BBJ_COUNT
};

#ifdef DEBUG
const char* const BBjumpKindNames[] = {
"BBJ_EHFINALLYRET",
"BBJ_EHFAULTRET",
"BBJ_EHFILTERRET",
"BBJ_EHCATCHRET",
"BBJ_THROW",
"BBJ_RETURN",
"BBJ_NONE",
"BBJ_ALWAYS",
"BBJ_LEAVE",
"BBJ_CALLFINALLY",
"BBJ_COND",
"BBJ_SWITCH",
"BBJ_COUNT"
};
#endif // DEBUG

// clang-format on

struct GenTree;
Expand Down Expand Up @@ -829,7 +848,7 @@ struct BasicBlock : private LIR::Range
// GetSucc() without a Compiler*.
//
// The behavior of NumSucc()/GetSucc() is different when passed a Compiler* for blocks that end in:
// (1) BBJ_EHFINALLYRET (a return from a finally or fault block)
// (1) BBJ_EHFINALLYRET (a return from a finally block)
// (2) BBJ_EHFILTERRET (a return from EH filter block)
// (3) BBJ_SWITCH
//
Expand Down Expand Up @@ -1656,6 +1675,7 @@ inline BasicBlock::BBSuccList::BBSuccList(const BasicBlock* block)
case BBJ_THROW:
case BBJ_RETURN:
case BBJ_EHFINALLYRET:
case BBJ_EHFAULTRET:
case BBJ_EHFILTERRET:
// We don't need m_succs.
m_begin = nullptr;
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/codegencommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ void CodeGen::genMarkLabelsForCodegen()
break;

case BBJ_EHFINALLYRET:
case BBJ_EHFAULTRET:
case BBJ_EHFILTERRET:
case BBJ_RETURN:
case BBJ_THROW:
Expand Down Expand Up @@ -7857,7 +7858,7 @@ void CodeGen::genReturn(GenTree* treeNode)
#else // !FEATURE_EH_FUNCLETS
// Don't generate stack checks for x86 finally/filter EH returns: these are not invoked
// with the same SP as the main function. See also CodeGen::genEHFinallyOrFilterRet().
if ((compiler->compCurBB->bbJumpKind == BBJ_EHFINALLYRET) || (compiler->compCurBB->bbJumpKind == BBJ_EHFILTERRET))
if (compiler->compCurBB->KindIs(BBJ_EHFINALLYRET, BBJ_EHFAULTRET, BBJ_EHFILTERRET))
{
doStackPointerCheck = false;
}
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/codegenlinear.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,7 @@ void CodeGen::genCodeForBBlist()

case BBJ_RETURN:
case BBJ_EHFINALLYRET:
case BBJ_EHFAULTRET:
case BBJ_EHFILTERRET:
// These are the "epilog follows" case, handled in the emitter.

Expand Down Expand Up @@ -715,6 +716,7 @@ void CodeGen::genCodeForBBlist()
FALLTHROUGH;

case BBJ_EHFINALLYRET:
case BBJ_EHFAULTRET:
case BBJ_EHFILTERRET:
genReserveFuncletEpilog(block);
break;
Expand All @@ -726,6 +728,7 @@ void CodeGen::genCodeForBBlist()
break;

case BBJ_EHFINALLYRET:
case BBJ_EHFAULTRET:
case BBJ_EHFILTERRET:
genEHFinallyOrFilterRet(block);
break;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/codegenxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ void CodeGen::genEHFinallyOrFilterRet(BasicBlock* block)
assert(block->lastNode() != nullptr);
assert(block->lastNode()->OperGet() == GT_RETFILT);

if (block->bbJumpKind == BBJ_EHFINALLYRET)
if (block->KindIs(BBJ_EHFINALLYRET, BBJ_EHFAULTRET))
{
assert(block->lastNode()->AsOp()->gtOp1 == nullptr); // op1 == nullptr means endfinally

Expand Down
38 changes: 29 additions & 9 deletions src/coreclr/jit/fgbasic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2802,6 +2802,7 @@ void Compiler::fgLinkBasicBlocks()
// We do it in impFixPredLists.
break;

case BBJ_EHFAULTRET:
case BBJ_THROW:
case BBJ_RETURN:
break;
Expand Down Expand Up @@ -3064,6 +3065,7 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F
break;

case CEE_ENDFINALLY:
// Start with BBJ_EHFINALLYRET; change to BBJ_EHFAULTRET later if it's in a 'fault' clause.
jmpKind = BBJ_EHFINALLYRET;
break;

Expand Down Expand Up @@ -3782,6 +3784,13 @@ void Compiler::fgFindBasicBlocks()
if (!block->hasHndIndex())
{
block->setHndIndex(XTnum);

// If the most nested EH handler region of this block is a 'fault' region, then change any
// BBJ_EHFINALLYRET that were imported to BBJ_EHFAULTRET.
if ((hndBegBB->bbCatchTyp == BBCT_FAULT) && block->KindIs(BBJ_EHFINALLYRET))
{
block->bbJumpKind = BBJ_EHFAULTRET;
}
}

// All blocks in a catch handler or filter are rarely run, except the entry
Expand Down Expand Up @@ -4060,6 +4069,7 @@ void Compiler::fgCheckBasicBlockControlFlow()
break;

case BBJ_EHFINALLYRET:
case BBJ_EHFAULTRET:
case BBJ_EHFILTERRET:

if (!blk->hasHndIndex()) // must be part of a handler
Expand All @@ -4077,17 +4087,28 @@ void Compiler::fgCheckBasicBlockControlFlow()
BADCODE("Unexpected endfilter");
}
}
// endfinally allowed only in a finally/fault block
else if (!HBtab->HasFinallyOrFaultHandler())
else if (blk->bbJumpKind == BBJ_EHFILTERRET)
{
BADCODE("Unexpected endfinally");
// endfinally allowed only in a finally block
if (!HBtab->HasFinallyHandler())
{
BADCODE("Unexpected endfinally");
}
}
else if (blk->bbJumpKind == BBJ_EHFAULTRET)
{
// 'endfault' (alias of IL 'endfinally') allowed only in a fault block
if (!HBtab->HasFaultHandler())
{
BADCODE("Unexpected endfault");
}
}

// The handler block should be the innermost block
// Exception blocks are listed, innermost first.
if (blk->hasTryIndex() && (blk->getTryIndex() < blk->getHndIndex()))
{
BADCODE("endfinally / endfilter in nested try block");
BADCODE("endfinally / endfault / endfilter in nested try block");
}

break;
Expand Down Expand Up @@ -6298,8 +6319,7 @@ BasicBlock* Compiler::fgFindInsertPoint(unsigned regionIndex,
#endif // DEBUG

JITDUMP("fgFindInsertPoint(regionIndex=%u, putInTryRegion=%s, startBlk=" FMT_BB ", endBlk=" FMT_BB
", nearBlk=" FMT_BB ", "
"jumpBlk=" FMT_BB ", runRarely=%s)\n",
", nearBlk=" FMT_BB ", jumpBlk=" FMT_BB ", runRarely=%s)\n",
regionIndex, dspBool(putInTryRegion), startBlk->bbNum, (endBlk == nullptr) ? 0 : endBlk->bbNum,
(nearBlk == nullptr) ? 0 : nearBlk->bbNum, (jumpBlk == nullptr) ? 0 : jumpBlk->bbNum, dspBool(runRarely));

Expand Down Expand Up @@ -6697,10 +6717,10 @@ _FoundAfterBlk:;
/* We have decided to insert the block after 'afterBlk'. */
noway_assert(afterBlk != nullptr);

JITDUMP("fgNewBBinRegion(jumpKind=%u, tryIndex=%u, hndIndex=%u, putInFilter=%s, runRarely=%s, insertAtEnd=%s): "
JITDUMP("fgNewBBinRegion(jumpKind=%s, tryIndex=%u, hndIndex=%u, putInFilter=%s, runRarely=%s, insertAtEnd=%s): "
"inserting after " FMT_BB "\n",
jumpKind, tryIndex, hndIndex, dspBool(putInFilter), dspBool(runRarely), dspBool(insertAtEnd),
afterBlk->bbNum);
BBjumpKindNames[jumpKind], tryIndex, hndIndex, dspBool(putInFilter), dspBool(runRarely),
dspBool(insertAtEnd), afterBlk->bbNum);

return fgNewBBinRegionWorker(jumpKind, afterBlk, regionIndex, putInTryRegion);
}
Expand Down
8 changes: 7 additions & 1 deletion src/coreclr/jit/fgdiagnostic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ void Compiler::fgDebugCheckUpdate()
{
case BBJ_CALLFINALLY:
case BBJ_EHFINALLYRET:
case BBJ_EHFAULTRET:
case BBJ_EHFILTERRET:
case BBJ_RETURN:
/* for BBJ_ALWAYS is probably just a GOTO, but will have to be treated */
Expand Down Expand Up @@ -1994,6 +1995,10 @@ void Compiler::fgTableDispBasicBlock(BasicBlock* block, int ibcColWidth /* = 0 *
printf("%*s (finret)", maxBlockNumWidth - 2, "");
break;

case BBJ_EHFAULTRET:
printf("%*s (falret)", maxBlockNumWidth - 2, "");
break;

case BBJ_EHFILTERRET:
printf("%*s (fltret)", maxBlockNumWidth - 2, "");
break;
Expand Down Expand Up @@ -2633,9 +2638,10 @@ bool BBPredsChecker::CheckJump(BasicBlock* blockPred, BasicBlock* block)
assert(CheckEHFinallyRet(blockPred, block));
return true;

case BBJ_EHFAULTRET:
case BBJ_THROW:
case BBJ_RETURN:
assert(!"THROW and RETURN block cannot be in the predecessor list!");
assert(!"EHFAULTRET, THROW, and RETURN block cannot be in the predecessor list!");
break;

case BBJ_SWITCH:
Expand Down
25 changes: 20 additions & 5 deletions src/coreclr/jit/fgehopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1035,7 +1035,7 @@ PhaseStatus Compiler::fgCloneFinally()
// statement after the finally, so they can share the clone.
//
// Clone the finally body, and splice it into the flow graph
// within in the parent region of the try.
// within the parent region of the try.
//
const unsigned finallyTryIndex = firstBlock->bbTryIndex;
BasicBlock* insertAfter = nullptr;
Expand All @@ -1048,14 +1048,18 @@ PhaseStatus Compiler::fgCloneFinally()
{
BasicBlock* newBlock;

// Avoid asserts when `fgNewBBinRegion` verifies the handler table, by mapping any cloned finally
// return blocks to BBJ_ALWAYS (which we would do below if we didn't do it here).
BBjumpKinds bbNewJumpKind = (block->bbJumpKind == BBJ_EHFINALLYRET) ? BBJ_ALWAYS : block->bbJumpKind;

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(block->bbJumpKind, finallyTryIndex, hndIndex, nearBlk);
newBlock = fgNewBBinRegion(bbNewJumpKind, finallyTryIndex, hndIndex, nearBlk);

// If the clone ends up just after the finally, adjust
// the stopping point for finally traversal.
Expand All @@ -1069,7 +1073,7 @@ PhaseStatus Compiler::fgCloneFinally()
{
// Put subsequent blocks in the same region...
const bool extendRegion = true;
newBlock = fgNewBBafter(block->bbJumpKind, insertAfter, extendRegion);
newBlock = fgNewBBafter(bbNewJumpKind, insertAfter, extendRegion);
}

cloneBBCount++;
Expand Down Expand Up @@ -1121,7 +1125,7 @@ PhaseStatus Compiler::fgCloneFinally()
JITDUMP("Cloned finally blocks are: " FMT_BB " ... " FMT_BB "\n", blockMap[firstBlock]->bbNum,
blockMap[lastBlock]->bbNum);

// Redirect redirect any branches within the newly-cloned
// 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->bbNext)
Expand All @@ -1134,7 +1138,7 @@ PhaseStatus Compiler::fgCloneFinally()
GenTree* finallyRetExpr = finallyRet->GetRootNode();
assert(finallyRetExpr->gtOper == GT_RETFILT);
fgRemoveStmt(newBlock, finallyRet);
newBlock->bbJumpKind = BBJ_ALWAYS;
assert(newBlock->bbJumpKind == BBJ_ALWAYS); // we mapped this above already
newBlock->bbJumpDest = normalCallFinallyReturn;

fgAddRefPred(normalCallFinallyReturn, newBlock);
Expand Down Expand Up @@ -1231,6 +1235,17 @@ PhaseStatus Compiler::fgCloneFinally()
JITDUMP("All callfinallys retargeted; changing finally to fault.\n");
HBtab->ebdHandlerType = EH_HANDLER_FAULT_WAS_FINALLY;
firstBlock->bbCatchTyp = BBCT_FAULT;

// Change all BBJ_EHFINALLYRET to BBJ_EHFAULTRET in the now-fault region.
BasicBlock* const hndBegIter = HBtab->ebdHndBeg;
BasicBlock* const hndEndIter = HBtab->ebdHndLast->bbNext;
for (BasicBlock* block = hndBegIter; block != hndEndIter; block = block->bbNext)
{
if (block->bbJumpKind == BBJ_EHFINALLYRET)
{
block->bbJumpKind = BBJ_EHFAULTRET;
}
}
}
else
{
Expand Down
Loading