From a4c765d9f4c4221c7bb0c9d6336bf1f3c92d5db9 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 9 Mar 2021 08:35:15 -0800 Subject: [PATCH] JIT: update jit config defaults for PGO (#49267) Enable edge profiling by default, when jitting, or when prejitting for R2R. Keep block profiling there for classic ngen (so there is an entry probe). Enable minimal probing by default, when jitting. Keep full probing there for prejitting so presence of counts in a method implies method was executed. Enable class profiling by default when jitting. Still TBD if we will also do this when prejitting. Enable guarded devirtualization by default (will only kick in for PGO). Adjust experimental CI legs to reflect the above. Net effect is that when jitting, COMPlus_TieredPGO=1 turns on dynamic PGO with the expected set of behaviors, without any other options. We'll still need to set COMPlus_TC_QuickJitForLoops=1 to have methods with loops pass through Tier0. --- .../templates/runtimes/run-test-job.yml | 3 - src/coreclr/jit/fgprofile.cpp | 59 +++++++++++++++---- src/coreclr/jit/jitconfigvalues.h | 11 ++-- src/tests/Common/testenvironment.proj | 7 +-- 4 files changed, 55 insertions(+), 25 deletions(-) diff --git a/eng/pipelines/common/templates/runtimes/run-test-job.yml b/eng/pipelines/common/templates/runtimes/run-test-job.yml index 40da880c11387..54436b52f7a52 100644 --- a/eng/pipelines/common/templates/runtimes/run-test-job.yml +++ b/eng/pipelines/common/templates/runtimes/run-test-job.yml @@ -471,13 +471,10 @@ jobs: scenarios: - jitosr - jitosr_stress - - jitguardeddevirtualization - jitehwritethru - jitobjectstackallocation - jitpgo - jitpgo_inline - - jitpgo_classes - - jitpgo_edgeinstrumentation ${{ if in(parameters.testGroup, 'ilasm') }}: scenarios: - ilasmroundtrip diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 9e9e3faa7a05a..e81d61c2398c7 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -1487,35 +1487,54 @@ PhaseStatus Compiler::fgPrepareToInstrumentMethod() // Choose instrumentation technology. // + // We enable edge profiling by default, except when: + // * disabled by option + // * we are prejitting via classic ngen + // * we are jitting osr methods + // // Currently, OSR is incompatible with edge profiling. So if OSR is enabled, // always do block profiling. // // Note this incompatibility only exists for methods that actually have // patchpoints, but we won't know that until we import. // - const bool methodMayHavePatchpoints = - (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_TIER0) && (JitConfig.TC_OnStackReplacement() > 0)); + CLANG_FORMAT_COMMENT_ANCHOR; - if ((JitConfig.JitEdgeProfiling() > 0) && !methodMayHavePatchpoints) +#ifdef FEATURE_READYTORUN_COMPILER + const bool r2r = opts.IsReadyToRun(); +#else + const bool r2r = false; +#endif + const bool prejit = opts.jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT); + const bool classicNgen = prejit && !r2r; + const bool osr = (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_TIER0) && (JitConfig.TC_OnStackReplacement() > 0)); + const bool useEdgeProfiles = (JitConfig.JitEdgeProfiling() > 0) && !classicNgen && !osr; + + if (useEdgeProfiles) { fgCountInstrumentor = new (this, CMK_Pgo) EfficientEdgeCountInstrumentor(this); } else { - if (JitConfig.JitEdgeProfiling() > 0) - { - JITDUMP("OSR and edge profiling not yet compatible; using block profiling\n"); - } + JITDUMP("Using block profiling, because %s\n", + (JitConfig.JitEdgeProfiling() > 0) ? "edge profiles disabled" : classicNgen ? "classic Ngen" : "OSR"); fgCountInstrumentor = new (this, CMK_Pgo) BlockCountInstrumentor(this); } - if (JitConfig.JitClassProfiling() > 0) + // Enable class profiling by default, when jitting. + // Todo: we may also want this on by default for prejitting. + // + const bool useClassProfiles = (JitConfig.JitClassProfiling() > 0) && !prejit; + if (useClassProfiles) { fgClassInstrumentor = new (this, CMK_Pgo) ClassProbeInstrumentor(this); } else { + JITDUMP("Not doing class profiling, because %s\n", + (JitConfig.JitClassProfiling() > 0) ? "class profiles disabled" : "prejit"); + fgClassInstrumentor = new (this, CMK_Pgo) NonInstrumentor(this); } @@ -1575,13 +1594,29 @@ PhaseStatus Compiler::fgInstrumentMethod() // assert(fgClassInstrumentor->SchemaCount() == info.compClassProbeCount); - // Optionally, if there were no class probes and only one count probe, + // Optionally, when jitting, if there were no class probes and only one count probe, // suppress instrumentation. // - if ((JitConfig.JitMinimalProfiling() > 0) && (fgCountInstrumentor->SchemaCount() == 1) && - (fgClassInstrumentor->SchemaCount() == 0)) + // We leave instrumentation in place when prejitting as the sample hits in the method + // may be used to determine if the method should be prejitted or not. + // + // For jitting, no information is conveyed by the count in a single=block method. + // + bool minimalProbeMode = false; + + if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT)) + { + minimalProbeMode = (JitConfig.JitMinimalPrejitProfiling() > 0); + } + else + { + minimalProbeMode = (JitConfig.JitMinimalJitProfiling() > 0); + } + + if (minimalProbeMode && (fgCountInstrumentor->SchemaCount() == 1) && (fgClassInstrumentor->SchemaCount() == 0)) { - JITDUMP("Not instrumenting method: only one counter, and no class probes\n"); + JITDUMP( + "Not instrumenting method: minimal probing enabled, and method has only one counter and no class probes\n"); return PhaseStatus::MODIFIED_NOTHING; } diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 8019c61e0496d..eb89d630e67fa 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -439,8 +439,8 @@ CONFIG_INTEGER(JitEnableFinallyCloning, W("JitEnableFinallyCloning"), 1) CONFIG_INTEGER(JitEnableRemoveEmptyTry, W("JitEnableRemoveEmptyTry"), 1) #endif // DEBUG -// Overall master enable for Guarded Devirtualization. Currently not enabled by default. -CONFIG_INTEGER(JitEnableGuardedDevirtualization, W("JitEnableGuardedDevirtualization"), 0) +// Overall master enable for Guarded Devirtualization. +CONFIG_INTEGER(JitEnableGuardedDevirtualization, W("JitEnableGuardedDevirtualization"), 1) #if defined(DEBUG) // Various policies for GuardedDevirtualization @@ -453,9 +453,10 @@ CONFIG_INTEGER(TC_OnStackReplacement, W("TC_OnStackReplacement"), 0) CONFIG_INTEGER(TC_OnStackReplacement_InitialCounter, W("TC_OnStackReplacement_InitialCounter"), 1000) // Profile instrumentation options -CONFIG_INTEGER(JitMinimalProfiling, W("JitMinimalProfiling"), 0) -CONFIG_INTEGER(JitClassProfiling, W("JitClassProfiling"), 0) -CONFIG_INTEGER(JitEdgeProfiling, W("JitEdgeProfiling"), 0) +CONFIG_INTEGER(JitMinimalJitProfiling, W("JitMinimalJitProfiling"), 1) +CONFIG_INTEGER(JitMinimalPrejitProfiling, W("JitMinimalPrejitProfiling"), 0) +CONFIG_INTEGER(JitClassProfiling, W("JitClassProfiling"), 1) +CONFIG_INTEGER(JitEdgeProfiling, W("JitEdgeProfiling"), 1) // Control when Virtual Calls are expanded CONFIG_INTEGER(JitExpandCallsEarly, W("JitExpandCallsEarly"), 1) // Expand Call targets early (in the global morph diff --git a/src/tests/Common/testenvironment.proj b/src/tests/Common/testenvironment.proj index a4460c53130c6..b368747b4aebd 100644 --- a/src/tests/Common/testenvironment.proj +++ b/src/tests/Common/testenvironment.proj @@ -147,11 +147,8 @@ - - - - - + +