From e39da019354f2d0e97d393e6d1714339bd8f1912 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 8 Jul 2025 17:17:44 +0200 Subject: [PATCH 1/5] never set fgPgoSingleEdge for cctors or large single-edge blocks --- src/coreclr/jit/fgprofilesynthesis.cpp | 49 +++++++++++++++++++++++--- src/coreclr/jit/jit.h | 12 +------ 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/coreclr/jit/fgprofilesynthesis.cpp b/src/coreclr/jit/fgprofilesynthesis.cpp index 1e90c43c81b824..3f7a9c07089b32 100644 --- a/src/coreclr/jit/fgprofilesynthesis.cpp +++ b/src/coreclr/jit/fgprofilesynthesis.cpp @@ -250,14 +250,53 @@ void ProfileSynthesis::Run(ProfileSynthesisOption option) m_comp->fgPgoConsistent = !m_approximate; // A simple check whether the current method has more than one edge. - m_comp->fgPgoSingleEdge = true; - for (BasicBlock* const block : m_comp->Blocks()) + // Ignore static constructors - they're never expected to be called more than once. + const bool preferSize = m_comp->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_SIZE_OPT); + const bool isCctor = ((m_comp->info.compFlags & FLG_CCTOR) == FLG_CCTOR); + m_comp->fgPgoSingleEdge = !isCctor && !preferSize; + + if (m_comp->fgPgoSingleEdge) { - if (block->NumSucc() > 1) + for (BasicBlock* const block : m_comp->Blocks()) { - m_comp->fgPgoSingleEdge = false; - break; + if (block->NumSucc() > 1) + { + m_comp->fgPgoSingleEdge = false; + break; + } + } + } + + // fgPgoSingleEdge targets mostly small wrapper-like methods, so we ignore single-edge code + // with a lot of calls. + int callsCount = 0; + const int MaxCallsForSingleEdgeBlocks = 10; + if (m_comp->fgPgoSingleEdge) + { + // Loop all BBs again because the previous block-only loop was more likely to + // bail out early. + for (BasicBlock* const block : m_comp->Blocks()) + { + for (Statement* const stmt : block->Statements()) + { + if ((stmt->GetRootNode() != nullptr) && ((stmt->GetRootNode()->gtFlags & GTF_CALL) != 0)) + { + for (GenTree* const tree : stmt->TreeList()) + { + if (tree->IsCall()) + { + callsCount++; + if (callsCount > MaxCallsForSingleEdgeBlocks) + { + m_comp->fgPgoSingleEdge = false; + goto TOO_MANY_CALLS; + } + } + } + } + } } + TOO_MANY_CALLS: } m_comp->Metrics.ProfileSynthesizedBlendedOrRepaired++; diff --git a/src/coreclr/jit/jit.h b/src/coreclr/jit/jit.h index 3a3b85ba1d4863..e2a12b1a65c818 100644 --- a/src/coreclr/jit/jit.h +++ b/src/coreclr/jit/jit.h @@ -738,14 +738,7 @@ inline size_t unsigned_abs(int64_t x) #define FEATURE_TAILCALL_OPT_SHARED_RETURN 0 #endif // !FEATURE_TAILCALL_OPT -#define CLFLG_CODESIZE 0x00001 -#define CLFLG_CODESPEED 0x00002 -#define CLFLG_CSE 0x00004 #define CLFLG_REGVAR 0x00008 -#define CLFLG_RNGCHKOPT 0x00010 -#define CLFLG_DEADSTORE 0x00020 -#define CLFLG_CODEMOTION 0x00040 -#define CLFLG_QMARK 0x00080 #define CLFLG_TREETRANS 0x00100 #define CLFLG_INLINING 0x00200 @@ -761,10 +754,7 @@ inline size_t unsigned_abs(int64_t x) #define FEATURE_LOOP_ALIGN 0 #endif -#define CLFLG_MAXOPT \ - (CLFLG_CSE | CLFLG_REGVAR | CLFLG_RNGCHKOPT | CLFLG_DEADSTORE | CLFLG_CODEMOTION | CLFLG_QMARK | CLFLG_TREETRANS | \ - CLFLG_INLINING | CLFLG_STRUCTPROMOTE) - +#define CLFLG_MAXOPT (CLFLG_REGVAR | CLFLG_TREETRANS | CLFLG_INLINING | CLFLG_STRUCTPROMOTE) #define CLFLG_MINOPT (CLFLG_TREETRANS) /*****************************************************************************/ From 60ccf1d1751b1ac4192c96501e356cbf58de67fc Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 8 Jul 2025 17:21:58 +0200 Subject: [PATCH 2/5] cleanup --- src/coreclr/jit/fgprofilesynthesis.cpp | 9 +++++---- src/coreclr/jit/jit.h | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/fgprofilesynthesis.cpp b/src/coreclr/jit/fgprofilesynthesis.cpp index 3f7a9c07089b32..82597f1dcdab13 100644 --- a/src/coreclr/jit/fgprofilesynthesis.cpp +++ b/src/coreclr/jit/fgprofilesynthesis.cpp @@ -251,8 +251,8 @@ void ProfileSynthesis::Run(ProfileSynthesisOption option) // A simple check whether the current method has more than one edge. // Ignore static constructors - they're never expected to be called more than once. - const bool preferSize = m_comp->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_SIZE_OPT); - const bool isCctor = ((m_comp->info.compFlags & FLG_CCTOR) == FLG_CCTOR); + const bool preferSize = m_comp->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_SIZE_OPT); + const bool isCctor = ((m_comp->info.compFlags & FLG_CCTOR) == FLG_CCTOR); m_comp->fgPgoSingleEdge = !isCctor && !preferSize; if (m_comp->fgPgoSingleEdge) @@ -269,7 +269,7 @@ void ProfileSynthesis::Run(ProfileSynthesisOption option) // fgPgoSingleEdge targets mostly small wrapper-like methods, so we ignore single-edge code // with a lot of calls. - int callsCount = 0; + int callsCount = 0; const int MaxCallsForSingleEdgeBlocks = 10; if (m_comp->fgPgoSingleEdge) { @@ -289,6 +289,7 @@ void ProfileSynthesis::Run(ProfileSynthesisOption option) if (callsCount > MaxCallsForSingleEdgeBlocks) { m_comp->fgPgoSingleEdge = false; + JITDUMP("Too many calls for fgPgoSingleEdge - bail out.\n") goto TOO_MANY_CALLS; } } @@ -296,8 +297,8 @@ void ProfileSynthesis::Run(ProfileSynthesisOption option) } } } - TOO_MANY_CALLS: } +TOO_MANY_CALLS: m_comp->Metrics.ProfileSynthesizedBlendedOrRepaired++; diff --git a/src/coreclr/jit/jit.h b/src/coreclr/jit/jit.h index e2a12b1a65c818..d1e69950efc631 100644 --- a/src/coreclr/jit/jit.h +++ b/src/coreclr/jit/jit.h @@ -738,9 +738,9 @@ inline size_t unsigned_abs(int64_t x) #define FEATURE_TAILCALL_OPT_SHARED_RETURN 0 #endif // !FEATURE_TAILCALL_OPT -#define CLFLG_REGVAR 0x00008 -#define CLFLG_TREETRANS 0x00100 -#define CLFLG_INLINING 0x00200 +#define CLFLG_REGVAR 0x00008 +#define CLFLG_TREETRANS 0x00100 +#define CLFLG_INLINING 0x00200 #if FEATURE_STRUCTPROMOTE #define CLFLG_STRUCTPROMOTE 0x00400 From cde495f89a37df6cb4acac2eb174a96707a69ba4 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 8 Jul 2025 18:10:52 +0200 Subject: [PATCH 3/5] Address feedback --- src/coreclr/jit/compiler.cpp | 5 +-- src/coreclr/jit/compiler.h | 5 +-- src/coreclr/jit/fgbasic.cpp | 1 + src/coreclr/jit/fgprofilesynthesis.cpp | 42 +++----------------------- 4 files changed, 12 insertions(+), 41 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index ca4bd2a1366010..21321d7c69a480 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -2550,8 +2550,9 @@ void Compiler::compInitOptions(JitFlags* jitFlags) opts.genFPorder = true; opts.genFPopt = true; - opts.instrCount = 0; - opts.lvRefCount = 0; + opts.instrCount = 0; + opts.callInstrCount = 0; + opts.lvRefCount = 0; #ifdef PROFILING_SUPPORTED opts.compJitELTHookEnabled = false; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 9a2305d3eefb1c..e1fe40fbff91d7 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -9913,8 +9913,9 @@ class Compiler compSupportsISA = isas; } - unsigned compFlags; // method attributes - unsigned instrCount = 0; + unsigned compFlags; // method attributes + unsigned instrCount = 0; // number of IL opcodes + unsigned callInstrCount = 0; // number of IL opcodes (calls only). unsigned lvRefCount; codeOptimize compCodeOpt; // what type of code optimizations diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 4fa17a9c6c8207..ad17e3ddacf131 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -3171,6 +3171,7 @@ void Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed case CEE_CALLVIRT: case CEE_CALLI: { + opts.callInstrCount++; if (compIsForInlining() || // Ignore tail call in the inlinee. Period. (!tailCall && !compTailCallStress()) // A new BB with BBJ_RETURN would have been created diff --git a/src/coreclr/jit/fgprofilesynthesis.cpp b/src/coreclr/jit/fgprofilesynthesis.cpp index 82597f1dcdab13..e92b5e3138429e 100644 --- a/src/coreclr/jit/fgprofilesynthesis.cpp +++ b/src/coreclr/jit/fgprofilesynthesis.cpp @@ -249,12 +249,13 @@ void ProfileSynthesis::Run(ProfileSynthesisOption option) m_comp->fgPgoSynthesized = true; m_comp->fgPgoConsistent = !m_approximate; - // A simple check whether the current method has more than one edge. - // Ignore static constructors - they're never expected to be called more than once. + // If a method has just one edge, we simulate having PGO data for it since we typically + // don't instrument such methods. To avoid giving excessive inlining boost to large and/or + // infrequently executed methods, we apply the following heuristics to exclude: + // const bool preferSize = m_comp->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_SIZE_OPT); const bool isCctor = ((m_comp->info.compFlags & FLG_CCTOR) == FLG_CCTOR); - m_comp->fgPgoSingleEdge = !isCctor && !preferSize; - + m_comp->fgPgoSingleEdge = !isCctor && !preferSize && m_comp->opts.callInstrCount < 10; if (m_comp->fgPgoSingleEdge) { for (BasicBlock* const block : m_comp->Blocks()) @@ -267,39 +268,6 @@ void ProfileSynthesis::Run(ProfileSynthesisOption option) } } - // fgPgoSingleEdge targets mostly small wrapper-like methods, so we ignore single-edge code - // with a lot of calls. - int callsCount = 0; - const int MaxCallsForSingleEdgeBlocks = 10; - if (m_comp->fgPgoSingleEdge) - { - // Loop all BBs again because the previous block-only loop was more likely to - // bail out early. - for (BasicBlock* const block : m_comp->Blocks()) - { - for (Statement* const stmt : block->Statements()) - { - if ((stmt->GetRootNode() != nullptr) && ((stmt->GetRootNode()->gtFlags & GTF_CALL) != 0)) - { - for (GenTree* const tree : stmt->TreeList()) - { - if (tree->IsCall()) - { - callsCount++; - if (callsCount > MaxCallsForSingleEdgeBlocks) - { - m_comp->fgPgoSingleEdge = false; - JITDUMP("Too many calls for fgPgoSingleEdge - bail out.\n") - goto TOO_MANY_CALLS; - } - } - } - } - } - } - } -TOO_MANY_CALLS: - m_comp->Metrics.ProfileSynthesizedBlendedOrRepaired++; if (m_approximate) From 8773590d129c1de13fd4ec56d5ef3d6f6a4c1bbd Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 8 Jul 2025 18:15:55 +0200 Subject: [PATCH 4/5] clean up --- src/coreclr/jit/fgprofilesynthesis.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/fgprofilesynthesis.cpp b/src/coreclr/jit/fgprofilesynthesis.cpp index e92b5e3138429e..5c1693315ab9a7 100644 --- a/src/coreclr/jit/fgprofilesynthesis.cpp +++ b/src/coreclr/jit/fgprofilesynthesis.cpp @@ -254,8 +254,9 @@ void ProfileSynthesis::Run(ProfileSynthesisOption option) // infrequently executed methods, we apply the following heuristics to exclude: // const bool preferSize = m_comp->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_SIZE_OPT); - const bool isCctor = ((m_comp->info.compFlags & FLG_CCTOR) == FLG_CCTOR); - m_comp->fgPgoSingleEdge = !isCctor && !preferSize && m_comp->opts.callInstrCount < 10; + const bool isCctor = (m_comp->info.compFlags & FLG_CCTOR) == FLG_CCTOR; + m_comp->fgPgoSingleEdge = !isCctor && !preferSize && (m_comp->opts.callInstrCount < 10); + if (m_comp->fgPgoSingleEdge) { for (BasicBlock* const block : m_comp->Blocks()) From 637301a2f2f5b798e5f53427cb81e09ca6764b0b Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Tue, 8 Jul 2025 20:50:33 +0200 Subject: [PATCH 5/5] Update fgbasic.cpp --- src/coreclr/jit/fgbasic.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index ad17e3ddacf131..76f841f8b2e275 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -3290,6 +3290,7 @@ void Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed // These ctrl-flow opcodes don't need any special handling case CEE_NEWOBJ: // CTRL_CALL + opts.callInstrCount++; break; // what's left are forgotten instructions