From 63c58378c8d8bea39e903265eaeeb986bc0c0e3a Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 26 Aug 2026 18:53:50 -0700 Subject: [PATCH 1/2] JIT: stop EnableExtraSuperPmiQueries from changing what gets compiled EnableExtraSuperPmiQueries makes additional JIT-EE queries so that recorded SuperPMI method contexts carry data a later replay might ask for. superpmi.py sets it for every collection, and forwards it to crossgen2 as --codegenopt. Such a query has to be observationally inert, and it was not: the EE is free to fail a query the JIT would never have made on its own. crossgen2's embedClassHandle throws RequiresRuntimeJitException for a type that does not version with the compilation bubble. Under a non-composite crossgen2 collection the extra query made after a newarr hit that for ordinary element types such as int[]; the exception escaped the diagnostic block, impJitErrorTrap caught it and abandoned the enclosing inline, and the collection then imported less IL than a replay of the same context does. Replay reached a call the collection never imported, asked for a token that was never recorded, and superpmi.py --clean discarded the context. Wrap the extra queries so an EE failure cannot escape. eeRunExtraSuperPmiQueries nests the two existing traps: runWithErrorTrap absorbs the EE's failure during collection, and runWithSPMIErrorTrap absorbs SuperPMI's missing-data failure for a replay that re-enables the queries against a context collected without them. Both are needed, because the collector shim services runWithSPMIErrorTrap itself and only forwards runWithErrorTrap to crossgen2. Only EE queries go inside a trap. JIT work stays outside, so a noway_assert, NOMEM or assert still fails the method rather than being absorbed. Where the queries run JIT work that sets compFloatingPointUsed, save and restore it, since that flag reaches LSRA and ARM frame encoding. Measured over 15 framework assemblies, non-composite crossgen2, merged with mcs -merge -recursive -dedup -thin and replayed with superpmi.exe: before 30,170 contexts 919 missing 3.05% after 32,213 contexts 20 missing 0.062% The remaining 20 reproduce identically with the queries disabled, so they are a separate pre-existing issue. Collection also keeps more contexts, because methods whose inlines were previously abandoned now compile. Replaying one 1,158-context collection with and without the queries now produces identical JitDisasmSummary output for every method. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c6bbfae-eed7-4545-8f2a-5cfb15d62df4 --- src/coreclr/jit/compiler.cpp | 42 ++++++++++++++------------- src/coreclr/jit/compiler.h | 47 +++++++++++++++++++++++++++++++ src/coreclr/jit/importer.cpp | 44 +++++++++++++++++++++++------ src/coreclr/jit/importercalls.cpp | 24 ++++++++++------ src/coreclr/jit/lclvars.cpp | 13 +++++++-- 5 files changed, 132 insertions(+), 38 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 156a5eeb910641..137d4aef25d0a5 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -785,7 +785,9 @@ var_types Compiler::getReturnTypeForStruct(CORINFO_CLASS_HANDLE clsHnd, // if (JitConfig.EnableExtraSuperPmiQueries() && IsReadyToRun()) { - info.compCompHnd->getWasmLowering(clsHnd); + eeRunExtraSuperPmiQueries([&]() { + info.compCompHnd->getWasmLowering(clsHnd); + }); } #endif // DEBUG #endif // defined(TARGET_WASM) @@ -6227,31 +6229,33 @@ int Compiler::compCompileAfterInit(CORINFO_MODULE_HANDLE classPtr, #ifdef DEBUG if (JitConfig.EnableExtraSuperPmiQueries()) { - // Get the assembly name, to aid finding any particular SuperPMI method context function - (void)eeGetClassAssemblyName(info.compClassHnd); + eeRunExtraSuperPmiQueries([&]() { + // Get the assembly name, to aid finding any particular SuperPMI method context function + (void)eeGetClassAssemblyName(info.compClassHnd); - // Fetch class names for the method's generic parameters. - // - CORINFO_SIG_INFO sig; - info.compCompHnd->getMethodSig(info.compMethodHnd, &sig, nullptr); + // Fetch class names for the method's generic parameters. + // + CORINFO_SIG_INFO sig; + info.compCompHnd->getMethodSig(info.compMethodHnd, &sig, nullptr); - const unsigned classInst = sig.sigInst.classInstCount; - if (classInst > 0) - { - for (unsigned i = 0; i < classInst; i++) + const unsigned classInst = sig.sigInst.classInstCount; + if (classInst > 0) { - eeGetClassName(sig.sigInst.classInst[i]); + for (unsigned i = 0; i < classInst; i++) + { + eeGetClassName(sig.sigInst.classInst[i]); + } } - } - const unsigned methodInst = sig.sigInst.methInstCount; - if (methodInst > 0) - { - for (unsigned i = 0; i < methodInst; i++) + const unsigned methodInst = sig.sigInst.methInstCount; + if (methodInst > 0) { - eeGetClassName(sig.sigInst.methInst[i]); + for (unsigned i = 0; i < methodInst; i++) + { + eeGetClassName(sig.sigInst.methInst[i]); + } } - } + }); } #endif // DEBUG diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index eada0554a807f1..91284ff441a05c 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -9832,6 +9832,53 @@ class Compiler bool eeRunWithSPMIErrorTrapImp(void (*function)(void*), void* param); + template + bool eeRunFunctorWithErrorTrap(Functor f) + { + return eeRunWithErrorTrap( + [](Functor* pf) { + (*pf)(); + }, + &f); + } + +#ifdef DEBUG + //------------------------------------------------------------------------ + // eeRunExtraSuperPmiQueries: make JIT-EE queries whose only purpose is to enrich + // the recorded SuperPMI method context (see JitConfig.EnableExtraSuperPmiQueries). + // + // Type parameters: + // Functor - callable that makes the queries + // + // Arguments: + // f - the functor + // + // Notes: + // Extra queries must be observationally inert: enabling them must not change what + // the JIT compiles. That is not automatic, because the EE may fail a query that the + // JIT would never have made on its own. An AOT compiler in particular throws for a + // handle it cannot embed, such as a type outside the current version bubble, and an + // escaping exception would abort the enclosing inline or method. The collection + // would then import less IL than a later replay does, and the context it recorded + // would be missing the data that replay goes on to ask for. + // + // The inner trap absorbs such an EE failure during collection. The outer one absorbs + // SuperPMI's own "missing data" failure, for a replay that re-enables the queries + // against a context collected without them. + // + // Wrap only EE queries. JIT work must stay outside, because neither trap discriminates + // by origin: a noway_assert, NOMEM or assert raised inside the functor would be quietly + // absorbed instead of failing the method. + // + template + void eeRunExtraSuperPmiQueries(Functor f) + { + eeRunFunctorWithSPMIErrorTrap([&]() { + eeRunFunctorWithErrorTrap(f); + }); + } +#endif // DEBUG + // Utility functions static CORINFO_METHOD_HANDLE eeFindHelper(unsigned helper); diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 9c1d8b3c47ae96..560dec0add32e8 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -10260,17 +10260,45 @@ void Compiler::impImportBlockCode(BasicBlock* block) // if (JitConfig.EnableExtraSuperPmiQueries() && !eeIsSharedInst(resolvedToken.hClass)) { - void* pEmbedClsHnd; - info.compCompHnd->embedClassHandle(resolvedToken.hClass, &pEmbedClsHnd); - CORINFO_CLASS_HANDLE elemClsHnd = NO_CLASS_HANDLE; - CorInfoType elemCorType = info.compCompHnd->getChildType(resolvedToken.hClass, &elemClsHnd); - var_types elemType = JITtype2varType(elemCorType); - if (elemType == TYP_STRUCT) + // Each query gets its own trap so that a failure of the first, which is + // the one an AOT compiler rejects for an out-of-bubble type, does not + // suppress the rest. + // + eeRunExtraSuperPmiQueries([&]() { + void* pEmbedClsHnd; + info.compCompHnd->embedClassHandle(resolvedToken.hClass, &pEmbedClsHnd); + }); + + CORINFO_CLASS_HANDLE elemClsHnd = NO_CLASS_HANDLE; + CorInfoType elemCorType = CORINFO_TYPE_UNDEF; + eeRunExtraSuperPmiQueries([&]() { + elemCorType = info.compCompHnd->getChildType(resolvedToken.hClass, &elemClsHnd); + }); + + if ((JITtype2varType(elemCorType) == TYP_STRUCT) && (elemClsHnd != NO_CLASS_HANDLE)) { + // JIT work, so deliberately not trapped. It can set compFloatingPointUsed + // via ClassLayout::Create -> impNormStructType, which would let the queries + // change codegen, so restore that. Note this cannot fully undo the layout + // being memoized, only the flag. + // + const bool savedFloatingPointUsed = compFloatingPointUsed; typGetObjLayout(elemClsHnd); - info.compCompHnd->isValueClass(elemClsHnd); + compFloatingPointUsed = savedFloatingPointUsed; + + eeRunExtraSuperPmiQueries([&]() { + info.compCompHnd->isValueClass(elemClsHnd); + }); } - compGetHelperFtn(CORINFO_HELP_MEMZERO); + + eeRunExtraSuperPmiQueries([&]() { + // Deliberately not compGetHelperFtn, whose assert would be absorbed here. + if (info.compMatchedVM) + { + CORINFO_CONST_LOOKUP lookup; + info.compCompHnd->getHelperFtn(CORINFO_HELP_MEMZERO, &lookup); + } + }); } #endif } diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 81d931b0816ad0..23b89aab749404 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -7955,14 +7955,16 @@ void Compiler::impSetupAsyncCall(GenTreeCall* call, #ifdef DEBUG if (JitConfig.EnableExtraSuperPmiQueries() && (call->gtCallType == CT_USER_FUNC)) { - // Query the async variants (twice, to get both directions) - CORINFO_METHOD_HANDLE method = call->gtCallMethHnd; - bool variantIsThunk; - method = info.compCompHnd->getAsyncOtherVariant(method, &variantIsThunk); - if (method != NO_METHOD_HANDLE) - { + eeRunExtraSuperPmiQueries([&]() { + // Query the async variants (twice, to get both directions) + CORINFO_METHOD_HANDLE method = call->gtCallMethHnd; + bool variantIsThunk; method = info.compCompHnd->getAsyncOtherVariant(method, &variantIsThunk); - } + if (method != NO_METHOD_HANDLE) + { + method = info.compCompHnd->getAsyncOtherVariant(method, &variantIsThunk); + } + }); } #endif } @@ -8317,8 +8319,12 @@ void Compiler::pickGDV(GenTreeCall* call, for (UINT32 i = 0; i < numberOfMethods; i++) { CORINFO_CONST_LOOKUP lookup = {}; - info.compCompHnd->getFunctionFixedEntryPoint((CORINFO_METHOD_HANDLE)likelyMethods[i].handle, false, - &lookup); + // This query is made only to dump the entry point, so it must not be able to fail + // the compilation: an AOT compiler throws for a method it cannot create a fixup for. + eeRunExtraSuperPmiQueries([&]() { + info.compCompHnd->getFunctionFixedEntryPoint((CORINFO_METHOD_HANDLE)likelyMethods[i].handle, false, + &lookup); + }); const char* methName = eeGetMethodFullName((CORINFO_METHOD_HANDLE)likelyMethods[i].handle); switch (lookup.accessType) diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 9e2dbbe81b3bda..87a2f95eba7cc5 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -996,7 +996,9 @@ void Compiler::lvaClassifyParameterABI(Classifier& classifier) CORINFO_CLASS_HANDLE clsHnd = structLayout->GetClassHandle(); if (clsHnd != NO_CLASS_HANDLE) { - info.compCompHnd->getWasmLowering(clsHnd); + eeRunExtraSuperPmiQueries([&]() { + info.compCompHnd->getWasmLowering(clsHnd); + }); } } #endif // DEBUG @@ -2608,7 +2610,13 @@ void Compiler::lvaSetStruct(unsigned varNum, ClassLayout* layout, bool unsafeVal #ifdef DEBUG if (JitConfig.EnableExtraSuperPmiQueries()) { + // makeExtraStructQueries runs real JIT work, so it is not trapped here. It can also set + // compFloatingPointUsed, via impNormStructType, GetHfaType, and ClassLayout::Create, + // which would let the queries change codegen, so restore that. + // + const bool savedFloatingPointUsed = compFloatingPointUsed; makeExtraStructQueries(layout->GetClassHandle(), 2); + compFloatingPointUsed = savedFloatingPointUsed; } #endif // DEBUG } @@ -2657,7 +2665,8 @@ void Compiler::makeExtraStructQueries(CORINFO_CLASS_HANDLE structHandle, int lev size_t numNodes = ArrLen(nodes); info.compCompHnd->getTypeLayout(structHandle, nodes, &numNodes); }; - queryLayout(); + // Trapped because an AOT compiler rejects this query for an out-of-bubble type. + eeRunExtraSuperPmiQueries(queryLayout); // Bypass fetching instance fields of ref classes for now, // as it requires traversing the class hierarchy. From a5ea966a138f2bfbd79eb3f9f88976b55c2fb879 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 31 Aug 2026 09:39:47 -0700 Subject: [PATCH 2/2] Address review feedback Drop the outer SuperPMI error trap. The extra queries only run when the JIT config is set, and that config is read once at jitStartup rather than per method context, so it is never recorded into the MCH and a replay never has it on. superpmi.py sets it only on the collect path. One trap, around the EE call, is enough. Compare elemCorType against CORINFO_TYPE_VALUECLASS instead of converting it. If the getChildType query is trapped, elemCorType is left CORINFO_TYPE_UNDEF, and JITtype2varType asserts on that -- so the extra-query path could still fail the compilation it is supposed to leave alone. CORINFO_TYPE_VALUECLASS is the only type that maps to TYP_STRUCT, so the test is equivalent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c6bbfae-eed7-4545-8f2a-5cfb15d62df4 --- src/coreclr/jit/compiler.h | 14 ++++---------- src/coreclr/jit/importer.cpp | 6 +++++- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 91284ff441a05c..02ce59decd5af7 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -9862,20 +9862,14 @@ class Compiler // would then import less IL than a later replay does, and the context it recorded // would be missing the data that replay goes on to ask for. // - // The inner trap absorbs such an EE failure during collection. The outer one absorbs - // SuperPMI's own "missing data" failure, for a replay that re-enables the queries - // against a context collected without them. - // - // Wrap only EE queries. JIT work must stay outside, because neither trap discriminates - // by origin: a noway_assert, NOMEM or assert raised inside the functor would be quietly - // absorbed instead of failing the method. + // Wrap only EE queries. JIT work must stay outside, because the trap does not + // discriminate by origin: a noway_assert, NOMEM or assert raised inside the functor + // would be quietly absorbed instead of failing the method. // template void eeRunExtraSuperPmiQueries(Functor f) { - eeRunFunctorWithSPMIErrorTrap([&]() { - eeRunFunctorWithErrorTrap(f); - }); + eeRunFunctorWithErrorTrap(f); } #endif // DEBUG diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 560dec0add32e8..1722594c211011 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -10275,7 +10275,11 @@ void Compiler::impImportBlockCode(BasicBlock* block) elemCorType = info.compCompHnd->getChildType(resolvedToken.hClass, &elemClsHnd); }); - if ((JITtype2varType(elemCorType) == TYP_STRUCT) && (elemClsHnd != NO_CLASS_HANDLE)) + // CORINFO_TYPE_VALUECLASS is the only type JITtype2varType maps to + // TYP_STRUCT. Test it directly, since JITtype2varType asserts if the + // query above was trapped and left elemCorType as CORINFO_TYPE_UNDEF. + // + if ((elemCorType == CORINFO_TYPE_VALUECLASS) && (elemClsHnd != NO_CLASS_HANDLE)) { // JIT work, so deliberately not trapped. It can set compFloatingPointUsed // via ClassLayout::Create -> impNormStructType, which would let the queries