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

Introduce BBJ_CALLFINALLYRET block kind #95945

Merged
merged 2 commits into from
Dec 15, 2023
Merged

Introduce BBJ_CALLFINALLYRET block kind #95945

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a3e5de
Introduce BBJ_CALLFINALLYRET block kind
BruceForstall Dec 13, 2023
a8fda09
Fix grammar
BruceForstall Dec 15, 2023
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
86 changes: 66 additions & 20 deletions src/coreclr/jit/block.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ FlowEdge* Compiler::BlockPredsWithEH(BasicBlock* blk)

res = blk->bbPreds;
unsigned tryIndex = blk->getHndIndex();
// Add all blocks handled by this handler (except for second blocks of BBJ_CALLFINALLY/BBJ_ALWAYS pairs;
// Add all blocks handled by this handler (except for second blocks of BBJ_CALLFINALLY/BBJ_CALLFINALLYRET pairs;
// these cannot cause transfer to the handler...)
// TODO-Throughput: It would be nice if we could iterate just over the blocks in the try, via
// something like:
Expand All @@ -140,7 +140,7 @@ FlowEdge* Compiler::BlockPredsWithEH(BasicBlock* blk)
// more than one sequence of contiguous blocks. We need to find a better way to do this.
for (BasicBlock* const bb : Blocks())
{
if (bbInExnFlowRegions(tryIndex, bb) && !bb->isBBCallAlwaysPairTail())
if (bbInExnFlowRegions(tryIndex, bb) && !bb->isBBCallFinallyPairTail())
{
res = new (this, CMK_FlowEdge) FlowEdge(bb, res);

Expand Down Expand Up @@ -670,6 +670,10 @@ void BasicBlock::dspKind()
printf(" -> " FMT_BB " (callf)", bbTarget->bbNum);
break;

case BBJ_CALLFINALLYRET:
printf(" -> " FMT_BB " (callfr)", bbTarget->bbNum);
break;

case BBJ_COND:
printf(" -> " FMT_BB " (cond)", bbTarget->bbNum);
break;
Expand Down Expand Up @@ -811,6 +815,34 @@ bool BasicBlock::CloneBlockState(
return true;
}

//------------------------------------------------------------------------
// CopyTarget: Copy the block kind and targets.
//
// Arguments:
// compiler - Jit compiler instance
// from - Block to copy from
//
void BasicBlock::CopyTarget(Compiler* compiler, const BasicBlock* from)
{
switch (from->GetKind())
{
case BBJ_SWITCH:
SetSwitch(new (compiler, CMK_BasicBlock) BBswtDesc(compiler, from->GetSwitchTargets()));
break;
case BBJ_EHFINALLYRET:
SetEhf(new (compiler, CMK_BasicBlock) BBehfDesc(compiler, from->GetEhfTargets()));
break;
case BBJ_COND:
SetCond(from->GetTrueTarget());
break;
default:
SetKindAndTarget(from->GetKind(), from->GetTarget());
CopyFlags(from, BBF_NONE_QUIRK);
break;
}
assert(KindIs(from->GetKind()));
}

// LIR helpers
void BasicBlock::MakeLIR(GenTree* firstNode, GenTree* lastNode)
{
Expand Down Expand Up @@ -1032,6 +1064,7 @@ bool BasicBlock::bbFallsThrough() const
case BBJ_EHCATCHRET:
case BBJ_RETURN:
case BBJ_ALWAYS:
case BBJ_CALLFINALLYRET:
case BBJ_LEAVE:
case BBJ_SWITCH:
return false;
Expand Down Expand Up @@ -1067,6 +1100,7 @@ unsigned BasicBlock::NumSucc() const
return 0;

case BBJ_CALLFINALLY:
case BBJ_CALLFINALLYRET:
case BBJ_ALWAYS:
case BBJ_EHCATCHRET:
case BBJ_EHFILTERRET:
Expand Down Expand Up @@ -1123,6 +1157,7 @@ BasicBlock* BasicBlock::GetSucc(unsigned i) const
switch (bbKind)
{
case BBJ_CALLFINALLY:
case BBJ_CALLFINALLYRET:
case BBJ_ALWAYS:
case BBJ_EHCATCHRET:
case BBJ_EHFILTERRET:
Expand Down Expand Up @@ -1190,6 +1225,7 @@ unsigned BasicBlock::NumSucc(Compiler* comp)
return bbEhfTargets->bbeCount;

case BBJ_CALLFINALLY:
case BBJ_CALLFINALLYRET:
case BBJ_ALWAYS:
case BBJ_EHCATCHRET:
case BBJ_EHFILTERRET:
Expand Down Expand Up @@ -1245,6 +1281,7 @@ BasicBlock* BasicBlock::GetSucc(unsigned i, Compiler* comp)
return bbEhfTargets->bbeSuccs[i];

case BBJ_CALLFINALLY:
case BBJ_CALLFINALLYRET:
case BBJ_ALWAYS:
case BBJ_EHCATCHRET:
case BBJ_LEAVE:
Expand Down Expand Up @@ -1560,33 +1597,30 @@ BasicBlock* BasicBlock::New(Compiler* compiler, BBKinds kind, unsigned targetOff
}

//------------------------------------------------------------------------
// isBBCallAlwaysPair: Determine if this is the first block of a BBJ_CALLFINALLY/BBJ_ALWAYS pair
// isBBCallFinallyPair: Determine if this is the first block of a BBJ_CALLFINALLY/BBJ_CALLFINALLYRET pair
//
// Return Value:
// True iff "this" is the first block of a BBJ_CALLFINALLY/BBJ_ALWAYS pair
// True iff "this" is the first block of a BBJ_CALLFINALLY/BBJ_CALLFINALLYRET pair
// -- a block corresponding to an exit from the try of a try/finally.
//
// Notes:
// In the flow graph, this becomes a block that calls the finally, and a second, immediately
// following empty block (in the bbNext chain) to which the finally will return, and which
// following (in the bbNext chain) empty block to which the finally will return, and which
// branches unconditionally to the next block to be executed outside the try/finally.
// Note that code is often generated differently than this description. For example, on ARM,
// the target of the BBJ_ALWAYS is loaded in LR (the return register), and a direct jump is
// Note that code is often generated differently than this description. For example, on x86,
// the target of the BBJ_CALLFINALLYRET is pushed on the stack and a direct jump is
// made to the 'finally'. The effect is that the 'finally' returns directly to the target of
// the BBJ_ALWAYS. A "retless" BBJ_CALLFINALLY is one that has no corresponding BBJ_ALWAYS.
// the BBJ_CALLFINALLYRET. A "retless" BBJ_CALLFINALLY is one that has no corresponding BBJ_CALLFINALLYRET.
// This can happen if the finally is known to not return (e.g., it contains a 'throw'). In
// that case, the BBJ_CALLFINALLY flags has BBF_RETLESS_CALL set. Note that ARM never has
// "retless" BBJ_CALLFINALLY blocks due to a requirement to use the BBJ_ALWAYS for
// generating code.
// that case, the BBJ_CALLFINALLY flags has BBF_RETLESS_CALL set.
//
bool BasicBlock::isBBCallAlwaysPair() const
bool BasicBlock::isBBCallFinallyPair() const
{
if (this->KindIs(BBJ_CALLFINALLY) && !this->HasFlag(BBF_RETLESS_CALL))
{
// Some asserts that the next block is a BBJ_ALWAYS of the proper form.
// Some asserts that the next block is of the proper form.
assert(!this->IsLast());
assert(this->Next()->KindIs(BBJ_ALWAYS));
assert(this->Next()->HasFlag(BBF_KEEP_BBJ_ALWAYS));
assert(this->Next()->KindIs(BBJ_CALLFINALLYRET));
assert(this->Next()->isEmpty());

return true;
Expand All @@ -1598,18 +1632,30 @@ bool BasicBlock::isBBCallAlwaysPair() const
}

//------------------------------------------------------------------------
// isBBCallAlwaysPairTail: Determine if this is the last block of a BBJ_CALLFINALLY/BBJ_ALWAYS pair
// isBBCallFinallyPairTail: Determine if this is the last block of a BBJ_CALLFINALLY/BBJ_CALLFINALLYRET pair
//
// Return Value:
// True iff "this" is the last block of a BBJ_CALLFINALLY/BBJ_ALWAYS pair
// True iff "this" is the last block of a BBJ_CALLFINALLY/BBJ_CALLFINALLYRET pair
// -- a block corresponding to an exit from the try of a try/finally.
//
// Notes:
// See notes on isBBCallAlwaysPair(), above.
// See notes on isBBCallFinallyPair(), above.
//
bool BasicBlock::isBBCallAlwaysPairTail() const
bool BasicBlock::isBBCallFinallyPairTail() const
{
return (bbPrev != nullptr) && bbPrev->isBBCallAlwaysPair();
if (KindIs(BBJ_CALLFINALLYRET))
{
// Some asserts that the previous block is of the proper form.
assert(!this->IsFirst());
assert(this->Prev()->KindIs(BBJ_CALLFINALLY));
assert(!this->Prev()->HasFlag(BBF_RETLESS_CALL));

return true;
}
else
{
return false;
}
}

//------------------------------------------------------------------------
Expand Down
38 changes: 29 additions & 9 deletions src/coreclr/jit/block.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ enum BBKinds : BYTE
BBJ_ALWAYS, // block always jumps to the target
BBJ_LEAVE, // block always jumps to the target, maybe out of guarded region. Only used until importing.
BBJ_CALLFINALLY, // block always calls the target finally
BBJ_CALLFINALLYRET, // block targets the return from finally, aka "finally continuation". Always paired with BBJ_CALLFINALLY.
BBJ_COND, // block conditionally jumps to the target
BBJ_SWITCH, // block ends with a switch statement

Expand All @@ -85,6 +86,7 @@ const char* const bbKindNames[] = {
"BBJ_ALWAYS",
"BBJ_LEAVE",
"BBJ_CALLFINALLY",
"BBJ_CALLFINALLYRET",
"BBJ_COND",
"BBJ_SWITCH",
"BBJ_COUNT"
Expand Down Expand Up @@ -406,14 +408,12 @@ enum BasicBlockFlags : unsigned __int64
BBF_HAS_NEWOBJ = MAKE_BBFLAG(23), // BB contains 'new' of an object type.

BBF_RETLESS_CALL = MAKE_BBFLAG(24), // BBJ_CALLFINALLY that will never return (and therefore, won't need a paired
// BBJ_ALWAYS); see isBBCallAlwaysPair().
// BBJ_CALLFINALLYRET); see isBBCallFinallyPair().
BBF_LOOP_PREHEADER = MAKE_BBFLAG(25), // BB is a loop preheader block
BBF_COLD = MAKE_BBFLAG(26), // BB is cold
BBF_PROF_WEIGHT = MAKE_BBFLAG(27), // BB weight is computed from profile data
BBF_KEEP_BBJ_ALWAYS = MAKE_BBFLAG(28), // A special BBJ_ALWAYS block, used by EH code generation. Keep the jump kind
// as BBJ_ALWAYS. Used for the paired BBJ_ALWAYS block following the
// BBJ_CALLFINALLY block, as well as, on x86, the final step block out of a
// finally.
// as BBJ_ALWAYS. Used on x86 for the final step block out of a finally.
BBF_HAS_CALL = MAKE_BBFLAG(29), // BB contains a call
BBF_DOMINATED_BY_EXCEPTIONAL_ENTRY = MAKE_BBFLAG(30), // Block is dominated by exceptional entry.
BBF_BACKWARD_JUMP = MAKE_BBFLAG(31), // BB is surrounded by a backward jump/switch arc
Expand Down Expand Up @@ -633,7 +633,8 @@ struct BasicBlock : private LIR::Range
bool HasTarget() const
{
// These block types should always have bbTarget set
return KindIs(BBJ_ALWAYS, BBJ_CALLFINALLY, BBJ_COND, BBJ_EHCATCHRET, BBJ_EHFILTERRET, BBJ_LEAVE);
return KindIs(BBJ_ALWAYS, BBJ_CALLFINALLY, BBJ_CALLFINALLYRET, BBJ_COND, BBJ_EHCATCHRET, BBJ_EHFILTERRET,
BBJ_LEAVE);
}

BasicBlock* GetTarget() const
Expand Down Expand Up @@ -780,6 +781,21 @@ struct BasicBlock : private LIR::Range
bbEhfTargets = ehfTarget;
}

// BBJ_CALLFINALLYRET uses the `bbTarget` field. However, also treat it specially:
// for callers that know they want a continuation, use this function instead of the
// general `GetTarget()` to allow asserting on the block kind.
BasicBlock* GetFinallyContinuation() const
{
assert(KindIs(BBJ_CALLFINALLYRET));
return bbTarget;
}

void SetFinallyContinuation(BasicBlock* finallyContinuation)
{
assert(KindIs(BBJ_CALLFINALLYRET));
bbTarget = finallyContinuation;
}

private:
BasicBlockFlags bbFlags;

Expand Down Expand Up @@ -1021,13 +1037,13 @@ struct BasicBlock : private LIR::Range

bool isValid() const;

// Returns "true" iff "this" is the first block of a BBJ_CALLFINALLY/BBJ_ALWAYS pair --
// Returns "true" iff "this" is the first block of a BBJ_CALLFINALLY/BBJ_CALLFINALLYRET pair --
// a block corresponding to an exit from the try of a try/finally.
bool isBBCallAlwaysPair() const;
bool isBBCallFinallyPair() const;

// Returns "true" iff "this" is the last block of a BBJ_CALLFINALLY/BBJ_ALWAYS pair --
// Returns "true" iff "this" is the last block of a BBJ_CALLFINALLY/BBJ_CALLFINALLYRET pair --
// a block corresponding to an exit from the try of a try/finally.
bool isBBCallAlwaysPairTail() const;
bool isBBCallFinallyPairTail() const;

bool KindIs(BBKinds kind) const
{
Expand Down Expand Up @@ -1658,6 +1674,9 @@ struct BasicBlock : private LIR::Range
static bool CloneBlockState(
Compiler* compiler, BasicBlock* to, const BasicBlock* from, unsigned varNum = (unsigned)-1, int varVal = 0);

// Copy the block kind and targets.
void CopyTarget(Compiler* compiler, const BasicBlock* from);

void MakeLIR(GenTree* firstNode, GenTree* lastNode);
bool IsLIR() const;

Expand Down Expand Up @@ -1917,6 +1936,7 @@ inline BasicBlock::BBSuccList::BBSuccList(const BasicBlock* block)
break;

case BBJ_CALLFINALLY:
case BBJ_CALLFINALLYRET:
case BBJ_ALWAYS:
case BBJ_EHCATCHRET:
case BBJ_EHFILTERRET:
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/clrjit.natvis
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Documentation for VS debugger format specifiers: https://docs.microsoft.com/en-u
</Type>

<Type Name="BasicBlock">
<DisplayString Condition="bbKind==BBJ_COND || bbKind==BBJ_ALWAYS || bbKind==BBJ_LEAVE || bbKind==BBJ_EHCATCHRET || bbKind==BBJ_CALLFINALLY || bbKind==BBJ_EHFILTERRET">BB{bbNum,d}->BB{bbTarget->bbNum,d}; {bbKind,en}</DisplayString>
<DisplayString Condition="bbKind==BBJ_COND || bbKind==BBJ_ALWAYS || bbKind==BBJ_LEAVE || bbKind==BBJ_EHCATCHRET || bbKind==BBJ_CALLFINALLY || bbKind==BBJ_CALLFINALLYRET || bbKind==BBJ_EHFILTERRET">BB{bbNum,d}->BB{bbTarget->bbNum,d}; {bbKind,en}</DisplayString>
<DisplayString Condition="bbKind==BBJ_SWITCH">BB{bbNum,d}; {bbKind,en}; {bbSwtTargets->bbsCount} cases</DisplayString>
<DisplayString Condition="bbKind==BBJ_EHFINALLYRET">BB{bbNum,d}; {bbKind,en}; {bbEhfTargets->bbeCount} succs</DisplayString>
<DisplayString>BB{bbNum,d}; {bbKind,en}</DisplayString>
Expand Down
23 changes: 9 additions & 14 deletions src/coreclr/jit/codegenarm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ bool CodeGen::genStackPointerAdjustment(ssize_t spDelta, regNumber tmpReg)
//
BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
assert(block->KindIs(BBJ_CALLFINALLY));

GetEmitter()->emitIns_J(INS_bl, block->GetTarget());

BasicBlock* nextBlock = block->Next();
Expand All @@ -127,20 +129,22 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
instGen(INS_BREAKPOINT);
}

return block;
}
else
{
assert((nextBlock != nullptr) && nextBlock->isBBCallAlwaysPairTail());
assert((nextBlock != nullptr) && nextBlock->isBBCallFinallyPairTail());

// Because of the way the flowgraph is connected, the liveness info for this one instruction
// after the call is not (can not be) correct in cases where a variable has a last use in the
// handler. So turn off GC reporting for this single instruction.
GetEmitter()->emitDisableGC();

BasicBlock* const jumpDest = nextBlock->GetTarget();
BasicBlock* const finallyContinuation = nextBlock->GetFinallyContinuation();

// Now go to where the finally funclet needs to return to.
if (nextBlock->NextIs(jumpDest) && !compiler->fgInDifferentRegions(nextBlock, jumpDest))
if (nextBlock->NextIs(finallyContinuation) && !compiler->fgInDifferentRegions(nextBlock, finallyContinuation))
{
// Fall-through.
// TODO-ARM-CQ: Can we get rid of this instruction, and just have the call return directly
Expand All @@ -150,22 +154,13 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
}
else
{
GetEmitter()->emitIns_J(INS_b, jumpDest);
GetEmitter()->emitIns_J(INS_b, finallyContinuation);
}

GetEmitter()->emitEnableGC();
}

// The BBJ_ALWAYS is used because the BBJ_CALLFINALLY can't point to the
// jump target using bbTarget - that is already used to point
// to the finally block. So just skip past the BBJ_ALWAYS unless the
// block is RETLESS.
if (!block->HasFlag(BBF_RETLESS_CALL))
{
assert(block->isBBCallAlwaysPair());
block = nextBlock;
return nextBlock;
}
return block;
}

//------------------------------------------------------------------------
Expand Down
Loading