Add edge likelihood dumping; fix one edge likelihood update case

src/coreclr/jit/block.cpp

@@ -79,11 +79,20 @@ void FlowEdge::setLikelihood(weight_t likelihood)
 {
     assert(likelihood >= 0.0);
     assert(likelihood <= 1.0);
+
+    if (m_likelihoodSet)
+    {
+        JITDUMP("setting likelihood of " FMT_BB " -> " FMT_BB " from " FMT_WT " to " FMT_WT "\n", m_sourceBlock->bbNum,
+                m_destBlock->bbNum, m_likelihood, likelihood);
+    }
+    else
+    {
+        JITDUMP("setting likelihood of " FMT_BB " -> " FMT_BB " to " FMT_WT "\n", m_sourceBlock->bbNum,
+                m_destBlock->bbNum, likelihood);
+    }
+
     m_likelihoodSet = true;
     m_likelihood    = likelihood;
-
-    JITDUMP("setting likelihood of " FMT_BB " -> " FMT_BB " to " FMT_WT "\n", m_sourceBlock->bbNum, m_destBlock->bbNum,
-            m_likelihood);
 }
 
 //------------------------------------------------------------------------
@@ -114,10 +123,11 @@ void FlowEdge::addLikelihood(weight_t addedLikelihood)
 
     assert(newLikelihood >= 0.0);
     assert(newLikelihood <= 1.0);
-    m_likelihood = newLikelihood;
 
-    JITDUMP("updating likelihood of " FMT_BB " -> " FMT_BB " to " FMT_WT "\n", m_sourceBlock->bbNum, m_destBlock->bbNum,
-            m_likelihood);
+    JITDUMP("updating likelihood of " FMT_BB " -> " FMT_BB " from " FMT_WT " to " FMT_WT "\n", m_sourceBlock->bbNum,
+            m_destBlock->bbNum, m_likelihood, newLikelihood);
+
+    m_likelihood = newLikelihood;
 }
 
 //------------------------------------------------------------------------
@@ -678,20 +688,33 @@ void BasicBlock::dspSuccs(Compiler* compiler)
 // things strictly.
 void BasicBlock::dspKind() const
 {
-    auto dspBlockNum = [](const BasicBlock* b) -> const char* {
+    auto dspBlockNum = [](const FlowEdge* e) -> const char* {
         static char buffers[3][64]; // static array of 3 to allow 3 concurrent calls in one printf()
         static int  nextBufferIndex = 0;
 
-        auto& buffer    = buffers[nextBufferIndex];
-        nextBufferIndex = (nextBufferIndex + 1) % ArrLen(buffers);
+        auto& buffer              = buffers[nextBufferIndex];
+        nextBufferIndex           = (nextBufferIndex + 1) % ArrLen(buffers);
+        const size_t sizeOfBuffer = ArrLen(buffer);
+        int          written;
 
+        const BasicBlock* b = e->getDestinationBlock();
         if (b == nullptr)
         {
-            _snprintf_s(buffer, ArrLen(buffer), ArrLen(buffer), "NULL");
+            written = _snprintf_s(buffer, sizeOfBuffer, sizeOfBuffer, "NULL");
         }
         else
         {
-            _snprintf_s(buffer, ArrLen(buffer), ArrLen(buffer), FMT_BB, b->bbNum);
+            written = _snprintf_s(buffer, sizeOfBuffer, sizeOfBuffer, FMT_BB, b->bbNum);
+        }
+
+        const bool printEdgeLikelihoods = true; // TODO: parameterize this?
+        if (printEdgeLikelihoods)
+        {
+            if (e->hasLikelihood())
+            {
+                written = _snprintf_s(buffer + written, sizeOfBuffer - written, sizeOfBuffer - written, "(" FMT_WT ")",
+                                      e->getLikelihood());
+            }
         }
 
         return buffer;
@@ -715,7 +738,7 @@ void BasicBlock::dspKind() const
 
                 for (unsigned i = 0; i < jumpCnt; i++)
                 {
-                    printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]->getDestinationBlock()));
+                    printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]));
                 }
             }
 
@@ -728,11 +751,11 @@ void BasicBlock::dspKind() const
             break;
 
         case BBJ_EHFILTERRET:
-            printf(" -> %s (fltret)", dspBlockNum(GetTarget()));
+            printf(" -> %s (fltret)", dspBlockNum(GetTargetEdge()));
             break;
 
         case BBJ_EHCATCHRET:
-            printf(" -> %s (cret)", dspBlockNum(GetTarget()));
+            printf(" -> %s (cret)", dspBlockNum(GetTargetEdge()));
             break;
 
         case BBJ_THROW:
@@ -746,28 +769,28 @@ void BasicBlock::dspKind() const
         case BBJ_ALWAYS:
             if (HasFlag(BBF_KEEP_BBJ_ALWAYS))
             {
-                printf(" -> %s (ALWAYS)", dspBlockNum(GetTarget()));
+                printf(" -> %s (ALWAYS)", dspBlockNum(GetTargetEdge()));
             }
             else
             {
-                printf(" -> %s (always)", dspBlockNum(GetTarget()));
+                printf(" -> %s (always)", dspBlockNum(GetTargetEdge()));
             }
             break;
 
         case BBJ_LEAVE:
