JIT: profile synthesis consistency checking and more

Add the ability to verify that the profile counts produced by synthesis are
self-consistent, and optionally to assert if they're not. Disable checking
if we know profile flow will be inconsistent (because of irreducible loops
and/or capped cyclic probabilities).

Consistently ignore the likely flow out of handlers. Generally we model
handlers as isolated subgraphs that do not contribute flow to the main flow
graph. This is generally acceptable.

The one caveat is for flow into finallies. The plan here is to fix the weights
for finallies up in a subsequent pass via simple scaling once callfinallies
are introduced. Flow weights out of finallies will be ignored as the
callfinally block will be modeled as always flowing to its pair tail.

Also add the ability to propagate the synthesized counts into the live profile
data. This is mainly to facilitate using the MIBC comparison tools we have
to assess how closely the synthesiszed data comes to actual PGO data.

Finally, enable the new synthesized plus checked modes in a few of our PGO
pipelines.

Contributes to dotnet#82964.

Author: AndyAyersMS

eng/pipelines/common/templates/runtimes/run-test-job.yml

@@ -541,6 +541,7 @@ jobs:
             - fullpgo_random_gdv_methodprofiling_only
             - fullpgo_random_gdv_edge
             - fullpgo_methodprofiling_always_optimized
+            - syntheticpgo
         ${{ if in(parameters.testGroup, 'gc-longrunning') }}:
           longRunningGcTests: true
           scenarios:

eng/pipelines/libraries/run-test-job.yml

@@ -200,3 +200,4 @@ jobs:
                 - fullpgo_random_gdv_edge
                 - jitosr_stress
                 - jitosr_stress_random
+                - syntheticpgo

src/coreclr/jit/compiler.h

@@ -1577,6 +1577,53 @@ inline constexpr FlowGraphUpdates operator&(FlowGraphUpdates a, FlowGraphUpdates
     return (FlowGraphUpdates)((unsigned int)a & (unsigned int)b);
 }
 
+// Profile checking options
+//
+// clang-format off
+enum class ProfileChecks : unsigned int
+{
+    CHECK_NONE          = 0,
+    CHECK_CLASSIC       = 1 << 0, // check "classic" jit weights
+    CHECK_LIKELY        = 1 << 1, // check likelihood based weights
+    RAISE_ASSERT        = 1 << 2, // assert on check failure
+    CHECK_ALL_BLOCKS    = 1 << 3, // check blocks even if bbHasProfileWeight is false
+};
+
+inline constexpr ProfileChecks operator ~(ProfileChecks a)
+{
+    return (ProfileChecks)(~(unsigned int)a);
+}
+
+inline constexpr ProfileChecks operator |(ProfileChecks a, ProfileChecks b)
+{
+    return (ProfileChecks)((unsigned int)a | (unsigned int)b);
+}
+
+inline constexpr ProfileChecks operator &(ProfileChecks a, ProfileChecks b)
+{
+    return (ProfileChecks)((unsigned int)a & (unsigned int)b);
+}
+
+inline ProfileChecks& operator |=(ProfileChecks& a, ProfileChecks b)
+{
+    return a = (ProfileChecks)((unsigned int)a | (unsigned int)b);
+}
+
+inline ProfileChecks& operator &=(ProfileChecks& a, ProfileChecks b)
+{
+    return a = (ProfileChecks)((unsigned int)a & (unsigned int)b);
+}
+
+inline ProfileChecks& operator ^=(ProfileChecks& a, ProfileChecks b)
+{
+    return a = (ProfileChecks)((unsigned int)a ^ (unsigned int)b);
+}
+
+inline bool hasFlag(const ProfileChecks& flagSet, const ProfileChecks& flag)
+{
+    return ((flagSet & flag) == flag);
+}
+
 //---------------------------------------------------------------
 // Compilation time.
 //
