JIT: Remove BBF_RUN_RARELY (#118147)

Author: amanasifkhalid

src/coreclr/jit/async.cpp

@@ -1730,10 +1730,8 @@ BasicBlock* AsyncTransformation::CreateResumption(BasicBlock*               bloc
     BasicBlock* resumeBB      = m_comp->fgNewBBafter(BBJ_ALWAYS, m_lastResumptionBB, true);
     FlowEdge*   remainderEdge = m_comp->fgAddRefPred(remainder, resumeBB);
 
-    // It does not really make sense to inherit from the target, but given this
-    // is always 0% this just propagates the profile weight flag + sets
-    // BBF_RUN_RARELY.
-    resumeBB->inheritWeightPercentage(remainder, 0);
+    resumeBB->bbSetRunRarely();
+    resumeBB->CopyFlags(remainder, BBF_PROF_WEIGHT);
     resumeBB->SetTargetEdge(remainderEdge);
     resumeBB->clearTryIndex();
     resumeBB->clearHndIndex();

src/coreclr/jit/block.cpp

@@ -491,7 +491,6 @@ void BasicBlock::dspFlags() const
         {BBF_IMPORTED, "i"},
         {BBF_IS_LIR, "LIR"},
         {BBF_PROF_WEIGHT, "IBC"},
-        {BBF_RUN_RARELY, "rare"},
         {BBF_MARKED, "m"},
         {BBF_REMOVED, "del"},
         {BBF_DONT_REMOVE, "keep"},

src/coreclr/jit/block.h

@@ -415,37 +415,36 @@ enum BasicBlockFlags : uint64_t
     BBF_CLONED_FINALLY_BEGIN = MAKE_BBFLAG( 7), // First block of a cloned finally region
     BBF_CLONED_FINALLY_END   = MAKE_BBFLAG( 8), // Last block of a cloned finally region
     BBF_HAS_SUPPRESSGC_CALL  = MAKE_BBFLAG( 9), // BB contains a call to a method with SuppressGCTransitionAttribute
-    BBF_RUN_RARELY           = MAKE_BBFLAG(10), // BB is rarely run (catch clauses, blocks with throws etc)
-    BBF_HAS_LABEL            = MAKE_BBFLAG(11), // BB needs a label
-    BBF_LOOP_ALIGN           = MAKE_BBFLAG(12), // Block is lexically the first block in a loop we intend to align.
-    BBF_HAS_ALIGN            = MAKE_BBFLAG(13), // BB ends with 'align' instruction
-    BBF_HAS_JMP              = MAKE_BBFLAG(14), // BB executes a JMP instruction (instead of return)
-    BBF_GC_SAFE_POINT        = MAKE_BBFLAG(15), // BB has a GC safe point (e.g. a call)
-    BBF_HAS_MDARRAYREF       = MAKE_BBFLAG(16), // Block has a multi-dimensional array reference
-    BBF_HAS_NEWOBJ           = MAKE_BBFLAG(17), // BB contains 'new' of an object type.
-
-    BBF_RETLESS_CALL                   = MAKE_BBFLAG(18), // BBJ_CALLFINALLY that will never return (and therefore, won't need a paired
+    BBF_HAS_LABEL            = MAKE_BBFLAG(10), // BB needs a label
+    BBF_LOOP_ALIGN           = MAKE_BBFLAG(11), // Block is lexically the first block in a loop we intend to align.
+    BBF_HAS_ALIGN            = MAKE_BBFLAG(12), // BB ends with 'align' instruction
+    BBF_HAS_JMP              = MAKE_BBFLAG(13), // BB executes a JMP instruction (instead of return)
+    BBF_GC_SAFE_POINT        = MAKE_BBFLAG(14), // BB has a GC safe point (e.g. a call)
+    BBF_HAS_MDARRAYREF       = MAKE_BBFLAG(15), // Block has a multi-dimensional array reference
+    BBF_HAS_NEWOBJ           = MAKE_BBFLAG(16), // BB contains 'new' of an object type.
+
+    BBF_RETLESS_CALL                   = MAKE_BBFLAG(17), // BBJ_CALLFINALLY that will never return (and therefore, won't need a paired
                                                           // BBJ_CALLFINALLYRET); see isBBCallFinallyPair().
-    BBF_COLD                           = MAKE_BBFLAG(19), // BB is cold
-    BBF_PROF_WEIGHT                    = MAKE_BBFLAG(20), // BB weight is computed from profile data
-    BBF_KEEP_BBJ_ALWAYS                = MAKE_BBFLAG(21), // A special BBJ_ALWAYS block, used by EH code generation. Keep the jump kind
+    BBF_COLD                           = MAKE_BBFLAG(18), // BB is cold
+    BBF_PROF_WEIGHT                    = MAKE_BBFLAG(19), // BB weight is computed from profile data
+    BBF_KEEP_BBJ_ALWAYS                = MAKE_BBFLAG(20), // A special BBJ_ALWAYS block, used by EH code generation. Keep the jump kind
                                                           // as BBJ_ALWAYS. Used on x86 for the final step block out of a finally.
-    BBF_HAS_CALL                       = MAKE_BBFLAG(22), // BB contains a call
-    BBF_DOMINATED_BY_EXCEPTIONAL_ENTRY = MAKE_BBFLAG(23), // Block is dominated by exceptional entry.
-    BBF_BACKWARD_JUMP                  = MAKE_BBFLAG(24), // BB is surrounded by a backward jump/switch arc
-    BBF_BACKWARD_JUMP_SOURCE           = MAKE_BBFLAG(25), // Block is a source of a backward jump
-    BBF_BACKWARD_JUMP_TARGET           = MAKE_BBFLAG(26), // Block is a target of a backward jump
-    BBF_PATCHPOINT                     = MAKE_BBFLAG(27), // Block is a patchpoint
-    BBF_PARTIAL_COMPILATION_PATCHPOINT = MAKE_BBFLAG(28), // Block is a partial compilation patchpoint
-    BBF_HAS_HISTOGRAM_PROFILE          = MAKE_BBFLAG(29), // BB contains a call needing a histogram profile
-    BBF_TAILCALL_SUCCESSOR             = MAKE_BBFLAG(30), // BB has pred that has potential tail call
-    BBF_RECURSIVE_TAILCALL             = MAKE_BBFLAG(31), // Block has recursive tailcall that may turn into a loop
-    BBF_NO_CSE_IN                      = MAKE_BBFLAG(32), // Block should kill off any incoming CSE
-    BBF_CAN_ADD_PRED                   = MAKE_BBFLAG(33), // Ok to add pred edge to this block, even when "safe" edge creation disabled
-    BBF_HAS_VALUE_PROFILE              = MAKE_BBFLAG(34), // Block has a node that needs a value probing
-    BBF_HAS_NEWARR                     = MAKE_BBFLAG(35), // BB contains 'new' of an array type.
-    BBF_MAY_HAVE_BOUNDS_CHECKS         = MAKE_BBFLAG(36), // BB *likely* has a bounds check (after rangecheck phase).
-    BBF_ASYNC_RESUMPTION               = MAKE_BBFLAG(37), // Block is a resumption block in an async method
+    BBF_HAS_CALL                       = MAKE_BBFLAG(21), // BB contains a call
+    BBF_DOMINATED_BY_EXCEPTIONAL_ENTRY = MAKE_BBFLAG(22), // Block is dominated by exceptional entry.
+    BBF_BACKWARD_JUMP                  = MAKE_BBFLAG(23), // BB is surrounded by a backward jump/switch arc
+    BBF_BACKWARD_JUMP_SOURCE           = MAKE_BBFLAG(24), // Block is a source of a backward jump
+    BBF_BACKWARD_JUMP_TARGET           = MAKE_BBFLAG(25), // Block is a target of a backward jump
+    BBF_PATCHPOINT                     = MAKE_BBFLAG(26), // Block is a patchpoint
+    BBF_PARTIAL_COMPILATION_PATCHPOINT = MAKE_BBFLAG(27), // Block is a partial compilation patchpoint
+    BBF_HAS_HISTOGRAM_PROFILE          = MAKE_BBFLAG(28), // BB contains a call needing a histogram profile
+    BBF_TAILCALL_SUCCESSOR             = MAKE_BBFLAG(29), // BB has pred that has potential tail call
+    BBF_RECURSIVE_TAILCALL             = MAKE_BBFLAG(30), // Block has recursive tailcall that may turn into a loop
+    BBF_NO_CSE_IN                      = MAKE_BBFLAG(31), // Block should kill off any incoming CSE
+    BBF_CAN_ADD_PRED                   = MAKE_BBFLAG(32), // Ok to add pred edge to this block, even when "safe" edge creation disabled
+    BBF_HAS_VALUE_PROFILE              = MAKE_BBFLAG(33), // Block has a node that needs a value probing
+    BBF_HAS_NEWARR                     = MAKE_BBFLAG(34), // BB contains 'new' of an array type.
+    BBF_MAY_HAVE_BOUNDS_CHECKS         = MAKE_BBFLAG(35), // BB *likely* has a bounds check (after rangecheck phase).
+    BBF_ASYNC_RESUMPTION               = MAKE_BBFLAG(36), // Block is a resumption block in an async method
 
     // The following are sets of flags.
 
@@ -468,7 +467,6 @@ enum BasicBlockFlags : uint64_t
     // Flags gained by the bottom block when a block is split.
     // Note, this is a conservative guess.
     // For example, the bottom block might or might not have BBF_HAS_NEWARR, but we assume it has BBF_HAS_NEWARR.
-    // TODO: Should BBF_RUN_RARELY be added to BBF_SPLIT_GAINED ?
 
     BBF_SPLIT_GAINED = BBF_DONT_REMOVE | BBF_HAS_JMP | BBF_BACKWARD_JUMP | BBF_PROF_WEIGHT | BBF_HAS_NEWARR | \
                        BBF_HAS_NEWOBJ | BBF_KEEP_BBJ_ALWAYS | BBF_CLONED_FINALLY_END | BBF_HAS_HISTOGRAM_PROFILE | BBF_HAS_VALUE_PROFILE | BBF_HAS_MDARRAYREF | BBF_NEEDS_GCPOLL | BBF_MAY_HAVE_BOUNDS_CHECKS | BBF_ASYNC_RESUMPTION,
@@ -1169,9 +1167,16 @@ struct BasicBlock : private LIR::Range
     unsigned bbRefs; // number of blocks that can reach here, either by fall-through or a branch. If this falls to zero,
                      // the block is unreachable.
 
+#define BB_UNITY_WEIGHT          100.0 // how much a normal execute once block weighs
+#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.
+
     bool isRunRarely() const
     {
-        return HasFlag(BBF_RUN_RARELY);
+        return (bbWeight == BB_ZERO_WEIGHT);
     }
 
     bool isLoopAlign() const
@@ -1196,13 +1201,6 @@ struct BasicBlock : private LIR::Range
     const char* dspToString(int blockNumPadding = 0) const;
 #endif // DEBUG
 
-#define BB_UNITY_WEIGHT          100.0 // how much a normal execute once block weighs
-#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
 
     // getCalledCount -- get the value used to normalize weights for this method
@@ -1235,15 +1233,6 @@ struct BasicBlock : private LIR::Range
     {
         this->SetFlags(BBF_PROF_WEIGHT);
         this->bbWeight = weight;
-
-        if (weight == BB_ZERO_WEIGHT)
-        {
-            this->SetFlags(BBF_RUN_RARELY);
-        }
-        else
-        {
-            this->RemoveFlags(BBF_RUN_RARELY);
-        }
     }
 
     // increaseBBProfileWeight -- Increase the profile-derived weight for a basic block
@@ -1278,44 +1267,20 @@ struct BasicBlock : private LIR::Range
     {
         assert(0 <= percentage && percentage <= 100);
 
-        this->bbWeight = (bSrc->bbWeight * percentage) / 100;
-
-        if (bSrc->hasProfileWeight())
-        {
-            this->SetFlags(BBF_PROF_WEIGHT);
-        }
-        else
-        {
-            this->RemoveFlags(BBF_PROF_WEIGHT);
-        }
-
-        if (this->bbWeight == BB_ZERO_WEIGHT)
-        {
-            this->SetFlags(BBF_RUN_RARELY);
-        }
-        else
-        {
-            this->RemoveFlags(BBF_RUN_RARELY);
-        }
+        this->bbWeight                         = (bSrc->bbWeight * percentage) / 100;
+        const BasicBlockFlags hasProfileWeight = bSrc->GetFlagsRaw() & BBF_PROF_WEIGHT;
+        this->RemoveFlags(BBF_PROF_WEIGHT);
+        this->SetFlags(hasProfileWeight);
     }
 
     // Scale a blocks' weight by some factor.
     //
     void scaleBBWeight(weight_t scale)
     {
         this->bbWeight = this->bbWeight * scale;
-
-        if (this->bbWeight == BB_ZERO_WEIGHT)
-        {
-            this->SetFlags(BBF_RUN_RARELY);
-        }
-        else
-        {
-            this->RemoveFlags(BBF_RUN_RARELY);
-        }
     }
 
-    // Set block weight to zero, and set run rarely flag.
+    // Set block weight to zero.
     //
     void bbSetRunRarely()
     {
@@ -1808,71 +1773,6 @@ struct BasicBlock : private LIR::Range
     {
     }
 
-    // Iteratable collection of successors of a block.
-    template <typename TPosition>
-    class Successors
-    {
-        Compiler*   m_comp;
-        BasicBlock* m_block;
-
-    public:
-        Successors(Compiler* comp, BasicBlock* block)
-            : m_comp(comp)
-            , m_block(block)
-        {
-        }
-
-        class iterator
-        {
-            Compiler*   m_comp;
-            BasicBlock* m_block;
-            TPosition   m_pos;
-
-        public:
-            iterator(Compiler* comp, BasicBlock* block)
-                : m_comp(comp)
-                , m_block(block)
-                , m_pos(comp, block)
-            {
-            }
-
-            iterator()
-                : m_pos()
-            {
-            }
-
-            void operator++(void)
-            {
-                m_pos.Advance(m_comp, m_block);
-            }
-
-            BasicBlock* operator*()
-            {
-                return m_pos.Current(m_comp, m_block);
-            }
-
-            bool operator==(const iterator& other)
-            {
-                return m_pos == other.m_pos;
-            }
-
-            bool operator!=(const iterator& other)
-            {
-                return m_pos != other.m_pos;
-            }
-        };
-
-        iterator begin()
-        {
-            return iterator(m_comp, m_block);
-        }
-
-        iterator end()
-        {
-            return iterator();
-        }
-    };
-
     template <typename TFunc>
     BasicBlockVisit VisitEHEnclosedHandlerSecondPassSuccs(Compiler* comp, TFunc func);
 

src/coreclr/jit/fgdiagnostic.cpp

@@ -828,7 +828,7 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos)
                 const bool isTryEntryBlock     = bbIsTryBeg(block);
                 const bool isFuncletEntryBlock = fgFuncletsCreated && bbIsFuncletBeg(block);
 
-                if (isTryEntryBlock || isFuncletEntryBlock || block->HasAnyFlag(BBF_RUN_RARELY | BBF_LOOP_ALIGN))
+                if (isTryEntryBlock || isFuncletEntryBlock || block->HasFlag(BBF_LOOP_ALIGN))
                 {
                     // Display a very few, useful, block flags
                     fprintf(fgxFile, " [");
@@ -840,10 +840,6 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos)
                     {
                         fprintf(fgxFile, "F");
                     }
-                    if (block->HasFlag(BBF_RUN_RARELY))
-                    {
-                        fprintf(fgxFile, "R");
-                    }
                     if (block->HasFlag(BBF_LOOP_ALIGN))
                     {
                         fprintf(fgxFile, "A");
@@ -3133,16 +3129,6 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef
                 }
             }
         }