-            printf(" -> %s (leave)", dspBlockNum(GetTarget()));
+            printf(" -> %s (leave)", dspBlockNum(GetTargetEdge()));
             break;
 
         case BBJ_CALLFINALLY:
-            printf(" -> %s (callf)", dspBlockNum(GetTarget()));
+            printf(" -> %s (callf)", dspBlockNum(GetTargetEdge()));
             break;
 
         case BBJ_CALLFINALLYRET:
-            printf(" -> %s (callfr)", dspBlockNum(GetTarget()));
+            printf(" -> %s (callfr)", dspBlockNum(GetTargetEdge()));
             break;
 
         case BBJ_COND:
-            printf(" -> %s,%s (cond)", dspBlockNum(GetTrueTarget()), dspBlockNum(GetFalseTarget()));
+            printf(" -> %s,%s (cond)", dspBlockNum(GetTrueEdge()), dspBlockNum(GetFalseEdge()));
             break;
 
         case BBJ_SWITCH:
@@ -779,7 +802,7 @@ void BasicBlock::dspKind() const
 
             for (unsigned i = 0; i < jumpCnt; i++)
             {
-                printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]->getDestinationBlock()));
+                printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]));
 
                 const bool isDefault = bbSwtTargets->bbsHasDefault && (i == jumpCnt - 1);
                 if (isDefault)

src/coreclr/jit/block.h

@@ -46,9 +46,13 @@ typedef BitVec_ValRet_T ASSERT_VALRET_TP;
 // Use this format for loop indices
 #define FMT_LP "L%02u"
 
-// And this format for profile weights
+// Use this format for profile weights
 #define FMT_WT "%.7g"
 
