Simplify PR, get rid of strategies

docs/design/features/DynamicPgo-InstrumentedTiers.md

@@ -23,57 +23,43 @@ Yellow line provides the highest level of performance (RPS) by sacrificing start
 
 # Tiered compilation workflow in TieredPGO mode
 
-The following diagram explains how the instrumentation for hot R2R code works under the hood when TieredPGO is enabled (it's disabled by default):
+The following diagram explains how the instrumentation for hot code works under the hood when TieredPGO is enabled (it's disabled by default):
 
 ```mermaid
 flowchart
     prestub(.NET Function) -->|Compilation| hasAO{"Marked with<br/>[AggressiveOpts]?"}
     hasAO-->|Yes|tier1ao["JIT to <b><ins>Tier1</ins></b><br/><br/>(that attribute is extremely<br/> rarely a good idea)"]
     hasAO-->|No|hasR2R
-    hasR2R{"Is prejitted (R2R)<br/>and ReadyToRun==1"?} -->|No| istrTier0Q
+    hasR2R{"Is prejitted (R2R)<br/>and ReadyToRun==1"?} -->|No| tier000
 
-    istrTier0Q{"<b>TieredPGO_Strategy:</b><br/>Instrument only<br/>hot Tier0 code?"}
-    istrTier0Q-->|No, always instrument tier0|tier0
-    istrTier0Q-->|Yes, only hot|tier000
     tier000["JIT to <b><ins>Tier0</ins></b><br/><br/>(not optimized, not instrumented,<br/> with patchpoints)"]-->|Running...|ishot555
     ishot555{"Is hot?<br/>(called >30 times)"}
     ishot555-.->|No,<br/>keep running...|ishot555
     ishot555-->|Yes|tier0
 
     hasR2R -->|Yes| R2R
-    R2R["Use <b><ins>R2R</ins></b> code<br/><br/>(optimized, not instrumented,<br/>with patchpoints)"] -->|Running...|ishot1
+    R2R["Use <b><ins>R2R</ins></b> code<br/><br/>(optimized, not instrumented,<br/>no patchpoints)"] -->|Running...|ishot1
     ishot1{"Is hot?<br/>(called >30 times)"}-.->|No,<br/>keep running...|ishot1
-    ishot1--->|"Yes"|instrumentR2R
-
-    instrumentR2R{"<b>TieredPGO_Strategy:</b><br/>Instrument hot<br/>R2R'd code?"}
-    instrumentR2R-->|Yes, instrument R2R'd code|istier1inst
-    instrumentR2R-->|No, don't instrument R2R'd code|tier1nopgo["JIT to <b><ins>Tier1</ins></b><br/><br/>(no dynamic profile data)"]
+    ishot1--->|"Yes"|tier1inst
 
     tier0["JIT to <b><ins>InstrumentedTier</ins></b><br/><br/>(not optimized, instrumented,<br/> with patchpoints)"]-->|Running...|ishot5
     tier1pgo2["JIT to <b><ins>Tier1</ins></b><br/><br/>(optimized with profile data)"]
-    tier1pgo2_1["JIT to <b><ins>Tier1</ins></b><br/><br/>(optimized with profile data)"]
 
-    istier1inst{"<b>TieredPGO_Strategy:</b><br/>Enable optimizations<br/>for InstrumentedTier?"}-->|"No"|tier0_1
-    istier1inst--->|"Yes"|tier1inst["JIT to <b><ins>InstrumentedTierOptimized</ins></b><br/><br/>(optimized, instrumented, <br/>with patchpoints)"]
-    tier1inst-->|Running...|ishot5_1
+    tier1inst["JIT to <b><ins>InstrumentedTierOptimized</ins></b><br/><br/>(optimized, instrumented, <br/>no patchpoints)"]
+    tier1inst-->|Running...|ishot5
     ishot5{"Is hot?<br/>(called >30 times)"}-->|Yes|tier1pgo2
     ishot5-.->|No,<br/>keep running...|ishot5
-
-
-    ishot5_1{"Is hot?<br/>(called >30 times)"}
-    ishot5_1-.->|No,<br/>keep running...|ishot5_1
-    ishot5_1{"Is hot?<br/>(called >30 times)"}-->|Yes|tier1pgo2_1
-
-    tier0_1["JIT to <b><ins>InstrumentedTier</ins></b><br/><br/>(not optimized, instrumented,<br/> with patchpoints)"]
-    tier0_1-->|Running...|ishot5_1
 ```
-(_VSCode doesn't support mermaid diagrams, consider installing external add-ins_)
+(_VSCode doesn't support mermaid diagrams out of the box, consider installing external add-ins_)
 
 ## Pros & cons of using optimizations inside the instrumented tiers
 
+Pros & cons of using optimizations inside the instrumented tiers
 Pros:
-* Lower overhead from instrumentation (and thanks to optimizations we _can_ optimize probes and emit less of those)
-* Optimized code is able to inline methods so we won't be producing new Compilation units for even small methods
+   * Lower overhead from instrumentation
+   * We definitely don't want to use unoptimized instrumented tier for R2R - it will produce a lot of new first-time compilations for non-inlined methods
+     and performance impact might be noticeable (when we switch from fast R2R to extremely slow Tier0+instrumentation)
 
 Cons:
-* Currently, we won't instrument inlinees -> we'll probably miss a lot of opportunities and produce less accurate profile leading to a less optimized final tier
+   * Currently, we don't instrument inlinees -> we'll probably miss some oportunities and produce less accurate profile leading to a less optimized final tier
+   * Non-optimized instrumented tier is faster to prepare for use

src/coreclr/inc/clrconfigvalues.h

@@ -607,25 +607,11 @@ RETAIL_CONFIG_DWORD_INFO(INTERNAL_ReadPGOData, W("ReadPGOData"), 0, "Read PGO da
 RETAIL_CONFIG_DWORD_INFO(INTERNAL_WritePGOData, W("WritePGOData"), 0, "Write PGO data")
 RETAIL_CONFIG_DWORD_INFO(EXTERNAL_TieredPGO, W("TieredPGO"), 0, "Instrument Tier0 code and make counts available to Tier1")
 
-// TieredPGO_Strategy values:
+// TieredPGO_InstrumentOnlyHotCode values:
 //
-// 0) Instrument any non-prejitted code
-// 1) Instrument any non-prejitted code and only hot R2R code
-// 2) Instrument any non-prejitted code and only hot R2R code (use optimizations in the instrumented tier for hot R2R)
-// 3) Instrument only hot non-prejitted code and only hot R2R code
-// 4) Instrument only hot non-prejitted code and only hot R2R code (use optimizations in the instrumented tier for hot R2R)
-//
-//
-// Pros & cons of using optimizations inside the instrumented tiers (mode '2' and '4')
-// Pros:
-//   * Lower overhead from instrumentation (and thanks to optimizations we _can_ optimize probes and emit less of those)
-//   * Optimized code is able to inline methods so we won't be producing new Compilation units for even small methods
-//
-// Cons:
-//   * Currently, we won't instrument inlinees -> we'll probably miss a lot of oportunities and produce less accurate profile
-//     leading to a less optimized final tier
-//
-RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_TieredPGO_Strategy, W("TieredPGO_Strategy"), 0, "Strategy for TieredPGO, see comments in clrconfigvalues.h")
+// 0) Instrument all IL-only code, R2R'd code is never instrumented
+// 1) Instrument only hot IL-only and hot R2R code (use optimizations in the instrumented tier for hot R2R and no optimizations for hot IL-only)
+RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_TieredPGO_InstrumentOnlyHotCode, W("TieredPGO_InstrumentOnlyHotCode"), 0, "Strategy for TieredPGO, see comments in clrconfigvalues.h")
 #endif
 
 ///

src/coreclr/jit/importercalls.cpp

@@ -218,6 +218,15 @@ var_types Compiler::impImportCall(OPCODE                  opcode,
         {
             return impImportJitTestLabelMark(sig->numArgs);
         }
+
+        // static ulong JitHelpers_JitFlags() => 0;
+        // can be defined anywhere and will be replaced by Debug-version of RyuJIT
+        if ((mflags & CORINFO_FLG_STATIC) && (sig->numArgs == 0) && (sig->retType == CorInfoType::CORINFO_TYPE_ULONG) &&
+            (strcmp("JitHelpers_JitFlags", eeGetMethodName(methHnd, nullptr)) == 0))
+        {
+            call = gtNewLconNode((__int64)opts.jitFlags->GetRawFlags());
+            goto DONE_CALL;
+        }
 #endif // DEBUG
 
         const bool isIntrinsic = (mflags & CORINFO_FLG_INTRINSIC) != 0;
@@ -6290,7 +6299,7 @@ bool Compiler::impConsiderCallProbe(GenTreeCall* call, IL_OFFSET ilOffset)
         return false;
     }
 
-    assert(opts.OptimizationDisabled() || opts.IsOSR());
+    assert(opts.OptimizationDisabled() || opts.IsInstrumentedOptimized());
     assert(!compIsForInlining());
 
     // During importation, optionally flag this block as one that

src/coreclr/vm/eeconfig.cpp

@@ -239,7 +239,7 @@ HRESULT EEConfig::Init()
 
 #if defined(FEATURE_PGO)
     fTieredPGO = false;
-    tieredPGO_Strategy = (TieredPGOStrategy)0;
+    tieredPGO_InstrumentOnlyHotCode = false;
 #endif
 
 #if defined(FEATURE_READYTORUN)
@@ -783,26 +783,13 @@ HRESULT EEConfig::sync()
 
 #if defined(FEATURE_PGO)
     fTieredPGO = Configuration::GetKnobBooleanValue(W("System.Runtime.TieredPGO"), CLRConfig::EXTERNAL_TieredPGO);
-    tieredPGO_Strategy = (TieredPGOStrategy)CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_TieredPGO_Strategy);
+    tieredPGO_InstrumentOnlyHotCode = CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_TieredPGO_InstrumentOnlyHotCode) == 1;
 
     // We need quick jit for TieredPGO
     if (!fTieredCompilation_QuickJit)
     {
         fTieredPGO = false;
     }
-    else
-    {
-        if (tieredPGO_Strategy == InstrumentColdNonPrejittedCode_InstrumentHotPrejittedCode ||
-            tieredPGO_Strategy == InstrumentHotNonPrejittedCode_InstrumentHotPrejittedCode)
-        {
-            // When we're not using optimizations in the instrumented tiers we produce a lot of new first-time compilation 
-            // due to disabled inlining even for very small methods - such first-time compilations delay promotions by
-            // tieredCompilation_CallCountingDelayMs
-            tieredCompilation_CallCountingDelayMs /= 3;
-            tieredCompilation_CallCountingDelayMs = max(1, tieredCompilation_CallCountingDelayMs);
-        }
-        _ASSERTE(tieredPGO_Strategy >= 0 && tieredPGO_Strategy <= 4);
-    }
 #endif
 
 #if defined(FEATURE_ON_STACK_REPLACEMENT)

src/coreclr/vm/eeconfig.h

@@ -51,25 +51,6 @@ enum ParseCtl {
     stopAfterRuntimeSection // stop after <runtime>...</runtime> section
 };
 
-// Keep in sync with comments in clrconfigvalues.h around "TieredPGO_Strategy"
-enum TieredPGOStrategy
-{
-    // Instrument any non-prejitted code
-    InstrumentColdNonPrejittedCode = 0,
-
-    // Instrument any non-prejitted code and only hot R2R code
-    InstrumentColdNonPrejittedCode_InstrumentHotPrejittedCode = 1,
-
-    // Instrument any non-prejitted code and only hot R2R code (use optimizations in the instrumented tier for hot R2R)
-    InstrumentColdNonPrejittedCode_InstrumentHotPrejittedCode_Optimized = 2,
-
-    // Instrument only hot non-prejitted code and only hot R2R code
-    InstrumentHotNonPrejittedCode_InstrumentHotPrejittedCode = 3,
-
-    // Instrument only hot non-prejitted code and only hot R2R code (use optimizations in the instrumented tier for hot R2R)
-    InstrumentHotNonPrejittedCode_InstrumentHotPrejittedCode_Optimized = 4
-};
-
 class EEConfig
 {
 public:
@@ -110,8 +91,8 @@ class EEConfig
 #endif
 
 #if defined(FEATURE_PGO)
-    bool              TieredPGO(void) const { LIMITED_METHOD_CONTRACT;  return fTieredPGO; }
-    TieredPGOStrategy TieredPGO_Strategy(void) const { LIMITED_METHOD_CONTRACT;  return tieredPGO_Strategy; }
+    bool          TieredPGO(void) const { LIMITED_METHOD_CONTRACT;  return fTieredPGO; }
+    bool          TieredPGO_InstrumentOnlyHotCode(void) const { LIMITED_METHOD_CONTRACT;  return tieredPGO_InstrumentOnlyHotCode; }
 #endif
 
 #if defined(FEATURE_READYTORUN)
@@ -678,7 +659,7 @@ class EEConfig
 
 #if defined(FEATURE_PGO)
     bool fTieredPGO;
-    TieredPGOStrategy tieredPGO_Strategy;
+    bool tieredPGO_InstrumentOnlyHotCode;
 #endif
 
 #if defined(FEATURE_READYTORUN)

src/coreclr/vm/tieredcompilation.cpp

@@ -114,29 +114,16 @@ NativeCodeVersion::OptimizationTier TieredCompilationManager::GetInitialOptimiza
 #ifdef FEATURE_PGO
     if (g_pConfig->TieredPGO())
     {
-        switch (g_pConfig->TieredPGO_Strategy())
+        // Initial tier for R2R is always just OptimizationTier0
+        // For ILOnly it depends on TieredPGO_InstrumentOnlyHotCode:
+        // 1 - OptimizationTier0 as we don't want to instrument the initial version (will only instrument hot Tier0)
+        // 2 - OptimizationTierInstrumented - instrument all ILOnly code
+        if (g_pConfig->TieredPGO_InstrumentOnlyHotCode() || 
+            ExecutionManager::IsReadyToRunCode(pMethodDesc->GetNativeCode()))
         {
-            // Use OptimizationTierInstrumented for non-prejitted code as the initial tier
-            // and OptimizationTier0 for R2R'd
-            case InstrumentColdNonPrejittedCode:
-            case InstrumentColdNonPrejittedCode_InstrumentHotPrejittedCode:
-            case InstrumentColdNonPrejittedCode_InstrumentHotPrejittedCode_Optimized:
-            {
-                if (ExecutionManager::IsReadyToRunCode(pMethodDesc->GetNativeCode()))
-                {
-                    return NativeCodeVersion::OptimizationTier0;
-                }
-                return NativeCodeVersion::OptimizationTierInstrumented;
-            }
-
-            // These never start with OptimizationTierInstrumented
-            case InstrumentHotNonPrejittedCode_InstrumentHotPrejittedCode:
-            case InstrumentHotNonPrejittedCode_InstrumentHotPrejittedCode_Optimized:
-                return NativeCodeVersion::OptimizationTier0;
-
-            default:
-                UNREACHABLE_MSG("Unknown TieredPGO_Strategy");
+            return NativeCodeVersion::OptimizationTier0;
         }
+        return NativeCodeVersion::OptimizationTierInstrumented;
     }
 #endif
 
@@ -295,54 +282,26 @@ void TieredCompilationManager::AsyncPromoteToTier1(
     NativeCodeVersion::OptimizationTier nextTier = NativeCodeVersion::OptimizationTier1;
 
 #ifdef FEATURE_PGO
-    // If TieredPGO is enabled, follow TieredPGO_Strategy, see comments in clrconfigvalues.h around it
     if (g_pConfig->TieredPGO())
     {
         if (currentNativeCodeVersion.IsDefaultVersion() &&
-            currentNativeCodeVersion.GetOptimizationTier() == NativeCodeVersion::OptimizationTier0)
+            currentNativeCodeVersion.GetOptimizationTier() == NativeCodeVersion::OptimizationTier0 &&
+            g_pConfig->TieredPGO_InstrumentOnlyHotCode())
         {
-            switch (g_pConfig->TieredPGO_Strategy())
+            if (ExecutionManager::IsReadyToRunCode(currentNativeCodeVersion.GetNativeCode()))
             {
-                // 0: In this mode previous tier is OptimizationTier0 only in case of R2R
-                case InstrumentColdNonPrejittedCode:
-                    _ASSERT(ExecutionManager::IsReadyToRunCode(currentNativeCodeVersion.GetNativeCode()));
-                    break;
-
-                // 1: Promote hot R2R code to TierInstrumented
-                case InstrumentColdNonPrejittedCode_InstrumentHotPrejittedCode:
-                    if (ExecutionManager::IsReadyToRunCode(currentNativeCodeVersion.GetNativeCode()))
-                    {
-                        nextTier = NativeCodeVersion::OptimizationTierInstrumented;
-                    }
-                    break;
-
-                // 2: Promote hot R2R code to TierInstrumentedOptimized
-                case InstrumentColdNonPrejittedCode_InstrumentHotPrejittedCode_Optimized:
-                    if (ExecutionManager::IsReadyToRunCode(currentNativeCodeVersion.GetNativeCode()))
-                    {
-                        nextTier = NativeCodeVersion::OptimizationTierInstrumentedOptimized;
-                    }
-                    break;
-
-                // 3: Promote hot Tier0/R2R code to TierInstrumented
-                case InstrumentHotNonPrejittedCode_InstrumentHotPrejittedCode:
-                    nextTier = NativeCodeVersion::OptimizationTierInstrumented;
-                    break;
-
-                // 4: Promote hot Tier0 to TierInstrumented and hot R2R to TierInstrumentedOptimized
-                case InstrumentHotNonPrejittedCode_InstrumentHotPrejittedCode_Optimized:
-                    if (ExecutionManager::IsReadyToRunCode(currentNativeCodeVersion.GetNativeCode()))
-                    {
-                        nextTier = NativeCodeVersion::OptimizationTierInstrumentedOptimized;
-                    }
-                    else
-                    {
-                        nextTier = NativeCodeVersion::OptimizationTierInstrumented;
-                    }
-                    break;
-
-                default:
-                    UNREACHABLE_MSG("Unknown TieredPGO_Strategy");
+                // We definitely don't want to use unoptimized instrumentation tier for hot R2R:
+                // 1) It will produce a lot of new compilations for small methods which were inlined in R2R
+                // 2) Noticeable performance regression from fast R2R to slow instrumented Tier0
+                nextTier = NativeCodeVersion::OptimizationTierInstrumentedOptimized;
+            }
+            else
+            {
+                // For ILOnly it's fine to use unoptimized instrumented tier:
+                // 1) No new compilations since previous tier already triggered them
+                // 2) Better profile since we'll be able to instrument inlinees
+                // 3) Unoptimized instrumented tier is faster to produce and wire up
+                nextTier = NativeCodeVersion::OptimizationTierInstrumented;
             }
         }
     }

src/tests/JIT/PGO/InstrumentedTiers/InstrumentedTiers_strat1.csproj → src/tests/JIT/PGO/InstrumentedTiers/InstrumentedTiers.csproj

@@ -6,13 +6,13 @@
       $(CLRTestBatchPreCommands)
       set DOTNET_TieredCompilation=1
       set DOTNET_TieredPGO=1
-      set TieredPGO_Strategy=1
+      set DOTNET_TieredPGO_InstrumentOnlyHotCode=1
       ]]></CLRTestBatchPreCommands>
           <BashCLRTestPreCommands><![CDATA[
       $(BashCLRTestPreCommands)
       export DOTNET_TieredCompilation=1
       export DOTNET_TieredPGO=1
-      export TieredPGO_Strategy=1
+      export DOTNET_TieredPGO_InstrumentOnlyHotCode=1
       ]]></BashCLRTestPreCommands>
   </PropertyGroup>
   <ItemGroup>