@@ -5516,8 +5563,9 @@ class Compiler
     void fgDebugCheckFlagsHelper(GenTree* tree, GenTreeFlags actualFlags, GenTreeFlags expectedFlags);
     void fgDebugCheckTryFinallyExits();
     void fgDebugCheckProfileWeights();
-    bool fgDebugCheckIncomingProfileData(BasicBlock* block);
-    bool fgDebugCheckOutgoingProfileData(BasicBlock* block);
+    void fgDebugCheckProfileWeights(ProfileChecks checks);
+    bool fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks checks);
+    bool fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks checks);
 
 #endif // DEBUG
 

src/coreclr/jit/fgdiagnostic.cpp

@@ -957,7 +957,7 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos)
             }
 
             // "Raw" Profile weight
-            if (block->hasProfileWeight())
+            if (block->hasProfileWeight() || (JitConfig.JitSynthesizeCounts() > 0))
             {
                 fprintf(fgxFile, "\\n\\n%7.2f", ((double)block->getBBWeight(this)) / BB_UNITY_WEIGHT);
             }

src/coreclr/jit/fgprofile.cpp

@@ -596,6 +596,26 @@ void BlockCountInstrumentor::Instrument(BasicBlock* block, Schema& schema, uint8
            (entry.InstrumentationKind == ICorJitInfo::PgoInstrumentationKind::BasicBlockLongCount));
     uint8_t* addrOfCurrentExecutionCount = entry.Offset + profileMemory;
 
+#ifdef DEBUG
+    if (JitConfig.JitPropagateSynthesizedCountsToProfileData() > 0)
+    {
+        // Write the current synthesized count as the profile data
+        //
+        weight_t blockWeight = block->bbWeight;
+
+        if (entry.InstrumentationKind == ICorJitInfo::PgoInstrumentationKind::EdgeIntCount)
+        {
+            *((uint32_t*)addrOfCurrentExecutionCount) = (uint32_t)blockWeight;
+        }
+        else
+        {
+            *((uint64_t*)addrOfCurrentExecutionCount) = (uint64_t)blockWeight;
+        }
+
+        return;
+    }
+#endif
+
     var_types typ =
         entry.InstrumentationKind == ICorJitInfo::PgoInstrumentationKind::BasicBlockIntCount ? TYP_INT : TYP_LONG;
 
@@ -1742,6 +1762,32 @@ void EfficientEdgeCountInstrumentor::Instrument(BasicBlock* block, Schema& schem
 
         uint8_t* addrOfCurrentExecutionCount = profileMemory + entry.Offset;
 
+#ifdef DEBUG
+        if (JitConfig.JitPropagateSynthesizedCountsToProfileData() > 0)
+        {
+            // Write the current synthesized count as the profile data
+            //
+            // Todo: handle pseudo edges!
+            FlowEdge* const edge = m_comp->fgGetPredForBlock(source, target);
+
+            if (edge != nullptr)
+            {
+                weight_t edgeWeight = edge->getLikelyWeight();
+
+                if (entry.InstrumentationKind == ICorJitInfo::PgoInstrumentationKind::EdgeIntCount)
+                {
+                    *((uint32_t*)addrOfCurrentExecutionCount) = (uint32_t)edgeWeight;
+                }
+                else
+                {
+                    *((uint64_t*)addrOfCurrentExecutionCount) = (uint64_t)edgeWeight;
+                }
+            }
+
+            return;
+        }
+#endif
+
         // Determine where to place the probe.
         //
         BasicBlock* instrumentedBlock = nullptr;
@@ -2428,7 +2474,7 @@ PhaseStatus Compiler::fgIncorporateProfileData()
     }
 
 #ifdef DEBUG
-    // Optionally just run synthesis
+    // Optionally run synthesis
     //
     if ((JitConfig.JitSynthesizeCounts() > 0) && !compIsForInlining())
     {
@@ -2439,6 +2485,17 @@ PhaseStatus Compiler::fgIncorporateProfileData()
             return PhaseStatus::MODIFIED_EVERYTHING;
         }
     }
