JIT: fix inliner profile consistency computation (#109714)

It's possible for the JIT to inline a profiled inlinee into an unprofiled context, and then have a subsequent inline fold a profiled branch. If so we may see a case where the folded edges don't have profile information.

Tolerate this.

Fixes #109657

Re-morphing of a statement during early-prop may mistakenly believe it has altered the flow graph and so invalidates DFS. Value numbering is not set up to handle this and crashes. Since this seems like a rare occurrence, have morph detect if it has really changed the flowgraph and avoid invalidating the DFS when it hasn't.

Fixes #109730

src/coreclr/jit/fginline.cpp

@@ -664,30 +664,44 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
                 //
                 if (block->hasProfileWeight())
                 {
-                    bool           repairWasComplete = true;
-                    weight_t const weight            = removedEdge->getLikelyWeight();
+                    bool           repairWasComplete  = true;
+                    bool           missingProfileData = false;
+                    weight_t const weight             = removedEdge->getLikelyWeight();
 
                     if (weight > 0)
                     {
                         // Target block weight will increase.
                         //
                         BasicBlock* const target = block->GetTarget();
-                        assert(target->hasProfileWeight());
-                        target->setBBProfileWeight(target->bbWeight + weight);
+
+                        // We may have a profiled inlinee in an unprofiled method
+                        //
+                        if (target->hasProfileWeight())
+                        {
+                            target->setBBProfileWeight(target->bbWeight + weight);
+                            missingProfileData = true;
+                        }
 
                         // Alternate weight will decrease
                         //
                         BasicBlock* const alternate = removedEdge->getDestinationBlock();
-                        assert(alternate->hasProfileWeight());
-                        weight_t const alternateNewWeight = alternate->bbWeight - weight;
 
-                        // If profile weights are consistent, expect at worst a slight underflow.
-                        //
-                        if (m_compiler->fgPgoConsistent && (alternateNewWeight < 0))
+                        if (alternate->hasProfileWeight())
                         {
-                            assert(m_compiler->fgProfileWeightsEqual(alternateNewWeight, 0));
+                            weight_t const alternateNewWeight = alternate->bbWeight - weight;
+
+                            // If profile weights are consistent, expect at worst a slight underflow.
+                            //
+                            if (m_compiler->fgPgoConsistent && (alternateNewWeight < 0))
+                            {
+                                assert(m_compiler->fgProfileWeightsEqual(alternateNewWeight, 0));
+                            }
+                            alternate->setBBProfileWeight(max(0.0, alternateNewWeight));
+                        }
+                        else
+                        {
+                            missingProfileData = true;
                         }
-                        alternate->setBBProfileWeight(max(0.0, alternateNewWeight));
 
                         // This will affect profile transitively, so in general
                         // the profile will become inconsistent.
@@ -698,13 +712,18 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
                         // block's postdominator is target's target (simple
                         // if/then/else/join).
                         //
-                        if (target->KindIs(BBJ_ALWAYS))
+                        if (!missingProfileData && target->KindIs(BBJ_ALWAYS))
                         {
                             repairWasComplete =
                                 alternate->KindIs(BBJ_ALWAYS) && alternate->TargetIs(target->GetTarget());
                         }
                     }
 
+                    if (missingProfileData)
+                    {
+                        JITDUMP("Profile data could not be locally repaired. Data was missing.\n");
+                    }
+
                     if (!repairWasComplete)
                     {
                         JITDUMP("Profile data could not be locally repaired. Data %s inconsistent.\n",
@@ -1503,8 +1522,13 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
             // Inlinee contains just one BB. So just insert its statement list to topBlock.
             if (InlineeCompiler->fgFirstBB->bbStmtList != nullptr)
             {
+                JITDUMP("\nInserting inlinee code into " FMT_BB "\n", iciBlock->bbNum);
                 stmtAfter = fgInsertStmtListAfter(iciBlock, stmtAfter, InlineeCompiler->fgFirstBB->firstStmt());
             }
+            else
+            {
+                JITDUMP("\ninlinee was empty\n");
+            }
 
             // Copy inlinee bbFlags to caller bbFlags.
             const BasicBlockFlags inlineeBlockFlags = InlineeCompiler->fgFirstBB->GetFlagsRaw();
@@ -1539,16 +1563,24 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
             fgInlineAppendStatements(pInlineInfo, iciBlock, stmtAfter);
             insertInlineeBlocks = false;
         }
+        else
+        {
+            JITDUMP("\ninlinee was single-block, but not BBJ_RETURN\n");
+        }
     }
 
     //
     // ======= Inserting inlinee's basic blocks ===============
     //
     if (insertInlineeBlocks)
     {
+        JITDUMP("\nInserting inlinee blocks\n");
         bottomBlock              = fgSplitBlockAfterStatement(topBlock, stmtAfter);
         unsigned const baseBBNum = fgBBNumMax;
 
+        JITDUMP("split " FMT_BB " after the inlinee call site; after portion is now " FMT_BB "\n", topBlock->bbNum,
+                bottomBlock->bbNum);
+
         // The newly split block is not special so doesn't need to be kept.
         //
         bottomBlock->RemoveFlags(BBF_DONT_REMOVE);
@@ -1583,7 +1615,7 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
             if (block->KindIs(BBJ_RETURN))
             {
                 noway_assert(!block->HasFlag(BBF_HAS_JMP));
-                JITDUMP("\nConvert bbKind of " FMT_BB " to BBJ_ALWAYS to bottomBlock " FMT_BB "\n", block->bbNum,
+                JITDUMP("\nConvert bbKind of " FMT_BB " to BBJ_ALWAYS to bottom block " FMT_BB "\n", block->bbNum,
                         bottomBlock->bbNum);
 
                 FlowEdge* const newEdge = fgAddRefPred(bottomBlock, block);

src/coreclr/jit/morph.cpp

@@ -12949,10 +12949,12 @@ bool Compiler::fgMorphBlockStmt(BasicBlock*     block,
         // We should not convert it to a ThrowBB.
         if ((block != fgFirstBB) || !fgFirstBB->HasFlag(BBF_INTERNAL))
         {
-            // Convert block to a throw bb
+            // Convert block to a throw bb, or make it rarely run if already a throw.
+            //
+            const bool isThrow = block->KindIs(BBJ_THROW);
             fgConvertBBToThrowBB(block);
 
-            if (invalidateDFSTreeOnFGChange)
+            if (!isThrow && invalidateDFSTreeOnFGChange)
             {
                 fgInvalidateDfsTree();
             }

src/coreclr/jit/valuenum.cpp

@@ -10806,6 +10806,9 @@ PhaseStatus Compiler::fgValueNumber()
         }
     }
 
+    assert(m_dfsTree != nullptr);
+    assert(m_loops != nullptr);
+
     m_blockToLoop = BlockToNaturalLoopMap::Build(m_loops);
     // Compute the side effects of loops.
     optComputeLoopSideEffects();