JIT: fixes for mixed PGO/nonPGO compiles

src/coreclr/jit/block.cpp

@@ -993,21 +993,6 @@ Statement* BasicBlock::FirstNonPhiDefOrCatchArgAsg()
     return stmt;
 }
 
-/*****************************************************************************
- *
- *  Mark a block as rarely run, we also don't want to have a loop in a
- *   rarely run block, and we set it's weight to zero.
- */
-
-void BasicBlock::bbSetRunRarely()
-{
-    setBBWeight(BB_ZERO_WEIGHT);
-    if (bbWeight == BB_ZERO_WEIGHT)
-    {
-        bbFlags |= BBF_RUN_RARELY; // This block is never/rarely run
-    }
-}
-
 /*****************************************************************************
  *
  *  Can a BasicBlock be inserted after this without altering the flowgraph

src/coreclr/jit/block.h

@@ -548,17 +548,6 @@ struct BasicBlock : private LIR::Range
         return ((this->bbFlags & BBF_PROF_WEIGHT) != 0);
     }
 
-    // setBBWeight -- if the block weight is not derived from a profile,
-    // then set the weight to the input weight, making sure to not overflow BB_MAX_WEIGHT
-    // Note to set the weight from profile data, instead use setBBProfileWeight
-    void setBBWeight(weight_t weight)
-    {
-        if (!hasProfileWeight())
-        {
-            this->bbWeight = min(weight, BB_MAX_WEIGHT);
-        }
-    }
-
     // setBBProfileWeight -- Set the profile-derived weight for a basic block
     // and update the run rarely flag as appropriate.
     void setBBProfileWeight(weight_t weight)
@@ -576,16 +565,6 @@ struct BasicBlock : private LIR::Range
         }
     }
 
-    // modifyBBWeight -- same as setBBWeight, but also make sure that if the block is rarely run, it stays that
-    // way, and if it's not rarely run then its weight never drops below 1.
-    void modifyBBWeight(weight_t weight)
-    {
-        if (this->bbWeight != BB_ZERO_WEIGHT)
-        {
-            setBBWeight(max(weight, 1));
-        }
-    }
-
     // this block will inherit the same weight and relevant bbFlags as bSrc
     //
     void inheritWeight(BasicBlock* bSrc)
@@ -596,28 +575,38 @@ struct BasicBlock : private LIR::Range
     // Similar to inheritWeight(), but we're splitting a block (such as creating blocks for qmark removal).
     // So, specify a percentage (0 to 100) of the weight the block should inherit.
     //
+    // Can be invoked as a self-rescale, eg: block->inheritWeightPecentage(block, 50))
+    //
     void inheritWeightPercentage(BasicBlock* bSrc, unsigned percentage)
     {
         assert(0 <= percentage && percentage <= 100);
 
-        // Check for overflow
-        if ((bSrc->bbWeight * 100) <= bSrc->bbWeight)
+        this->bbWeight = (bSrc->bbWeight * percentage) / 100;
+
+        if (bSrc->hasProfileWeight())
         {
-            this->bbWeight = bSrc->bbWeight;
+            this->bbFlags |= BBF_PROF_WEIGHT;
         }
         else
         {
-            this->bbWeight = (bSrc->bbWeight * percentage) / 100;
+            this->bbFlags &= ~BBF_PROF_WEIGHT;
         }
 
-        if (bSrc->hasProfileWeight())
+        if (this->bbWeight == BB_ZERO_WEIGHT)
         {
-            this->bbFlags |= BBF_PROF_WEIGHT;
+            this->bbFlags |= BBF_RUN_RARELY;
         }
         else
         {
-            this->bbFlags &= ~BBF_PROF_WEIGHT;
+            this->bbFlags &= ~BBF_RUN_RARELY;
         }
+    }
+
+    // Scale a blocks' weight by some factor.
+    //
+    void scaleBBWeight(BasicBlock::weight_t scale)
+    {
+        this->bbWeight = this->bbWeight * scale;
 
         if (this->bbWeight == BB_ZERO_WEIGHT)
         {
@@ -629,6 +618,13 @@ struct BasicBlock : private LIR::Range
         }
     }
 
+    // Set block weight to zero, and set run rarely flag.
+    //
+    void bbSetRunRarely()
+    {
+        this->scaleBBWeight(BB_ZERO_WEIGHT);
+    }
+
     // makeBlockHot()
     //     This is used to override any profiling data
     //     and force a block to be in the hot region.
@@ -1044,7 +1040,6 @@ struct BasicBlock : private LIR::Range
     unsigned      bbStackDepthOnEntry();
     void bbSetStack(void* stackBuffer);
     StackEntry* bbStackOnEntry();
-    void        bbSetRunRarely();
 
     // "bbNum" is one-based (for unknown reasons); it is sometimes useful to have the corresponding
     // zero-based number for use as an array index.

src/coreclr/jit/codegencommon.cpp

@@ -2179,7 +2179,7 @@ void CodeGen::genGenerateMachineCode()
 
         if (compiler->fgHaveProfileData())
         {
-            printf("; with IBC profile data, edge weights are %s, and fgCalledCount is %.0f\n",
+            printf("; with PGO: edge weights are %s, and fgCalledCount is " FMT_WT "\n",
                    compiler->fgHaveValidEdgeWeights ? "valid" : "invalid", compiler->fgCalledCount);
         }
 
@@ -2188,6 +2188,12 @@ void CodeGen::genGenerateMachineCode()
             printf("; %s\n", compiler->fgPgoFailReason);
         }
 
+        if ((compiler->fgPgoInlineePgo + compiler->fgPgoInlineeNoPgo + compiler->fgPgoInlineeNoPgoSingleBlock) > 0)
+        {
+            printf("; %u inlinees with PGO data; %u single block inlinees; %u inlinees without PGO data\n",
+                   compiler->fgPgoInlineePgo, compiler->fgPgoInlineeNoPgoSingleBlock, compiler->fgPgoInlineeNoPgo);
+        }
+
         if (compiler->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_ALT_JIT))
         {
             printf("; invoked as altjit\n");

src/coreclr/jit/compiler.h

@@ -5653,6 +5653,9 @@ class Compiler
     UINT32                                 fgPgoBlockCounts;
     UINT32                                 fgPgoEdgeCounts;
     UINT32                                 fgPgoClassProfiles;
+    unsigned                               fgPgoInlineePgo;
+    unsigned                               fgPgoInlineeNoPgo;
+    unsigned                               fgPgoInlineeNoPgoSingleBlock;
 
     void WalkSpanningTree(SpanningTreeVisitor* visitor);
     void fgSetProfileWeight(BasicBlock* block, BasicBlock::weight_t weight);

src/coreclr/jit/fgbasic.cpp

@@ -164,18 +164,21 @@ void Compiler::fgInit()
     fgPreviousCandidateSIMDFieldAsgStmt = nullptr;
 #endif
 
-    fgHasSwitch          = false;
-    fgPgoDisabled        = false;
-    fgPgoSchema          = nullptr;
-    fgPgoData            = nullptr;
-    fgPgoSchemaCount     = 0;
-    fgNumProfileRuns     = 0;
-    fgPgoBlockCounts     = 0;
-    fgPgoEdgeCounts      = 0;
-    fgPgoClassProfiles   = 0;
-    fgCountInstrumentor  = nullptr;
-    fgClassInstrumentor  = nullptr;
-    fgPredListSortVector = nullptr;
+    fgHasSwitch                  = false;
+    fgPgoDisabled                = false;
+    fgPgoSchema                  = nullptr;
+    fgPgoData                    = nullptr;
+    fgPgoSchemaCount             = 0;
+    fgNumProfileRuns             = 0;
+    fgPgoBlockCounts             = 0;
+    fgPgoEdgeCounts              = 0;
+    fgPgoClassProfiles           = 0;
+    fgPgoInlineePgo              = 0;
+    fgPgoInlineeNoPgo            = 0;
+    fgPgoInlineeNoPgoSingleBlock = 0;
+    fgCountInstrumentor          = nullptr;
+    fgClassInstrumentor          = nullptr;
+    fgPredListSortVector         = nullptr;
 }
 
 /*****************************************************************************

src/coreclr/jit/fgehopt.cpp

@@ -2255,6 +2255,14 @@ void Compiler::fgTailMergeThrowsFallThroughHelper(BasicBlock* predBlock,
 
     // Note there is now a jump to the canonical block
     canonicalBlock->bbFlags |= (BBF_JMP_TARGET | BBF_HAS_LABEL);
+
+    // If nonCanonicalBlock has only one pred, all its flow transfers.
+    // If it has multiple preds, then we need edge counts or likelihoods
+    // to figure things out.
+    //
+    // For now just do a minimal update.
+    //
+    newBlock->inheritWeight(nonCanonicalBlock);
 }
 
 //------------------------------------------------------------------------

src/coreclr/jit/fginline.cpp

@@ -1365,15 +1365,16 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
             continue;
         }
 
-        if (inheritWeight)
-        {
-            block->inheritWeight(iciBlock);
-            inheritWeight = false;
-        }
-        else
+        // If we were unable to compute a scale for some reason, then
+        // try to do something plausible. Entry/exit blocks match call
+        // site, internal blocks scaled by half; all rare blocks left alone.
+        //
+        if (!block->isRunRarely())
         {
-            block->modifyBBWeight(iciBlock->bbWeight / 2);
+            block->inheritWeightPercentage(iciBlock, inheritWeight ? 100 : 50);
         }
+
+        inheritWeight = false;
     }
 
     // Insert inlinee's blocks into inliner's block list.
@@ -1426,7 +1427,30 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
     // Update unmanaged call details
     info.compUnmanagedCallCountWithGCTransition += InlineeCompiler->info.compUnmanagedCallCountWithGCTransition;
 
-// Update optMethodFlags
+    // Update stats for inlinee PGO
+    //
+    if (InlineeCompiler->fgPgoSchema != nullptr)
+    {
+        fgPgoInlineePgo++;
+    }
+    else if (InlineeCompiler->fgPgoFailReason != nullptr)
+    {
+        // Single block inlinees may not have probes
+        // when we've ensabled minimal profiling (which
+        // is now the default).
+        //
+        if (InlineeCompiler->fgBBcount == 1)
+        {
+            fgPgoInlineeNoPgoSingleBlock++;
+        }
+        else
+        {
+            fgPgoInlineeNoPgo++;
+        }
+    }
+
+    // Update optMethodFlags
+    CLANG_FORMAT_COMMENT_ANCHOR;
 
 #ifdef DEBUG
     unsigned optMethodFlagsBefore = optMethodFlags;