From b2259dd71ff792fdb40ab67adb38589cf2227b8a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 6 Jul 2021 23:51:44 +0300 Subject: [PATCH 1/2] Set default value for JitExtDefaultPolicyProfTrust 0 --- src/coreclr/jit/jitconfigvalues.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 7527fd49221ce0..bfa343acb345dd 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -462,8 +462,19 @@ CONFIG_STRING(JitInlineReplayFile, W("JitInlineReplayFile")) CONFIG_INTEGER(JitExtDefaultPolicy, W("JitExtDefaultPolicy"), 1) CONFIG_INTEGER(JitExtDefaultPolicyMaxIL, W("JitExtDefaultPolicyMaxIL"), 0x64) CONFIG_INTEGER(JitExtDefaultPolicyMaxBB, W("JitExtDefaultPolicyMaxBB"), 7) + +// Inliner uses the following formula for PGO-driven decisions: +// +// BM = BM * ((1.0 - ProfTrust) + ProfWeight * ProfScale) +// +// Where BM is a benefit multiplier composed from various observations (e.g. "const arg makes a branch foldable"). +// If a profile data can be trusted for 100% we can safely just give up on inlining anything inside cold blocks +// (except the cases where inlining in cold blocks improves type info/escape analysis for the whole caller). +// It improves JIT's TP and makes prejitted code smaller. +// The default value here is set to zero because our built-in profile just can't be 100% suitable for all cases. +// Currently, we can't tell in JIT what was the source of it. +CONFIG_INTEGER(JitExtDefaultPolicyProfTrust, W("JitExtDefaultPolicyProfTrust"), 0x0) CONFIG_INTEGER(JitExtDefaultPolicyProfScale, W("JitExtDefaultPolicyProfScale"), 0x2A) -CONFIG_INTEGER(JitExtDefaultPolicyProfTrust, W("JitExtDefaultPolicyProfTrust"), 0x7) CONFIG_INTEGER(JitInlinePolicyModel, W("JitInlinePolicyModel"), 0) CONFIG_INTEGER(JitInlinePolicyProfile, W("JitInlinePolicyProfile"), 0) From c99a877562ffdd7fe364d870a18004377b9d818a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 7 Jul 2021 22:36:23 +0300 Subject: [PATCH 2/2] Address feedback --- src/coreclr/jit/inlinepolicy.cpp | 10 +++++++++- src/coreclr/jit/jitconfigvalues.h | 8 +++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/inlinepolicy.cpp b/src/coreclr/jit/inlinepolicy.cpp index c328233cdf5bb4..fdf4ae19341c2f 100644 --- a/src/coreclr/jit/inlinepolicy.cpp +++ b/src/coreclr/jit/inlinepolicy.cpp @@ -1648,7 +1648,15 @@ double ExtendedDefaultPolicy::DetermineMultiplier() const double profileTrustCoef = (double)JitConfig.JitExtDefaultPolicyProfTrust() / 10.0; const double profileScale = (double)JitConfig.JitExtDefaultPolicyProfScale() / 10.0; - multiplier *= (1.0 - profileTrustCoef) + min(m_ProfileFrequency, 1.0) * profileScale; + if (m_RootCompiler->fgPgoSource == ICorJitInfo::PgoSource::Dynamic) + { + // For now we only "trust" dynamic profiles. + multiplier *= (1.0 - profileTrustCoef) + min(m_ProfileFrequency, 1.0) * profileScale; + } + else + { + multiplier *= min(m_ProfileFrequency, 1.0) * profileScale; + } JITDUMP("\nCallsite has profile data: %g.", m_ProfileFrequency); } diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index bfa343acb345dd..cc10adc0b88097 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -468,12 +468,10 @@ CONFIG_INTEGER(JitExtDefaultPolicyMaxBB, W("JitExtDefaultPolicyMaxBB"), 7) // BM = BM * ((1.0 - ProfTrust) + ProfWeight * ProfScale) // // Where BM is a benefit multiplier composed from various observations (e.g. "const arg makes a branch foldable"). -// If a profile data can be trusted for 100% we can safely just give up on inlining anything inside cold blocks +// If a profile data can be trusted for 100% we can safely just give up on inlining anything inside cold blocks // (except the cases where inlining in cold blocks improves type info/escape analysis for the whole caller). -// It improves JIT's TP and makes prejitted code smaller. -// The default value here is set to zero because our built-in profile just can't be 100% suitable for all cases. -// Currently, we can't tell in JIT what was the source of it. -CONFIG_INTEGER(JitExtDefaultPolicyProfTrust, W("JitExtDefaultPolicyProfTrust"), 0x0) +// For now, it's only applied for dynamic PGO. +CONFIG_INTEGER(JitExtDefaultPolicyProfTrust, W("JitExtDefaultPolicyProfTrust"), 0x7) CONFIG_INTEGER(JitExtDefaultPolicyProfScale, W("JitExtDefaultPolicyProfScale"), 0x2A) CONFIG_INTEGER(JitInlinePolicyModel, W("JitInlinePolicyModel"), 0)