From d74460bcdba752bee1d9a57ff65d08c78d7e37b6 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 6 Aug 2025 11:38:12 -0700 Subject: [PATCH 1/3] JIT: create inferred GDVs for enumerator var uses that lack PGO Long-running enumerator loops at Tier0+instr will see their executions transition over to a non-instrumented OSR version of the code. This can cause loss of PGO data in the portions of the method that execute after the leaving the loop that inspires OSR. For enumerator vars we can safely deduce the likely classes from probes made earlier in the method. So when we see a class profile for an enumerator var, remember it and use it for subsequent calls that lack their own profile data. Addresses part of #118420. --- src/coreclr/jit/compiler.h | 23 ++++++++- src/coreclr/jit/importercalls.cpp | 86 +++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 8707af8c967e0f..414dff92480e47 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4488,9 +4488,16 @@ class Compiler // Enumerator de-abstraction support // + struct InferredGdvEntry + { + CORINFO_CLASS_HANDLE m_classHandle; + unsigned m_likelihood; + }; + typedef JitHashTable, unsigned> NodeToUnsignedMap; + typedef JitHashTable, InferredGdvEntry> VarToLikelyClassMap; - // Map is only set on the root instance. + // Maps are only set on the root instance. // NodeToUnsignedMap* impEnumeratorGdvLocalMap = nullptr; bool hasImpEnumeratorGdvLocalMap() { return impInlineRoot()->impEnumeratorGdvLocalMap != nullptr; } @@ -4506,6 +4513,20 @@ class Compiler return compiler->impEnumeratorGdvLocalMap; } + VarToLikelyClassMap* impEnumeratorLikelyTypeMap = nullptr; + bool hasEnumeratorLikelyTypeMap() { return impInlineRoot()->impEnumeratorLikelyTypeMap != nullptr; } + VarToLikelyClassMap* getImpEnumeratorLikelyTypeMap() + { + Compiler* compiler = impInlineRoot(); + if (compiler->impEnumeratorLikelyTypeMap == nullptr) + { + CompAllocator alloc(compiler->getAllocator(CMK_Generic)); + compiler->impEnumeratorLikelyTypeMap = new (alloc) VarToLikelyClassMap(alloc); + } + + return compiler->impEnumeratorLikelyTypeMap; + } + bool hasUpdatedTypeLocals = false; #define SMALL_STACK_SIZE 16 // number of elements in impSmallStack diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index bc7f0433287a10..c1938e4a6c6125 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -6986,6 +6986,7 @@ void Compiler::pickGDV(GenTreeCall* call, const int maxLikelyMethods = MAX_GDV_TYPE_CHECKS; LikelyClassMethodRecord likelyMethods[maxLikelyMethods]; unsigned numberOfMethods = 0; + bool isInferredGDV = false; // TODO-GDV: R2R support requires additional work to reacquire the // entrypoint, similar to what happens at the end of impDevirtualizeCall. @@ -7000,6 +7001,39 @@ void Compiler::pickGDV(GenTreeCall* call, pgoInfo.PgoData, ilOffset); } + if ((numberOfClasses < 1) && (numberOfMethods < 1)) + { + // See if we can infer a GDV here for enumerator var uses + // + CallArg* const thisArg = call->gtArgs.FindWellKnownArg(WellKnownArg::ThisPointer); + + if (thisArg != nullptr) + { + GenTree* const thisNode = thisArg->GetEarlyNode(); + if (thisNode->OperIs(GT_LCL_VAR)) + { + GenTreeLclVarCommon* thisLclNode = thisNode->AsLclVarCommon(); + LclVarDsc* const thisVarDsc = lvaGetDesc(thisLclNode); + unsigned const thisLclNum = thisLclNode->GetLclNum(); + + if (thisVarDsc->lvIsEnumerator) + { + VarToLikelyClassMap* const map = getImpEnumeratorLikelyTypeMap(); + InferredGdvEntry e; + if (map->Lookup(thisLclNum, &e)) + { + JITDUMP("Recalling that V%02u has %u%% likely class %s\n", thisLclNum, e.m_likelihood, + eeGetClassName(e.m_classHandle)); + numberOfClasses = 1; + likelyClasses[0].handle = (INT_PTR)e.m_classHandle; + likelyClasses[0].likelihood = e.m_likelihood; + isInferredGDV = true; + } + } + } + } + } + if ((numberOfClasses < 1) && (numberOfMethods < 1)) { if (verboseLogging) @@ -7186,6 +7220,58 @@ void Compiler::pickGDV(GenTreeCall* call, JITDUMP("Accepting type %s with likelihood %u as a candidate\n", eeGetClassName(classGuesses[guessIdx]), likelihoods[guessIdx]) } + + // If the 'this' arg to the call is an enumerator var, record any + // dominant likely class so we can possibly infer a GDV at places where we + // never observed the var's value. (eg an unreached Dispose call if + // control is hijacked out of Tier0+i by OSR). + // + // Note enumerator vars are special as they are generally not redefined + // and we want to ensure all methods called on them get inlined to enable + // escape analysis to kick in, if possible. + // + const unsigned dominantLikelihood = 50; + + if (!isInferredGDV && (likelihoods[guessIdx] >= dominantLikelihood)) + { + CallArg* const thisArg = call->gtArgs.FindWellKnownArg(WellKnownArg::ThisPointer); + + if (thisArg != nullptr) + { + GenTree* const thisNode = thisArg->GetEarlyNode(); + if (thisNode->OperIs(GT_LCL_VAR)) + { + GenTreeLclVarCommon* thisLclNode = thisNode->AsLclVarCommon(); + LclVarDsc* const thisVarDsc = lvaGetDesc(thisLclNode); + unsigned const thisLclNum = thisLclNode->GetLclNum(); + + if (thisVarDsc->lvIsEnumerator) + { + VarToLikelyClassMap* const map = getImpEnumeratorLikelyTypeMap(); + + // If we have multiple type observations, we just use the first. + // + // Note importation order is somewhat reverse-post-orderish; + // a block is only imported if one of its imported preds is imported. + // + // Enumerator vars tend to have a dominating MoveNext call that will + // be the one subsequent uses will see, if they lack their own + // type observations. + // + if (!map->Lookup(thisLclNum)) + { + InferredGdvEntry e; + e.m_classHandle = classGuesses[guessIdx]; + e.m_likelihood = likelihoods[guessIdx]; + + JITDUMP("Rememmbering that V%02u has %u%% likely class %s\n", thisLclNum, + e.m_likelihood, eeGetClassName(e.m_classHandle)); + map->Set(thisLclNum, e); + } + } + } + } + } } else { From c7948a2ec5a2578643e95c5ff47eb0a6e2c2e255 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 6 Aug 2025 11:51:16 -0700 Subject: [PATCH 2/3] Update src/coreclr/jit/importercalls.cpp Fix spelling Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/coreclr/jit/importercalls.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index c1938e4a6c6125..7d53dda07789c7 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -7264,7 +7264,7 @@ void Compiler::pickGDV(GenTreeCall* call, e.m_classHandle = classGuesses[guessIdx]; e.m_likelihood = likelihoods[guessIdx]; - JITDUMP("Rememmbering that V%02u has %u%% likely class %s\n", thisLclNum, + JITDUMP("Remembering that V%02u has %u%% likely class %s\n", thisLclNum, e.m_likelihood, eeGetClassName(e.m_classHandle)); map->Set(thisLclNum, e); } From 037ed3652696d9716402087378c8d06a1b6a5dbb Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 6 Aug 2025 15:02:19 -0700 Subject: [PATCH 3/3] review feedback --- src/coreclr/jit/importercalls.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 7d53dda07789c7..dbc0a7f46eb61e 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -7001,7 +7001,7 @@ void Compiler::pickGDV(GenTreeCall* call, pgoInfo.PgoData, ilOffset); } - if ((numberOfClasses < 1) && (numberOfMethods < 1)) + if ((numberOfClasses < 1) && (numberOfMethods < 1) && hasEnumeratorLikelyTypeMap()) { // See if we can infer a GDV here for enumerator var uses //