From 35ce665c1af4d2800a03ca5ba47e51ffb6d9cf3b Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 17 Jun 2025 15:31:56 -0700 Subject: [PATCH 01/11] JIT: extend indirect virtual stub optimization to arrays Add support for array interface methods invoked via indirect virtual stubs. Trim out a layer of temps in the importer. Remember the base method handle and use that for detecting the GetEnumerator pattern and for array interface method devirtualization. Mark the SZGenericArrayEnumerator ctor as aggressively inlined; when inlined we can often prove the enumerator (conditionally) doesn't escape. Part of #108913 --- src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/importer.cpp | 5 +++- src/coreclr/jit/importercalls.cpp | 23 ++++++++++++------- src/coreclr/jit/indirectcalltransformer.cpp | 2 +- src/coreclr/jit/inline.h | 3 ++- src/coreclr/vm/jitinterface.cpp | 17 +++++++------- .../src/System/Array.Enumerators.cs | 1 + 7 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 6dd1c74b35ffb4..df868edfe303e7 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7562,6 +7562,7 @@ class Compiler unsigned likelihood, bool arrayInterface, bool instantiatingStub, + CORINFO_METHOD_HANDLE originalMethodHandle, CORINFO_CONTEXT_HANDLE originalContextHandle); int getGDVMaxTypeChecks() diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 660ddae95cc46f..05f2bb491913f8 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -6947,7 +6947,10 @@ void Compiler::impImportBlockCode(BasicBlock* block) } else if (call->IsGuardedDevirtualizationCandidate()) { - JITDUMP("No GDV IEnumerable check for [%06u]\n", dspTreeID(call)); + // Just check one of the GDV candidates (all should have the same original method handle) + // + InlineCandidateInfo* const info = call->GetGDVCandidateInfo(0); + ni = lookupNamedIntrinsic(info->originalMethodHandle); } if (ni == NI_System_Collections_Generic_IEnumerable_GetEnumerator) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 3b065e4f40af58..8e9163ceddb3c5 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -310,14 +310,17 @@ var_types Compiler::impImportCall(OPCODE opcode, // complex expression. As it is evaluated after the args, // it may cause registered args to be spilled. Simply spill it. // - unsigned const lclNum = lvaGrabTemp(true DEBUGARG("VirtualCall with runtime lookup")); - if (compDonotInline()) + if (!stubAddr->OperIs(GT_LCL_VAR)) { - return TYP_UNDEF; - } + unsigned const lclNum = lvaGrabTemp(true DEBUGARG("VirtualCall with runtime lookup")); + if (compDonotInline()) + { + return TYP_UNDEF; + } - impStoreToTemp(lclNum, stubAddr, CHECK_SPILL_NONE); - stubAddr = gtNewLclvNode(lclNum, TYP_I_IMPL); + impStoreToTemp(lclNum, stubAddr, CHECK_SPILL_NONE); + stubAddr = gtNewLclvNode(lclNum, TYP_I_IMPL); + } // Create the actual call node @@ -7212,7 +7215,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, addGuardedDevirtualizationCandidate(call, exactMethod, exactCls, exactContext, exactMethodAttrs, clsAttrs, likelyHood, dvInfo.wasArrayInterfaceDevirt, - dvInfo.isInstantiatingStub, originalContext); + dvInfo.isInstantiatingStub, baseMethod, originalContext); } if (call->GetInlineCandidatesCount() == numExactClasses) @@ -7361,7 +7364,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, // addGuardedDevirtualizationCandidate(call, likelyMethod, likelyClass, likelyContext, likelyMethodAttribs, likelyClassAttribs, likelihood, arrayInterface, instantiatingStub, - originalContext); + baseMethod, originalContext); } } @@ -7388,6 +7391,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, // likelihood - odds that this class is the class seen at runtime // arrayInterface - devirtualization of an array interface call // instantiatingStub - devirtualized method in an instantiating stub +// originalMethodHandle - method handle of base method (before devirt) // originalContextHandle - context for the original call // void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, @@ -7399,6 +7403,7 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, unsigned likelihood, bool arrayInterface, bool instantiatingStub, + CORINFO_METHOD_HANDLE originalMethodHandle, CORINFO_CONTEXT_HANDLE originalContextHandle) { // This transformation only makes sense for delegate and virtual calls @@ -7471,6 +7476,7 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call, pInfo->guardedMethodUnboxedEntryHandle = nullptr; pInfo->guardedMethodInstantiatedEntryHandle = nullptr; pInfo->guardedClassHandle = classHandle; + pInfo->originalMethodHandle = originalMethodHandle; pInfo->originalContextHandle = originalContextHandle; pInfo->likelihood = likelihood; pInfo->exactContextHandle = contextHandle; @@ -9371,6 +9377,7 @@ void Compiler::impCheckCanInline(GenTreeCall* call, pInfo->guardedMethodHandle = nullptr; pInfo->guardedMethodUnboxedEntryHandle = nullptr; pInfo->guardedMethodInstantiatedEntryHandle = nullptr; + pInfo->originalMethodHandle = nullptr; pInfo->originalContextHandle = nullptr; pInfo->likelihood = 0; pInfo->arrayInterface = false; diff --git a/src/coreclr/jit/indirectcalltransformer.cpp b/src/coreclr/jit/indirectcalltransformer.cpp index 48c66692a42e8c..cfa2a7518074ca 100644 --- a/src/coreclr/jit/indirectcalltransformer.cpp +++ b/src/coreclr/jit/indirectcalltransformer.cpp @@ -973,7 +973,7 @@ class IndirectCallTransformer // if (inlineInfo->arrayInterface) { - methodHnd = call->gtCallMethHnd; + methodHnd = inlineInfo->originalMethodHandle; context = inlineInfo->originalContextHandle; } diff --git a/src/coreclr/jit/inline.h b/src/coreclr/jit/inline.h index 6d869ab3bb8008..a89be81fa68895 100644 --- a/src/coreclr/jit/inline.h +++ b/src/coreclr/jit/inline.h @@ -610,9 +610,10 @@ struct InlineCandidateInfo : public HandleHistogramProfileCandidateInfo // CORINFO_CONTEXT_HANDLE exactContextHandle; - // Context handle of the call before any + // Method and context handle of the call before any // GDV/Inlining evaluation // + CORINFO_METHOD_HANDLE originalMethodHandle; CORINFO_CONTEXT_HANDLE originalContextHandle; // The GT_RET_EXPR node linking back to the inline candidate. diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 351cb5246864ef..9f29b6a2b3cc85 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8621,18 +8621,15 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // if (pObjMT->IsArray()) { - // If we're in a shared context we'll devirt to a shared - // generic method and won't be able to inline, so just bail. + // See if this interface is one of the ones arrays implement. // - if (pBaseMT->IsSharedByGenericInstantiations()) + if (!pBaseMT->HasInstantiation()) { - info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CANON; + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CAST; return false; } - // Ensure we can cast the array to the interface type - // - if (!TypeHandle(pObjMT).CanCastTo(TypeHandle(pBaseMT))) + if (!IsImplicitInterfaceOfSZArray(pBaseMT)) { info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CAST; return false; @@ -8650,7 +8647,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) { MethodTable* interfaceMT = nullptr; - if (pObjMT->IsSharedByGenericInstantiations() || pBaseMT->IsSharedByGenericInstantiations()) + if (!pObjMT->IsArray() && (pObjMT->IsSharedByGenericInstantiations() || pBaseMT->IsSharedByGenericInstantiations())) { MethodTable* pCanonBaseMT = pBaseMT->GetCanonicalMethodTable(); @@ -8687,6 +8684,10 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) pDevirtMD = pObjMT->GetMethodDescForInterfaceMethod(TypeHandle(interfaceMT), pBaseMD, FALSE /* throwOnConflict */); } + else if (pObjMT->IsArray()) + { + pDevirtMD = GetActualImplementationForArrayGenericIListOrIReadOnlyListMethod(pBaseMD, pObjMT->GetArrayElementTypeHandle()); + } else { pDevirtMD = pObjMT->GetMethodDescForInterfaceMethod(TypeHandle(pBaseMT), pBaseMD, FALSE /* throwOnConflict */); diff --git a/src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs b/src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs index c2bbd78c474f2d..0acdb27afa622b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Array.Enumerators.cs @@ -110,6 +110,7 @@ internal sealed class SZGenericArrayEnumerator : SZGenericArrayEnumeratorBase /// internal static readonly SZGenericArrayEnumerator Empty = new SZGenericArrayEnumerator(null, 0); + [MethodImpl(MethodImplOptions.AggressiveInlining)] internal SZGenericArrayEnumerator(T[]? array, int endIndex) : base(endIndex) { From 1643f33cddc3eb62a451f380d14bc88e53702395 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 18 Jun 2025 15:10:59 -0700 Subject: [PATCH 02/11] no array interface devirt unless array type is exact --- src/coreclr/jit/fginline.cpp | 5 +++++ src/coreclr/jit/importercalls.cpp | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index dce176b08b345a..bca50904cc9d85 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -546,6 +546,8 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitordspTreeID(call)); + assert(call->IsDevirtualizationCandidate(m_compiler)); if (call->IsVirtual()) { @@ -556,6 +558,9 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorgtCallAddr->AsCall()->gtArgs.FindWellKnownArg(WellKnownArg::RuntimeMethodHandle)->GetNode(); assert(runtimeMethHndNode != nullptr); + + JITDUMP(" ... via node [%06u]\n", m_compiler->dspTreeID(runtimeMethHndNode)); + switch (runtimeMethHndNode->OperGet()) { case GT_RUNTIMELOOKUP: diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 8e9163ceddb3c5..de64ef45393bc2 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -8405,6 +8405,15 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, return; } + // If we don't know the array type exactly we may have the wrong interface type here. + // Bail out. + // + if (!isExact) + { + JITDUMP("Array interface devirt: array type is inexact, sorry.\n"); + return; + } + // We want to inline the instantiating stub. Fetch the relevant info. // CORINFO_CLASS_HANDLE ignored = NO_CLASS_HANDLE; From eb2c4a018ab27ce4a823d6ee69d8abab81579323 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 19 Jun 2025 15:15:03 -0700 Subject: [PATCH 03/11] restrict array cases to ones where element type is not shared --- src/coreclr/vm/jitinterface.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 9f29b6a2b3cc85..e034ece5264f58 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8686,7 +8686,15 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) } else if (pObjMT->IsArray()) { - pDevirtMD = GetActualImplementationForArrayGenericIListOrIReadOnlyListMethod(pBaseMD, pObjMT->GetArrayElementTypeHandle()); + // We cannot devirtualize unless we know the exact array element type + // + TypeHandle elemType = pObjMT->GetArrayElementTypeHandle(); + if (elemType.IsCanonicalSubtype()) + { + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_LOOKUP; + return false; + } + pDevirtMD = GetActualImplementationForArrayGenericIListOrIReadOnlyListMethod(pBaseMD, elemType); } else { From 63edced32a4ba8d61421ac29180014954acd58e8 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 20 Jun 2025 13:10:25 -0700 Subject: [PATCH 04/11] don't impair devirt for the explicit array interfaces --- src/coreclr/vm/jitinterface.cpp | 68 +++++++++++++++++---------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index e034ece5264f58..f51ed00ab16a28 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8597,6 +8597,8 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) return false; } + bool isArrayImplicitInterface = false; + if (pBaseMT->IsInterface()) { @@ -8621,18 +8623,25 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // if (pObjMT->IsArray()) { - // See if this interface is one of the ones arrays implement. + // Does the array explicitly implements this interface? // - if (!pBaseMT->HasInstantiation()) + if (!TypeHandle(pObjMT).CanCastTo(TypeHandle(pBaseMT))) { - info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CAST; - return false; - } + // Does the array implicitly implement this interface? + // + if (!pBaseMT->HasInstantiation()) + { + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CAST; + return false; + } - if (!IsImplicitInterfaceOfSZArray(pBaseMT)) - { - info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CAST; - return false; + if (!IsImplicitInterfaceOfSZArray(pBaseMT)) + { + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CAST; + return false; + } + + isArrayImplicitInterface = true; } } else if (!pBaseMT->IsSharedByGenericInstantiations() && !pObjMT->CanCastToInterface(pBaseMT)) @@ -8647,7 +8656,21 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) { MethodTable* interfaceMT = nullptr; - if (!pObjMT->IsArray() && (pObjMT->IsSharedByGenericInstantiations() || pBaseMT->IsSharedByGenericInstantiations())) + if (isArrayImplicitInterface) + { + _ASSERTE(!pObjMT->IsArray()); + + // We cannot devirtualize unless we know the exact array element type + // + TypeHandle elemType = pObjMT->GetArrayElementTypeHandle(); + if (elemType.IsCanonicalSubtype()) + { + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_LOOKUP; + return false; + } + pDevirtMD = GetActualImplementationForArrayGenericIListOrIReadOnlyListMethod(pBaseMD, elemType); + } + else if ((pObjMT->IsSharedByGenericInstantiations() || pBaseMT->IsSharedByGenericInstantiations())) { MethodTable* pCanonBaseMT = pBaseMT->GetCanonicalMethodTable(); @@ -8684,18 +8707,6 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) pDevirtMD = pObjMT->GetMethodDescForInterfaceMethod(TypeHandle(interfaceMT), pBaseMD, FALSE /* throwOnConflict */); } - else if (pObjMT->IsArray()) - { - // We cannot devirtualize unless we know the exact array element type - // - TypeHandle elemType = pObjMT->GetArrayElementTypeHandle(); - if (elemType.IsCanonicalSubtype()) - { - info->detail = CORINFO_DEVIRTUALIZATION_FAILED_LOOKUP; - return false; - } - pDevirtMD = GetActualImplementationForArrayGenericIListOrIReadOnlyListMethod(pBaseMD, elemType); - } else { pDevirtMD = pObjMT->GetMethodDescForInterfaceMethod(TypeHandle(pBaseMT), pBaseMD, FALSE /* throwOnConflict */); @@ -8779,27 +8790,17 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // MethodTable* pApproxMT = pDevirtMD->GetMethodTable(); MethodTable* pExactMT = pApproxMT; - bool isArray = false; if (pApproxMT->IsInterface()) { // As noted above, we can't yet handle generic interfaces // with default methods. _ASSERTE(!pDevirtMD->HasClassInstantiation()); - - } - else if (pBaseMT->IsInterface() && pObjMT->IsArray()) - { - isArray = true; - } - else - { - pExactMT = pDevirtMD->GetExactDeclaringType(pObjMT); } // Success! Pass back the results. // - if (isArray) + if (isArrayImplicitInterface) { // Note if array devirtualization produced an instantiation stub // so jit can try and inline it. @@ -8810,6 +8811,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) } else { + pExactMT = pDevirtMD->GetExactDeclaringType(pObjMT); info->exactContext = MAKE_CLASSCONTEXT((CORINFO_CLASS_HANDLE) pExactMT); info->isInstantiatingStub = false; info->wasArrayInterfaceDevirt = false; From 5f720c5cdcc640dd217bc72a6886f8230e647a57 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 20 Jun 2025 14:52:26 -0700 Subject: [PATCH 05/11] fix --- src/coreclr/vm/jitinterface.cpp | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index f51ed00ab16a28..0867a524140f83 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8623,25 +8623,17 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // if (pObjMT->IsArray()) { - // Does the array explicitly implements this interface? + // Does the array implicitly implement this interface? // - if (!TypeHandle(pObjMT).CanCastTo(TypeHandle(pBaseMT))) - { - // Does the array implicitly implement this interface? - // - if (!pBaseMT->HasInstantiation()) - { - info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CAST; - return false; - } + isArrayImplicitInterface = pBaseMT->HasInstantiation() && IsImplicitInterfaceOfSZArray(pBaseMT); - if (!IsImplicitInterfaceOfSZArray(pBaseMT)) - { - info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CAST; - return false; - } - - isArrayImplicitInterface = true; + // If we can't cast the array to the interface, proceed + // only if this is an implicit implementation. + // + if (!TypeHandle(pObjMT).CanCastTo(TypeHandle(pBaseMT)) && !isArrayImplicitInterface) + { + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CAST; + return false; } } else if (!pBaseMT->IsSharedByGenericInstantiations() && !pObjMT->CanCastToInterface(pBaseMT)) @@ -8658,7 +8650,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) if (isArrayImplicitInterface) { - _ASSERTE(!pObjMT->IsArray()); + _ASSERTE(pObjMT->IsArray()); // We cannot devirtualize unless we know the exact array element type // From f5ba688eaec5169fca2c89f31ecae2e9fc2e087f Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 20 Jun 2025 18:04:56 -0700 Subject: [PATCH 06/11] fix --- src/coreclr/vm/jitinterface.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 0867a524140f83..be34a1635ec5cb 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8598,6 +8598,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) } bool isArrayImplicitInterface = false; + bool isArrayInterface = false; if (pBaseMT->IsInterface()) { @@ -8623,6 +8624,8 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // if (pObjMT->IsArray()) { + bool isArrayInterface = true; + // Does the array implicitly implement this interface? // isArrayImplicitInterface = pBaseMT->HasInstantiation() && IsImplicitInterfaceOfSZArray(pBaseMT); @@ -8789,10 +8792,14 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // with default methods. _ASSERTE(!pDevirtMD->HasClassInstantiation()); } + else if (!isArrayInterface) + { + pExactMT = pDevirtMD->GetExactDeclaringType(pObjMT); + } // Success! Pass back the results. // - if (isArrayImplicitInterface) + if (isArrayInterface) { // Note if array devirtualization produced an instantiation stub // so jit can try and inline it. @@ -8803,7 +8810,6 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) } else { - pExactMT = pDevirtMD->GetExactDeclaringType(pObjMT); info->exactContext = MAKE_CLASSCONTEXT((CORINFO_CLASS_HANDLE) pExactMT); info->isInstantiatingStub = false; info->wasArrayInterfaceDevirt = false; From 6b8643585219fd7c91f18f9209fe2bb5c5d670b4 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 26 Jun 2025 11:52:50 -0700 Subject: [PATCH 07/11] minimize diffs with main --- src/coreclr/vm/jitinterface.cpp | 35 ++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index be34a1635ec5cb..85ec1cd7d0f288 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8597,9 +8597,6 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) return false; } - bool isArrayImplicitInterface = false; - bool isArrayInterface = false; - if (pBaseMT->IsInterface()) { @@ -8622,6 +8619,8 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // We must ensure that pObjMT actually implements the // interface corresponding to pBaseMD. // + bool isArrayImplicitInterface = false; + if (pObjMT->IsArray()) { bool isArrayInterface = true; @@ -8630,13 +8629,21 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // isArrayImplicitInterface = pBaseMT->HasInstantiation() && IsImplicitInterfaceOfSZArray(pBaseMT); - // If we can't cast the array to the interface, proceed - // only if this is an implicit implementation. - // - if (!TypeHandle(pObjMT).CanCastTo(TypeHandle(pBaseMT)) && !isArrayImplicitInterface) + if (!isArrayImplicitInterface) { - info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CAST; - return false; + if (pBaseMT->IsSharedByGenericInstantiations()) + { + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CANON; + return false; + } + + // Ensure we can cast the array to the interface type + // + if (!TypeHandle(pObjMT).CanCastTo(TypeHandle(pBaseMT))) + { + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CAST; + return false; + } } } else if (!pBaseMT->IsSharedByGenericInstantiations() && !pObjMT->CanCastToInterface(pBaseMT)) @@ -8785,21 +8792,27 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) // MethodTable* pApproxMT = pDevirtMD->GetMethodTable(); MethodTable* pExactMT = pApproxMT; + bool isArray = false; if (pApproxMT->IsInterface()) { // As noted above, we can't yet handle generic interfaces // with default methods. _ASSERTE(!pDevirtMD->HasClassInstantiation()); + + } + else if (pBaseMT->IsInterface() && pObjMT->IsArray()) + { + isArray = true; } - else if (!isArrayInterface) + else { pExactMT = pDevirtMD->GetExactDeclaringType(pObjMT); } // Success! Pass back the results. // - if (isArrayInterface) + if (isArray) { // Note if array devirtualization produced an instantiation stub // so jit can try and inline it. From e9085502e6b0243dc691b47be00e0de4aa15c94a Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 26 Jun 2025 11:56:00 -0700 Subject: [PATCH 08/11] remove unneeded jit dump --- src/coreclr/jit/fginline.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 110145d21ca81b..1439d544c680dd 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -546,8 +546,6 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitordspTreeID(call)); - assert(call->IsDevirtualizationCandidate(m_compiler)); if (call->IsVirtual()) { @@ -558,9 +556,6 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorgtCallAddr->AsCall()->gtArgs.FindWellKnownArg(WellKnownArg::RuntimeMethodHandle)->GetNode(); assert(runtimeMethHndNode != nullptr); - - JITDUMP(" ... via node [%06u]\n", m_compiler->dspTreeID(runtimeMethHndNode)); - switch (runtimeMethHndNode->OperGet()) { case GT_RUNTIMELOOKUP: From f258b9bd3bac202a0ebaafa4c5c34fb38606de16 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 27 Jun 2025 10:30:32 -0700 Subject: [PATCH 09/11] Fix merge conflict --- src/coreclr/vm/jitinterface.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index e0108e8240f5b5..52d0acd39cf4cd 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8613,8 +8613,6 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) if (pObjMT->IsArray()) { - bool isArrayInterface = true; - // Does the array implicitly implement this interface? // isArrayImplicitInterface = pBaseMT->HasInstantiation() && IsImplicitInterfaceOfSZArray(pBaseMT); From 876513a89b182f32c82544c0f838f855f98a410a Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 27 Jun 2025 10:26:34 -0700 Subject: [PATCH 10/11] still need custom flagging for indir cases --- src/coreclr/jit/importer.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index be1873657fa8dc..dbb524e4d89d5d 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -6941,6 +6941,16 @@ void Compiler::impImportBlockCode(BasicBlock* block) bool isNonNull = false; CORINFO_CLASS_HANDLE retCls = gtGetClassHandle(call, &isExact, &isNonNull); + if ((retCls == NO_CLASS_HANDLE) && call->IsGuardedDevirtualizationCandidate()) + { + // Just check one of the GDV candidates (all should have the same original method handle) + // + InlineCandidateInfo* const inlineInfo = call->GetGDVCandidateInfo(0); + CORINFO_SIG_INFO sig; + info.compCompHnd->getMethodSig(inlineInfo->originalMethodHandle, &sig); + retCls = sig.retTypeClass; + } + if ((retCls != NO_CLASS_HANDLE) && info.compCompHnd->isIntrinsicType(retCls)) { const char* namespaceName; From 68d55c8a703c8ca85eb8a7897bf020347ccf53e9 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 27 Jun 2025 13:48:49 -0700 Subject: [PATCH 11/11] Update src/coreclr/vm/jitinterface.cpp Co-authored-by: Aman Khalid --- src/coreclr/vm/jitinterface.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 52d0acd39cf4cd..d3c9285468b28e 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8660,7 +8660,7 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) } pDevirtMD = GetActualImplementationForArrayGenericIListOrIReadOnlyListMethod(pBaseMD, elemType); } - else if ((pObjMT->IsSharedByGenericInstantiations() || pBaseMT->IsSharedByGenericInstantiations())) + else if (pObjMT->IsSharedByGenericInstantiations() || pBaseMT->IsSharedByGenericInstantiations()) { MethodTable* pCanonBaseMT = pBaseMT->GetCanonicalMethodTable();