Skip to content

Commit 637e822

Browse files
JIT: Remove fgFirstColdBlock checks in frontend phases (#110452)
Part of #107749. Now that hot/cold splitting runs after layout in the backend, where the flowgraph is expected to never change, we shouldn't need to check for the presence of a cold code section in the frontend.
1 parent c7f4149 commit 637e822

File tree

3 files changed

+20
-57
lines changed

3 files changed

+20
-57
lines changed

src/coreclr/jit/block.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ bool BasicBlock::IsFirstColdBlock(Compiler* compiler) const
424424
bool BasicBlock::CanRemoveJumpToNext(Compiler* compiler) const
425425
{
426426
assert(KindIs(BBJ_ALWAYS));
427-
return JumpsToNext() && (bbNext != compiler->fgFirstColdBlock);
427+
return JumpsToNext() && !IsLastHotBlock(compiler);
428428
}
429429

430430
//------------------------------------------------------------------------
@@ -441,7 +441,7 @@ bool BasicBlock::CanRemoveJumpToTarget(BasicBlock* target, Compiler* compiler) c
441441
{
442442
assert(KindIs(BBJ_COND));
443443
assert(TrueTargetIs(target) || FalseTargetIs(target));
444-
return NextIs(target) && !compiler->fgInDifferentRegions(this, target);
444+
return NextIs(target) && !IsLastHotBlock(compiler);
445445
}
446446

447447
#ifdef DEBUG

src/coreclr/jit/fgbasic.cpp

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4945,7 +4945,7 @@ BasicBlock* Compiler::fgSplitEdge(BasicBlock* curr, BasicBlock* succ)
49454945

49464946
// Removes the block from the bbPrev/bbNext chain
49474947
// Updates fgFirstBB and fgLastBB if necessary
4948-
// Does not update fgFirstFuncletBB or fgFirstColdBlock (fgUnlinkRange does)
4948+
// Does not update fgFirstFuncletBB
49494949
void Compiler::fgUnlinkBlock(BasicBlock* block)
49504950
{
49514951
if (block->IsFirst())
@@ -5006,6 +5006,9 @@ void Compiler::fgUnlinkRange(BasicBlock* bBeg, BasicBlock* bEnd)
50065006
assert(bBeg != nullptr);
50075007
assert(bEnd != nullptr);
50085008

5009+
// We shouldn't be churning the flowgraph after doing hot/cold splitting
5010+
assert(fgFirstColdBlock == nullptr);
5011+
50095012
BasicBlock* bPrev = bBeg->Prev();
50105013
assert(bPrev != nullptr); // Can't unlink a range starting with the first block
50115014

@@ -5020,12 +5023,6 @@ void Compiler::fgUnlinkRange(BasicBlock* bBeg, BasicBlock* bEnd)
50205023
bPrev->SetNext(bEnd->Next());
50215024
}
50225025

5023-
// If bEnd was the first Cold basic block update fgFirstColdBlock
5024-
if (bEnd->IsFirstColdBlock(this))
5025-
{
5026-
fgFirstColdBlock = bPrev->Next();
5027-
}
5028-
50295026
#ifdef DEBUG
50305027
if (UsesFunclets())
50315028
{
@@ -5056,6 +5053,9 @@ BasicBlock* Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable)
50565053
{
50575054
assert(block != nullptr);
50585055

5056+
// We shouldn't churn the flowgraph after doing hot/cold splitting
5057+
assert(fgFirstColdBlock == nullptr);
5058+
50595059
JITDUMP("fgRemoveBlock " FMT_BB ", unreachable=%s\n", block->bbNum, dspBool(unreachable));
50605060

50615061
BasicBlock* bPrev = block->Prev();
@@ -5079,12 +5079,6 @@ BasicBlock* Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable)
50795079
fgFirstFuncletBB = block->Next();
50805080
}
50815081

5082-
// If this is the first Cold basic block update fgFirstColdBlock
5083-
if (block->IsFirstColdBlock(this))
5084-
{
5085-
fgFirstColdBlock = block->Next();
5086-
}
5087-
50885082
// A BBJ_CALLFINALLY is usually paired with a BBJ_CALLFINALLYRET.
50895083
// If we delete such a BBJ_CALLFINALLY we also delete the BBJ_CALLFINALLYRET.
50905084
if (block->isBBCallFinallyPair())
@@ -5141,12 +5135,6 @@ BasicBlock* Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable)
51415135
skipUnmarkLoop = true;
51425136
}
51435137

