From 5b965288666486f993ecfcb21fab279f5129bf8d Mon Sep 17 00:00:00 2001
From: Aman Khalid <amankhalid@microsoft.com>
Date: Sat, 15 Jun 2024 01:24:59 +0000
Subject: [PATCH] JIT: Consider block weights instead of BBF_RUN_RARELY flag in
 fgMoveColdBlocks (#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.
---
 src/coreclr/jit/block.h   |  6 ++++++
 src/coreclr/jit/fgopt.cpp | 14 ++++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h
index b0baaf941832f..054edae23113b 100644
--- a/src/coreclr/jit/block.h
+++ b/src/coreclr/jit/block.h
@@ -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
@@ -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;
diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp
index de2b6a8f37c93..305af19b91df3 100644
--- a/src/coreclr/jit/fgopt.cpp
+++ b/src/coreclr/jit/fgopt.cpp
@@ -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;
             }
@@ -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)
@@ -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;
         }
@@ -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;
         }
@@ -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);