Skip to content

Commit

Permalink
JIT: Consider block weights instead of BBF_RUN_RARELY flag in fgMoveC…
Browse files Browse the repository at this point in the history
…oldBlocks (#103492)

Based on feedback in #102763 (comment), define "cold" blocks based on whether their weights are below a certain threshold, rather than only considering blocks marked with BBF_RUN_RARELY, in fgMoveColdBlocks. I added a BasicBlock method for doing this weight check rather than localizing it to fgMoveColdBlocks, as I plan to use it elsewhere in the layout phase.
  • Loading branch information
amanasifkhalid authored Jun 15, 2024
1 parent eb6f1ae commit 5b96528
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 6 deletions.
6 changes: 6 additions & 0 deletions src/coreclr/jit/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -1163,6 +1163,7 @@ struct BasicBlock : private LIR::Range
#define BB_UNITY_WEIGHT_UNSIGNED 100 // how much a normal execute once block weighs
#define BB_LOOP_WEIGHT_SCALE 8.0 // synthetic profile scale factor for loops
#define BB_ZERO_WEIGHT 0.0
#define BB_COLD_WEIGHT 0.01 // Upper bound for cold weights; used during block layout
#define BB_MAX_WEIGHT FLT_MAX // maximum finite weight -- needs rethinking.

weight_t bbWeight; // The dynamic execution weight of this block
Expand Down Expand Up @@ -1261,6 +1262,11 @@ struct BasicBlock : private LIR::Range
return (bbWeight >= BB_MAX_WEIGHT);
}

bool isBBWeightCold(Compiler* comp) const
{
return getBBWeight(comp) < BB_COLD_WEIGHT;
}

// Returns "true" if the block is empty. Empty here means there are no statement
// trees *except* PHI definitions.
bool isEmpty() const;
Expand Down
14 changes: 8 additions & 6 deletions src/coreclr/jit/fgopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4912,7 +4912,8 @@ void Compiler::fgMoveColdBlocks()
// as we want to keep these pairs contiguous
// (if we encounter the end of a pair below, we'll move the whole pair).
//
if (!block->isRunRarely() || block->hasTryIndex() || block->hasHndIndex() || block->isBBCallFinallyPair())
if (!block->isBBWeightCold(this) || block->hasTryIndex() || block->hasHndIndex() ||
block->isBBCallFinallyPair())
{
continue;
}
Expand All @@ -4937,7 +4938,7 @@ void Compiler::fgMoveColdBlocks()
// We have moved all cold main blocks before lastMainBB to after lastMainBB.
// If lastMainBB itself is cold, move it to the end of the method to restore its relative ordering.
//
if (lastMainBB->isRunRarely())
if (lastMainBB->isBBWeightCold(this))
{
BasicBlock* const newLastMainBB = this->fgLastBBInMainFunction();
if (lastMainBB != newLastMainBB)
Expand Down Expand Up @@ -4991,13 +4992,14 @@ void Compiler::fgMoveColdBlocks()
{
prev = block->Prev();

// Only consider rarely-run blocks in try regions.
// Only consider cold blocks in try regions.
// If we have such a block that is also part of an exception handler, don't bother moving it.
// Finally, don't move block if it is the beginning of a call-finally pair,
// as we want to keep these pairs contiguous
// (if we encounter the end of a pair below, we'll move the whole pair).
//
if (!block->hasTryIndex() || !block->isRunRarely() || block->hasHndIndex() || block->isBBCallFinallyPair())
if (!block->isBBWeightCold(this) || !block->hasTryIndex() || block->hasHndIndex() ||
block->isBBCallFinallyPair())
{
continue;
}
Expand All @@ -5008,7 +5010,7 @@ void Compiler::fgMoveColdBlocks()
// Don't move the beginning of a try region.
// Also, if this try region's entry is cold, don't bother moving its blocks.
//
if ((HBtab->ebdTryBeg == block) || (HBtab->ebdTryBeg->isRunRarely()))
if ((HBtab->ebdTryBeg == block) || HBtab->ebdTryBeg->isBBWeightCold(this))
{
continue;
}
Expand Down Expand Up @@ -5069,7 +5071,7 @@ void Compiler::fgMoveColdBlocks()
// We moved cold blocks to the end of this try region, but the old end block is cold, too.
// Move the old end block to the end of the region to preserve its relative ordering.
//
if ((tryEnd != newTryEnd) && tryEnd->isRunRarely() && !tryEnd->hasHndIndex())
if ((tryEnd != newTryEnd) && !tryEnd->hasHndIndex() && tryEnd->isBBWeightCold(this))
{
BasicBlock* const prev = tryEnd->Prev();
fgUnlinkBlock(tryEnd);
Expand Down

0 comments on commit 5b96528

Please sign in to comment.