+
+    // Or run synthesis and save the data out as the actual profile data
+    //
+    if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR) &&
+        (JitConfig.JitPropagateSynthesizedCountsToProfileData() > 0) && !compIsForInlining())
+    {
+        JITDUMP("Synthesizing profile data and writing it out as the actual profile data\n");
+        ProfileSynthesis::Run(this, ProfileSynthesisOption::AssignLikelihoods);
+        fgPgoHaveWeights = false;
+        return PhaseStatus::MODIFIED_EVERYTHING;
+    }
 #endif
 
     // Do we have profile data?
@@ -4845,15 +4902,8 @@ bool Compiler::fgProfileWeightsConsistent(weight_t weight1, weight_t weight2)
 //   (or nearly so)
 //
 // Notes:
-//   For each profiled block, check that the flow of counts into
-//   the block matches the flow of counts out of the block.
-//
-//   We ignore EH flow as we don't have explicit edges and generally
-//   we expect EH edge counts to be small, so errors from ignoring
-//   them should be rare.
-//
-//   There's no point checking until we've built pred lists, as
-//   we can't easily reason about consistency without them.
+//   Does nothing, if profile checks are disabled, or there are
+//   no profile weights or pred lists.
 //
 void Compiler::fgDebugCheckProfileWeights()
 {
@@ -4865,13 +4915,37 @@ void Compiler::fgDebugCheckProfileWeights()
         return;
     }
 