-
-        /* Check if BBF_RUN_RARELY is set that we have bbWeight of zero */
-        if (block->isRunRarely())
-        {
-            assert(block->bbWeight == BB_ZERO_WEIGHT);
-        }
-        else
-        {
-            assert(block->bbWeight > BB_ZERO_WEIGHT);
-        }
     }
 
     assert(fgBBcount == numBlocks);
@@ -3942,6 +3928,12 @@ void Compiler::fgDebugCheckBlockLinks()
         // If this is a switch, check that the tables are consistent.
         if (block->KindIs(BBJ_SWITCH))
         {
+            // Switch blocks with dominant cases must have profile-derived weights.
+            if (block->GetSwitchTargets()->HasDominantCase())
+            {
+                assert(block->hasProfileWeight());
+            }
+
             // Create a set with all the successors.
             BitVecTraits bitVecTraits(fgBBNumMax + 1, this);
             BitVec       succBlocks(BitVecOps::MakeEmpty(&bitVecTraits));

src/coreclr/jit/fginline.cpp

@@ -1591,8 +1591,8 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
             noway_assert((inlineeBlockFlags & BBF_HAS_JMP) == 0);
             noway_assert((inlineeBlockFlags & BBF_KEEP_BBJ_ALWAYS) == 0);
 
-            // Todo: we may want to exclude other flags here.
-            iciBlock->SetFlags(inlineeBlockFlags & ~BBF_RUN_RARELY);
+            // Todo: we may want to exclude some flags here.
+            iciBlock->SetFlags(inlineeBlockFlags);
 
 #ifdef DEBUG
             if (verbose)

src/coreclr/jit/fgopt.cpp

@@ -2538,11 +2538,11 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
     if (fgIsUsingProfileWeights())
     {
         // Only rely upon the profile weight when all three of these blocks
-        // have either good profile weights or are rarelyRun
+        // have either good profile weights or are rarely run
         //
-        if (bJump->HasAnyFlag(BBF_PROF_WEIGHT | BBF_RUN_RARELY) &&
-            bDest->HasAnyFlag(BBF_PROF_WEIGHT | BBF_RUN_RARELY) &&
-            trueTarget->HasAnyFlag(BBF_PROF_WEIGHT | BBF_RUN_RARELY))
+        if ((bJump->hasProfileWeight() || bJump->isRunRarely()) &&
+            (bDest->hasProfileWeight() || bDest->isRunRarely()) &&
+            (trueTarget->hasProfileWeight() || trueTarget->isRunRarely()))
         {
             haveProfileWeights = true;
 
@@ -3132,7 +3132,6 @@ bool Compiler::fgExpandRarelyRunBlocks()
                 // Set the BBJ_CALLFINALLY block to the same weight as the BBJ_CALLFINALLYRET block and
                 // mark it rarely run.
                 bPrev->bbWeight = block->bbWeight;
-                bPrev->SetFlags(BBF_RUN_RARELY);
 #ifdef DEBUG
                 if (verbose)
                 {
@@ -3147,7 +3146,6 @@ bool Compiler::fgExpandRarelyRunBlocks()
                 // Set the BBJ_CALLFINALLYRET block to the same weight as the BBJ_CALLFINALLY block and
                 // mark it rarely run.
                 block->bbWeight = bPrev->bbWeight;
-                block->SetFlags(BBF_RUN_RARELY);
 #ifdef DEBUG
                 if (verbose)
                 {

src/coreclr/jit/fgprofile.cpp

@@ -4345,14 +4345,6 @@ bool Compiler::fgComputeMissingBlockWeights()
                     changed        = true;
                     modified       = true;
                     bDst->bbWeight = newWeight;
-                    if (newWeight == BB_ZERO_WEIGHT)
-                    {
-                        bDst->SetFlags(BBF_RUN_RARELY);
-                    }
-                    else
-                    {
-                        bDst->RemoveFlags(BBF_RUN_RARELY);
-                    }
                 }
             }
             else if (!bDst->hasProfileWeight() && bbIsHandlerBeg(bDst) && !bDst->isRunRarely())

src/coreclr/jit/importer.cpp

@@ -5078,7 +5078,6 @@ void Compiler::impImportLeave(BasicBlock* block)
                     }
 
                     step2->inheritWeight(block);
-                    step2->CopyFlags(block, BBF_RUN_RARELY);
                     step2->SetFlags(BBF_IMPORTED);
 
 #ifdef DEBUG
@@ -5361,10 +5360,9 @@ void Compiler::impResetLeaveBlock(BasicBlock* block, unsigned jmpAddr)
         //  b) weight zero
         //  c) prevent from being imported
         //  d) as internal
-        //  e) as rarely run
-        dupBlock->bbRefs   = 0;
-        dupBlock->bbWeight = BB_ZERO_WEIGHT;
-        dupBlock->SetFlags(BBF_IMPORTED | BBF_INTERNAL | BBF_RUN_RARELY);
+        dupBlock->bbRefs = 0;
+        dupBlock->bbSetRunRarely();
+        dupBlock->SetFlags(BBF_IMPORTED | BBF_INTERNAL);
 
         // Insert the block right after the block which is getting reset so that BBJ_CALLFINALLY and BBJ_ALWAYS
         // will be next to each other.