+// Use this format for profile weights where we want to conserve horizontal space, at the expense of displaying
+// less precision.
+#define FMT_WT_NARROW "%.3g"
+
 /*****************************************************************************
  *
  *  Each basic block ends with a jump which is described as a value
@@ -1037,6 +1041,27 @@ struct BasicBlock : private LIR::Range
         return (bbFalseEdge == nullptr) ? nullptr : bbFalseEdge->getDestinationBlock();
     }
 
+    // Return the target edge; it might be null. Only used during dumping.
+    FlowEdge* GetTargetEdgeRaw() const
+    {
+        assert(HasTarget());
+        return bbTargetEdge;
+    }
+
+    // Return the BBJ_COND true target edge; it might be null. Only used during dumping.
+    FlowEdge* GetTrueEdgeRaw() const
+    {
+        assert(KindIs(BBJ_COND));
+        return bbTrueEdge;
+    }
+
+    // Return the BBJ_COND false target edge; it might be null. Only used during dumping.
+    FlowEdge* GetFalseEdgeRaw() const
+    {
+        assert(KindIs(BBJ_COND));
+        return bbFalseEdge;
+    }
+
 #endif // DEBUG
 
 private:
@@ -1537,7 +1562,7 @@ struct BasicBlock : private LIR::Range
     }
 
     // PredBlocksEditing: convenience method for enabling range-based `for` iteration over predecessor blocks, e.g.:
-    //    for (BasicBlock* const predBlock : block->PredBlocksList()) ...
+    //    for (BasicBlock* const predBlock : block->PredBlocksEditing()) ...
     // This iterator tolerates modifications to bbPreds.
     //
     PredBlockList<true> PredBlocksEditing() const

src/coreclr/jit/clrjit.natvis

@@ -8,9 +8,8 @@ The .NET Foundation licenses this file to you under the MIT license.
 <!--
 Visual Studio debugger visualizers for RyuJIT.
 
-Documentation for VS natvis format: https://docs.microsoft.com/en-us/visualstudio/debugger/create-custom-views-of-native-objects?view=vs-2019
-
-Documentation for VS debugger format specifiers: https://docs.microsoft.com/en-us/visualstudio/debugger/format-specifiers-in-cpp?view=vs-2019
+Documentation for VS natvis format: https://learn.microsoft.com/en-us/visualstudio/debugger/create-custom-views-of-native-objects?view=vs-2022
+Documentation for VS debugger format specifiers: https://learn.microsoft.com/en-us/visualstudio/debugger/format-specifiers-in-cpp?view=vs-2022
 -->
 
 <AutoVisualizer xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">
@@ -27,6 +26,11 @@ Documentation for VS debugger format specifiers: https://docs.microsoft.com/en-u
     <DisplayString>BB{bbNum,d}; {bbKind,en}</DisplayString>
   </Type>
 
+  <Type Name="FlowEdge">
+    <DisplayString Condition="m_dupCount!=1">BB{m_sourceBlock->bbNum,d}->BB{m_destBlock->bbNum,d} ({m_likelihood,g}) (dup {m_dupCount,d})</DisplayString>
+    <DisplayString>BB{m_sourceBlock->bbNum,d}->BB{m_destBlock->bbNum,d} ({m_likelihood,g})</DisplayString>
+  </Type>
+
   <Type Name="Compiler::LoopDsc">
     <DisplayString Condition="lpFlags &amp; LPFLG_REMOVED">REMOVED</DisplayString>
     <DisplayString Condition="lpFlags &amp; LPFLG_HAS_PREHEAD">[BB{lpTop->bbNum,d}..BB{lpBottom->bbNum,d}] pre-h:BB{lpHead->bbNum,d} e:BB{lpEntry->bbNum,d} {lpFlags,en}</DisplayString>

src/coreclr/jit/compiler.h

@@ -1584,7 +1584,7 @@ enum class ProfileChecks : unsigned int
     CHECK_NONE          = 0,
     CHECK_CLASSIC       = 1 << 0, // check "classic" jit weights
     CHECK_HASLIKELIHOOD = 1 << 1, // check all FlowEdges for hasLikelihood
-    CHECK_LIKELIHOODSUM = 1 << 2, // check block succesor likelihoods sum to 1                              
+    CHECK_LIKELIHOODSUM = 1 << 2, // check block successor likelihoods sum to 1                              
     CHECK_LIKELY        = 1 << 3, // fully check likelihood based weights
     RAISE_ASSERT        = 1 << 4, // assert on check failure
     CHECK_ALL_BLOCKS    = 1 << 5, // check blocks even if bbHasProfileWeight is false
@@ -6074,7 +6074,11 @@ class Compiler
 
     void fgDispBBLiveness(BasicBlock* block);
     void fgDispBBLiveness();
-    void fgTableDispBasicBlock(const BasicBlock* block, const BasicBlock* nextBlock = nullptr, int blockTargetFieldWidth = 21, int ibcColWidth = 0);
+    void fgTableDispBasicBlock(const BasicBlock* block,
+        const BasicBlock* nextBlock = nullptr,
+        bool printEdgeLikelihoods = true,
+        int blockTargetFieldWidth = 21,
+        int ibcColWidth = 0);
     void fgDispBasicBlocks(BasicBlock* firstBlock, BasicBlock* lastBlock, bool dumpTrees);
     void fgDispBasicBlocks(bool dumpTrees = false);
     void fgDumpStmtTree(const BasicBlock* block, Statement* stmt);

src/coreclr/jit/fgbasic.cpp

@@ -686,20 +686,56 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas
 
         case BBJ_SWITCH:
         {
-            unsigned const   jumpCnt = block->GetSwitchTargets()->bbsCount;
-            FlowEdge** const jumpTab = block->GetSwitchTargets()->bbsDstTab;
-            bool             changed = false;
+            unsigned const   jumpCnt      = block->GetSwitchTargets()->bbsCount;
+            FlowEdge** const jumpTab      = block->GetSwitchTargets()->bbsDstTab;
+            bool             existingEdge = false;
+            FlowEdge*        oldEdge      = nullptr;
+            FlowEdge*        newEdge      = nullptr;
+            bool             changed      = false;
 
             for (unsigned i = 0; i < jumpCnt; i++)
             {
+                if (jumpTab[i]->getDestinationBlock() == newTarget)
+                {
+                    // The new target already has an edge from this switch statement.
+                    // We'll need to add the likelihood from the edge we're redirecting
+                    // to the existing edge. Note that if there is no existing edge,
+                    // then we'll copy the likelihood from the existing edge we pass to
+                    // `fgAddRefPred`. Note also that we can visit the same edge multiple
+                    // times if there are multiple switch cases with the same target. The
+                    // edge has a dup count and a single likelihood for all the possible
+                    // paths to the target, so we only want to add the likelihood once
+                    // despite visiting the duplicated edges in the `jumpTab` array
+                    // multiple times.
+                    existingEdge = true;
+                }
+
                 if (jumpTab[i]->getDestinationBlock() == oldTarget)
                 {
-                    fgRemoveRefPred(jumpTab[i]);
-                    jumpTab[i] = fgAddRefPred(newTarget, block, jumpTab[i]);
+                    assert((oldEdge == nullptr) || (oldEdge == jumpTab[i]));
+                    oldEdge = jumpTab[i];
+                    fgRemoveRefPred(oldEdge);
+                    newEdge    = fgAddRefPred(newTarget, block, oldEdge);
+                    jumpTab[i] = newEdge;
                     changed    = true;
                 }
             }
 
+            if (existingEdge)
+            {
+                assert(oldEdge != nullptr);
+                assert(oldEdge->getSourceBlock() == block);
+                assert(oldEdge->getDestinationBlock() == oldTarget);
+                assert(newEdge != nullptr);
+                assert(newEdge->getSourceBlock() == block);
+                assert(newEdge->getDestinationBlock() == newTarget);
+
+                if (newEdge->hasLikelihood() && oldEdge->hasLikelihood())
+                {
+                    newEdge->addLikelihood(oldEdge->getLikelihood());
+                }
+            }
+
             assert(changed);
             InvalidateUniqueSwitchSuccMap();
             break;