+    fgDebugCheckProfileWeights((ProfileChecks)JitConfig.JitProfileChecks());
+}
+
+//------------------------------------------------------------------------
+// fgDebugCheckProfileWeights: verify profile weights are self-consistent
+//   (or nearly so)
+//
+// Arguments:
+//   checks - checker options
+//
+// Notes:
+//   For each profiled block, check that the flow of counts into
+//   the block matches the flow of counts out of the block.
+//
+//   We ignore EH flow as we don't have explicit edges and generally
+//   we expect EH edge counts to be small, so errors from ignoring
+//   them should be rare.
+//
+//   There's no point checking until we've built pred lists, as
+//   we can't easily reason about consistency without them.
+//
+void Compiler::fgDebugCheckProfileWeights(ProfileChecks checks)
+{
     // We can check classic (min/max, late computed) weights
     //   and/or
     // new likelyhood based weights.
     //
-    const bool verifyClassicWeights = fgEdgeWeightsComputed && (JitConfig.JitProfileChecks() & 0x1) == 0x1;
-    const bool verifyLikelyWeights  = (JitConfig.JitProfileChecks() & 0x2) == 0x2;
-    const bool assertOnFailure      = (JitConfig.JitProfileChecks() & 0x4) == 0x4;
+    const bool verifyClassicWeights = fgEdgeWeightsComputed && hasFlag(checks, ProfileChecks::CHECK_CLASSIC);
+    const bool verifyLikelyWeights  = hasFlag(checks, ProfileChecks::CHECK_LIKELY);
+    const bool assertOnFailure      = hasFlag(checks, ProfileChecks::RAISE_ASSERT);
+    const bool checkAllBlocks       = hasFlag(checks, ProfileChecks::CHECK_ALL_BLOCKS);
 
     if (!(verifyClassicWeights || verifyLikelyWeights))
     {
@@ -4891,7 +4965,7 @@ void Compiler::fgDebugCheckProfileWeights()
     //
     for (BasicBlock* const block : Blocks())
     {
-        if (!block->hasProfileWeight())
+        if (!block->hasProfileWeight() && !checkAllBlocks)
         {
             unprofiledBlocks++;
             continue;
@@ -4929,8 +5003,11 @@ void Compiler::fgDebugCheckProfileWeights()
         //
         if (block->KindIs(BBJ_RETURN, BBJ_THROW))
         {
-            exitWeight += blockWeight;
-            exitProfiled   = !opts.IsOSR();
+            if (BasicBlock::sameHndRegion(block, fgFirstBB))
+            {
+                exitWeight += blockWeight;
+                exitProfiled = !opts.IsOSR();
+            }
             verifyOutgoing = false;
         }
 
@@ -4969,12 +5046,12 @@ void Compiler::fgDebugCheckProfileWeights()
 
         if (verifyIncoming)
         {
-            incomingConsistent = fgDebugCheckIncomingProfileData(block);
+            incomingConsistent = fgDebugCheckIncomingProfileData(block, checks);
         }
 
         if (verifyOutgoing)
         {
-            outgoingConsistent = fgDebugCheckOutgoingProfileData(block);
+            outgoingConsistent = fgDebugCheckOutgoingProfileData(block, checks);
         }
 
         if (!incomingConsistent || !outgoingConsistent)
@@ -4987,7 +5064,13 @@ void Compiler::fgDebugCheckProfileWeights()
     //
     if (entryProfiled && exitProfiled)
     {
-        if (!fgProfileWeightsConsistent(entryWeight, exitWeight))
+        // Note these may not agree, if fgEntryBB is a loop header.
+        //
+        if (fgFirstBB->bbRefs > 1)
+        {
+            JITDUMP("  Method entry " FMT_BB " is loop head, can't check entry/exit balance\n");
+        }
+        else if (!fgProfileWeightsConsistent(entryWeight, exitWeight))
         {
             problemBlocks++;
             JITDUMP("  Method entry " FMT_WT " method exit " FMT_WT " weight mismatch\n", entryWeight, exitWeight);
@@ -5025,18 +5108,19 @@ void Compiler::fgDebugCheckProfileWeights()
 //   block matches the profile weight of the block.
 //
 // Arguments:
-//   block - block to check
+//   block  - block to check
+//   checks - checker options
 //
 // Returns:
 //   true if counts consistent or checking disabled, false otherwise.
 //
 // Notes:
 //   Only useful to call on blocks with predecessors.
 //
-bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block)
+bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks checks)
 {
-    const bool verifyClassicWeights = fgEdgeWeightsComputed && (JitConfig.JitProfileChecks() & 0x1) == 0x1;
-    const bool verifyLikelyWeights  = (JitConfig.JitProfileChecks() & 0x2) == 0x2;
+    const bool verifyClassicWeights = fgEdgeWeightsComputed && hasFlag(checks, ProfileChecks::CHECK_CLASSIC);
+    const bool verifyLikelyWeights  = hasFlag(checks, ProfileChecks::CHECK_LIKELY);
 
     if (!(verifyClassicWeights || verifyLikelyWeights))
     {
@@ -5056,7 +5140,10 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block)
         incomingWeightMax += predEdge->edgeWeightMax();
         if (predEdge->hasLikelihood())
         {
-            incomingLikelyWeight += predEdge->getLikelyWeight();
+            if (BasicBlock::sameHndRegion(block, predEdge->getSourceBlock()))
+            {
+                incomingLikelyWeight += predEdge->getLikelyWeight();
+            }
         }
         else
         {
@@ -5122,17 +5209,18 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block)
 //
 // Arguments:
 //   block - block to check
+//   checks - checker options
 //
 // Returns:
 //   true if counts consistent or checking disabled, false otherwise.
 //
 // Notes:
 //   Only useful to call on blocks with successors.
 //
-bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block)
+bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks checks)
 {
-    const bool verifyClassicWeights = fgEdgeWeightsComputed && (JitConfig.JitProfileChecks() & 0x1) == 0x1;
-    const bool verifyLikelyWeights  = (JitConfig.JitProfileChecks() & 0x2) == 0x2;
+    const bool verifyClassicWeights = fgEdgeWeightsComputed && hasFlag(checks, ProfileChecks::CHECK_CLASSIC);
+    const bool verifyLikelyWeights  = hasFlag(checks, ProfileChecks::CHECK_LIKELY);
 
     if (!(verifyClassicWeights || verifyLikelyWeights))
     {