From 3b243a24c65279dfcd21ae55e853b1fb458ff49b Mon Sep 17 00:00:00 2001
From: Andy Ayers <andya@microsoft.com>
Date: Thu, 2 May 2024 17:32:11 -0700
Subject: [PATCH] JIT: synthesize PGO if no schema, when dynamic PGO is active
 (#101739)

If we know dynamic PGO is active, and we do not find a PGO schema for a method,
synthesize PGO data. The schema may be missing if the method was prejitted but
not covered by static PGO, or was considered too simple to need profiling (aka
minimal profiling).

This synthesis removes the possibility of a mixed PGO/no PGO situation. These
are problematic, especially in methods that do a lot of inlining. Now when
dynamic PGO is active all methods that get optimized will have some form of
PGO data.

Only run profile incorporation when optimizing. Reset BBOPT/pgo vars if we
switch away from optimization or have a min opts failover.

Contributes to #93020.
---
 src/coreclr/jit/compiler.cpp  | 12 ++++++++++
 src/coreclr/jit/fgprofile.cpp | 44 +++++++++++++++++++++--------------
 src/coreclr/jit/optimizer.cpp |  2 +-
 3 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp
index f15820a7dc5746..8e82707adbbbfd 100644
--- a/src/coreclr/jit/compiler.cpp
+++ b/src/coreclr/jit/compiler.cpp
@@ -4088,7 +4088,18 @@ void Compiler::compSetOptimizationLevel()
     {
         info.compCompHnd->setMethodAttribs(info.compMethodHnd, CORINFO_FLG_SWITCHED_TO_MIN_OPT);
         opts.jitFlags->Clear(JitFlags::JIT_FLAG_TIER1);
+        opts.jitFlags->Clear(JitFlags::JIT_FLAG_BBOPT);
         compSwitchedToMinOpts = true;
+
+        // We may have read PGO data. Clear it out because we won't be using it.
+        //
+        fgPgoFailReason  = "method switched to min-opts";
+        fgPgoQueryResult = E_FAIL;
+        fgPgoHaveWeights = false;
+        fgPgoData        = nullptr;
+        fgPgoSchema      = nullptr;
+        fgPgoDisabled    = true;
+        fgPgoDynamic     = false;
     }
 
 #ifdef DEBUG
@@ -8056,6 +8067,7 @@ if (!inlineInfo &&
     compileFlags->Set(JitFlags::JIT_FLAG_MIN_OPT);
     compileFlags->Clear(JitFlags::JIT_FLAG_SIZE_OPT);
     compileFlags->Clear(JitFlags::JIT_FLAG_SPEED_OPT);
+    compileFlags->Clear(JitFlags::JIT_FLAG_BBOPT);
 
     goto START;
 }
diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp
index 10fdd6035e0b3f..6d7ef484fe9a87 100644
--- a/src/coreclr/jit/fgprofile.cpp
+++ b/src/coreclr/jit/fgprofile.cpp
@@ -18,26 +18,15 @@
 //   true if so
 //
 // Note:
-//   This now returns true for inlinees. We might consider preserving the
-//   old behavior for crossgen, since crossgen BBINSTRs still do inlining
-//   and don't instrument the inlinees.
+//   In most cases it is more appropriate to call fgHaveProfileWeights,
+//   since that tells you if blocks have profile-based weights.
 //
-//   Thus if BBINSTR and BBOPT do the same inlines (which can happen)
-//   profile data for an inlinee (if available) will not fully reflect
-//   the behavior of the inlinee when called from this method.
+//   This method literally checks if the runtime had a profile schema,
+//   from which we can derive weights.
 //
-//   If this inlinee was not inlined by the BBINSTR run then the
-//   profile data for the inlinee will reflect this method's influence.
-//
-//   * for ALWAYS_INLINE and FORCE_INLINE cases it is unlikely we'll find
-//     any profile data, as BBINSTR and BBOPT callers will both inline;
-//     only indirect callers will invoke the instrumented version to run.
-//   * for DISCRETIONARY_INLINE cases we may or may not find relevant
-//     data, depending, but chances are the data is relevant.
-//
-//  TieredPGO data comes from Tier0 methods, which currently do not do
-//  any inlining; thus inlinee profile data should be available and
-//  representative.
+//   Schema-based data comes from Tier0 methods, which currently do not do
+//   any inlining; thus inlinee profile data should be available and
+//   representative.
 //
 bool Compiler::fgHaveProfileData()
 {
@@ -47,6 +36,9 @@ bool Compiler::fgHaveProfileData()
 //------------------------------------------------------------------------
 // fgHaveProfileWeights: Check if we have a profile that has weights.
 //
+// Notes:
+//    These weights may come from instrumentation or from synthesis.
+//
 bool Compiler::fgHaveProfileWeights()
 {
     return fgPgoHaveWeights;
@@ -2848,6 +2840,14 @@ PhaseStatus Compiler::fgIncorporateProfileData()
         return PhaseStatus::MODIFIED_EVERYTHING;
     }
 
+    // For now we only rely on profile data when optimizing.
+    //
+    if (!opts.OptimizationEnabled())
+    {
+        JITDUMP("not optimizing, so not incorporating any profile data\n");
+        return PhaseStatus::MODIFIED_NOTHING;
+    }
+
 #ifdef DEBUG
     // Optionally run synthesis
     //
@@ -2887,6 +2887,14 @@ PhaseStatus Compiler::fgIncorporateProfileData()
             JITDUMP("BBOPT not set\n");
         }
 
+        // Is dynamic PGO active? If so, run synthesis.
+        //
+        if (fgPgoDynamic)
+        {
+            JITDUMP("Dynamic PGO active, synthesizing profile data\n");
+            ProfileSynthesis::Run(this, ProfileSynthesisOption::AssignLikelihoods);
+        }
+
         // Scale the "synthetic" block weights.
         //
         fgApplyProfileScale();
diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp
index 7f2ef4bf95d6e6..fae61fbf9b1c48 100644
--- a/src/coreclr/jit/optimizer.cpp
+++ b/src/coreclr/jit/optimizer.cpp
@@ -234,7 +234,7 @@ void Compiler::optScaleLoopBlocks(BasicBlock* begBlk, BasicBlock* endBlk)
     for (BasicBlock* const curBlk : BasicBlockRangeList(begBlk, endBlk))
     {
         // Don't change the block weight if it came from profile data.
-        if (curBlk->hasProfileWeight() && fgHaveProfileData())
+        if (curBlk->hasProfileWeight() && fgHaveProfileWeights())
         {
             reportBlockWeight(curBlk, "; unchanged: has profile weight");
             continue;