5144-
// If this is the first Cold basic block update fgFirstColdBlock
5145-
if (block->IsFirstColdBlock(this))
5146-
{
5147-
fgFirstColdBlock = block->Next();
5148-
}
5149-
51505138
// Update fgFirstFuncletBB if necessary
51515139
if (block == fgFirstFuncletBB)
51525140
{

src/coreclr/jit/fgopt.cpp

Lines changed: 11 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -830,13 +830,6 @@ bool Compiler::fgCanCompactBlock(BasicBlock* block)
830830
return false;
831831
}
832832

833-
// We don't want to compact blocks that are in different hot/cold regions
834-
//
835-
if (fgInDifferentRegions(block, target))
836-
{
837-
return false;
838-
}
839-
840833
// We cannot compact two blocks in different EH regions.
841834
//
842835
if (!BasicBlock::sameEHRegion(block, target))
@@ -872,6 +865,10 @@ bool Compiler::fgCanCompactBlock(BasicBlock* block)
872865
void Compiler::fgCompactBlock(BasicBlock* block)
873866
{
874867
assert(fgCanCompactBlock(block));
868+
869+
// We shouldn't churn the flowgraph after doing hot/cold splitting
870+
assert(fgFirstColdBlock == nullptr);
871+
875872
BasicBlock* const target = block->GetTarget();
876873

877874
JITDUMP("\nCompacting " FMT_BB " into " FMT_BB ":\n", target->bbNum, block->bbNum);
@@ -1356,6 +1353,9 @@ bool Compiler::fgOptimizeEmptyBlock(BasicBlock* block)
13561353
{
13571354
assert(block->isEmpty());
13581355

1356+
// We shouldn't churn the flowgraph after doing hot/cold splitting
1357+
assert(fgFirstColdBlock == nullptr);
1358+
13591359
bool madeChanges = false;
13601360
BasicBlock* bPrev = block->Prev();
13611361

@@ -1401,12 +1401,6 @@ bool Compiler::fgOptimizeEmptyBlock(BasicBlock* block)
14011401
break;
14021402
}
14031403

1404-
// can't allow fall through into cold code
1405-
if (block->IsLastHotBlock(this))
1406-
{
1407-
break;
1408-
}
1409-
14101404
// Don't remove fgEntryBB
14111405
if (block == fgEntryBB)
14121406
{
@@ -5606,6 +5600,9 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh
56065600

56075601
noway_assert(opts.OptimizationEnabled());
56085602

5603+
// We shouldn't be churning the flowgraph after doing hot/cold splitting
5604+
assert(fgFirstColdBlock == nullptr);
5605+
56095606
#ifdef DEBUG
56105607
if (verbose && !isPhase)
56115608
{
@@ -5762,9 +5759,7 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh
57625759
bNext->KindIs(BBJ_ALWAYS) && // the next block is a BBJ_ALWAYS block
57635760
!bNext->JumpsToNext() && // and it doesn't jump to the next block (we might compact them)
57645761
bNext->isEmpty() && // and it is an empty block
5765-
!bNext->TargetIs(bNext) && // special case for self jumps
5766-
!bDest->IsFirstColdBlock(this) &&
5767-
!fgInDifferentRegions(block, bDest)) // do not cross hot/cold sections
5762+
!bNext->TargetIs(bNext)) // special case for self jumps
57685763
{
57695764
assert(block->FalseTargetIs(bNext));
57705765

@@ -5808,20 +5803,6 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh
58085803
optimizeJump = false;
58095804
}
58105805

5811-
// If we are optimizing using real profile weights
5812-
// then don't optimize a conditional jump to an unconditional jump
5813-
// until after we have computed the edge weights
5814-
//
5815-
if (fgIsUsingProfileWeights())
5816-
{
5817-
// if block and bDest are in different hot/cold regions we can't do this optimization
5818-
// because we can't allow fall-through into the cold region.
5819-
if (fgInDifferentRegions(block, bDest))
5820-
{
5821-
optimizeJump = false;
5822-
}
5823-
}
5824-
58255806
if (optimizeJump && isJumpToJoinFree)
58265807
{
58275808
// In the join free case, we also need to move bDest right after bNext
@@ -5910,12 +5891,6 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh
59105891
/* Mark the block as removed */
59115892
bNext->SetFlags(BBF_REMOVED);
59125893

5913-
// If this is the first Cold basic block update fgFirstColdBlock
5914-
if (bNext->IsFirstColdBlock(this))
5915-
{
5916-
fgFirstColdBlock = bNext->Next();
5917-
}
5918-
59195894
//
59205895
// If we removed the end of a try region or handler region
59215896
// we will need to update ebdTryLast or ebdHndLast.

0 commit comments

Comments
 (0)