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.

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: Allow hot/cold splitting between a BBJ_COND block and its false target #96431

Merged
merged 3 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 16 additions & 1 deletion src/coreclr/jit/block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,12 +294,27 @@ bool BasicBlock::IsFirstColdBlock(Compiler* compiler) const
// Returns:
// true if block is a BBJ_ALWAYS to the next block that we can fall into
//
bool BasicBlock::CanRemoveJumpToNext(Compiler* compiler)
bool BasicBlock::CanRemoveJumpToNext(Compiler* compiler) const
{
assert(KindIs(BBJ_ALWAYS));
return JumpsToNext() && !hasAlign() && !compiler->fgInDifferentRegions(this, bbTarget);
}

//------------------------------------------------------------------------
// CanRemoveJumpToFalseTarget: determine if jump to false target can be omitted
//
// Arguments:
// compiler - current compiler instance
//
// Returns:
// true if block is a BBJ_COND that can fall into its false target
//
bool BasicBlock::CanRemoveJumpToFalseTarget(Compiler* compiler) const
{
assert(KindIs(BBJ_COND));
return NextIs(bbFalseTarget) && !hasAlign() && !compiler->fgInDifferentRegions(this, bbFalseTarget);
}

//------------------------------------------------------------------------
// checkPredListOrder: see if pred list is properly ordered
//
Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/jit/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,9 @@ struct BasicBlock : private LIR::Range

bool IsFirstColdBlock(Compiler* compiler) const;

bool CanRemoveJumpToNext(Compiler* compiler);
bool CanRemoveJumpToNext(Compiler* compiler) const;

bool CanRemoveJumpToFalseTarget(Compiler* compiler) const;

unsigned GetTargetOffs() const
{
Expand Down
7 changes: 7 additions & 0 deletions src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,13 @@ void CodeGen::genMarkLabelsForCodegen()
case BBJ_COND:
JITDUMP(" " FMT_BB " : branch target\n", block->GetTrueTarget()->bbNum);
block->GetTrueTarget()->SetFlags(BBF_HAS_LABEL);

// If we need a jump to the false target, give it a label
if (!block->CanRemoveJumpToFalseTarget(compiler))
{
JITDUMP(" " FMT_BB " : branch target\n", block->GetFalseTarget()->bbNum);
block->GetFalseTarget()->SetFlags(BBF_HAS_LABEL);
}
break;

case BBJ_SWITCH:
Expand Down
10 changes: 8 additions & 2 deletions src/coreclr/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,8 @@ void CodeGen::genCodeForBBlist()
printf("\nThis is the start of the cold region of the method\n");
}
#endif
// We should never have a block that falls through into the Cold section
noway_assert(!block->Prev()->bbFallsThrough());
// We should never split call/finally pairs between hot/cold sections
noway_assert(!block->isBBCallFinallyPairTail());

needLabel = true;
}
Expand Down Expand Up @@ -2619,6 +2619,12 @@ void CodeGen::genCodeForJcc(GenTreeCC* jcc)
assert(jcc->OperIs(GT_JCC));

inst_JCC(jcc->gtCondition, compiler->compCurBB->GetTrueTarget());

// If we cannot fall into the false target, emit a jump to it
if (!compiler->compCurBB->CanRemoveJumpToFalseTarget(compiler))
{
inst_JMP(EJ_jmp, compiler->compCurBB->GetFalseTarget());
}
}

//------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -6406,7 +6406,7 @@ class Compiler
bool fgIsThrow(GenTree* tree);

public:
bool fgInDifferentRegions(BasicBlock* blk1, BasicBlock* blk2);
bool fgInDifferentRegions(const BasicBlock* blk1, const BasicBlock* blk2) const;

private:
bool fgIsBlockCold(BasicBlock* block);
Expand Down
55 changes: 5 additions & 50 deletions src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ bool Compiler::fgIsThrow(GenTree* tree)
* It returns false when the blocks are both in the same regions
*/

bool Compiler::fgInDifferentRegions(BasicBlock* blk1, BasicBlock* blk2)
bool Compiler::fgInDifferentRegions(const BasicBlock* blk1, const BasicBlock* blk2) const
{
noway_assert(blk1 != nullptr);
noway_assert(blk2 != nullptr);
Expand Down Expand Up @@ -3185,56 +3185,11 @@ PhaseStatus Compiler::fgDetermineFirstColdBlock()
}
}

// When the last Hot block fall through into the Cold section
// we may need to add a jump
//
if (prevToFirstColdBlock->bbFallsThrough())
// Don't split up call/finally pairs
if (prevToFirstColdBlock->isBBCallFinallyPair())
{
switch (prevToFirstColdBlock->GetKind())
{
default:
noway_assert(!"Unhandled jumpkind in fgDetermineFirstColdBlock()");
break;

case BBJ_CALLFINALLY:
// A BBJ_CALLFINALLY that falls through is always followed
// by an empty BBJ_CALLFINALLYRET.
//
assert(prevToFirstColdBlock->isBBCallFinallyPair());
firstColdBlock =
firstColdBlock->Next(); // Note that this assignment could make firstColdBlock == nullptr
break;

case BBJ_COND:
//
// This is a slightly more complicated case, because we will
// probably need to insert a block to jump to the cold section.
//

// TODO-NoFallThrough: Below logic will need additional check once bbFalseTarget can diverge from
// bbNext
assert(prevToFirstColdBlock->FalseTargetIs(firstColdBlock));
if (firstColdBlock->isEmpty() && firstColdBlock->KindIs(BBJ_ALWAYS))
{
// We can just use this block as the transitionBlock
firstColdBlock = firstColdBlock->Next();
// Note that this assignment could make firstColdBlock == NULL
}
else
{
BasicBlock* transitionBlock =
fgNewBBafter(BBJ_ALWAYS, prevToFirstColdBlock, true, firstColdBlock);
transitionBlock->inheritWeight(firstColdBlock);
prevToFirstColdBlock->SetFalseTarget(transitionBlock);

// Update the predecessor list for firstColdBlock
fgReplacePred(firstColdBlock, prevToFirstColdBlock, transitionBlock);

// Add prevToFirstColdBlock as a predecessor for transitionBlock
fgAddRefPred(transitionBlock, prevToFirstColdBlock);
}
break;
}
// Note that this assignment could make firstColdBlock == nullptr
firstColdBlock = firstColdBlock->Next();
}
}

Expand Down
Loading