diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 15161eacd678d8..9ec22c22305585 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8096,7 +8096,8 @@ class Compiler bool isInterface, CORINFO_METHOD_HANDLE baseMethod, CORINFO_CLASS_HANDLE baseClass, - CORINFO_CONTEXT_HANDLE* pContextHandle); + CORINFO_CONTEXT_HANDLE* pContextHandle, + CORINFO_RESOLVED_TOKEN* pResolvedToken); bool isCompatibleMethodGDV(GenTreeCall* call, CORINFO_METHOD_HANDLE gdvTarget); diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 089f8a4dab62d8..adc24cbf8d7bf0 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -608,16 +608,17 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitorgtLateDevirtualizationInfo->methodHnd; - CORINFO_CONTEXT_HANDLE context = call->gtLateDevirtualizationInfo->exactContextHnd; - InlineContext* inlinersContext = call->gtInlineContext; - unsigned methodFlags = 0; - const bool isLateDevirtualization = true; - const bool explicitTailCall = call->IsTailPrefixedCall(); + CORINFO_METHOD_HANDLE method = call->gtLateDevirtualizationInfo->methodHnd; + CORINFO_CONTEXT_HANDLE context = call->gtLateDevirtualizationInfo->exactContextHnd; + InlineContext* inlinersContext = call->gtInlineContext; + CORINFO_RESOLVED_TOKEN* pResolvedToken = &call->gtLateDevirtualizationInfo->resolvedToken; + unsigned methodFlags = 0; + const bool isLateDevirtualization = true; + const bool explicitTailCall = call->IsTailPrefixedCall(); CORINFO_CONTEXT_HANDLE contextInput = context; context = nullptr; - m_compiler->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &contextInput, &context, + m_compiler->impDevirtualizeCall(call, pResolvedToken, &method, &methodFlags, &contextInput, &context, isLateDevirtualization, explicitTailCall); if (!call->IsDevirtualizationCandidate(m_compiler)) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 4fd9106f7d1291..65170b5307e77f 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1021,7 +1021,7 @@ var_types Compiler::impImportCall(OPCODE opcode, else if (call->AsCall()->IsDelegateInvoke()) { considerGuardedDevirtualization(call->AsCall(), rawILOffset, false, call->AsCall()->gtCallMethHnd, - NO_CLASS_HANDLE, nullptr); + NO_CLASS_HANDLE, nullptr, pResolvedToken); } } @@ -1291,6 +1291,7 @@ var_types Compiler::impImportCall(OPCODE opcode, info->methodHnd = callInfo->hMethod; info->exactContextHnd = exactContextHnd; info->ilLocation = impCurStmtDI.GetLocation(); + info->resolvedToken = *pResolvedToken; call->AsCall()->gtLateDevirtualizationInfo = info; } } @@ -7865,7 +7866,8 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, bool isInterface, CORINFO_METHOD_HANDLE baseMethod, CORINFO_CLASS_HANDLE baseClass, - CORINFO_CONTEXT_HANDLE* pContextHandle) + CORINFO_CONTEXT_HANDLE* pContextHandle, + CORINFO_RESOLVED_TOKEN* pResolvedToken) { JITDUMP("Considering guarded devirtualization at IL offset %u (0x%x)\n", ilOffset, ilOffset); @@ -7946,7 +7948,7 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call, dvInfo.virtualMethod = baseMethod; dvInfo.objClass = exactCls; dvInfo.context = originalContext; - dvInfo.pResolvedTokenVirtualMethod = nullptr; + dvInfo.pResolvedTokenVirtualMethod = pResolvedToken; JITDUMP("GDV exact: resolveVirtualMethod (method %p class %p context %p)\n", dvInfo.virtualMethod, dvInfo.objClass, dvInfo.context); @@ -9088,7 +9090,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, return; } - considerGuardedDevirtualization(call, ilOffset, isInterface, baseMethod, baseClass, pContextHandle); + considerGuardedDevirtualization(call, ilOffset, isInterface, baseMethod, baseClass, pContextHandle, + pResolvedToken); return; } @@ -9132,7 +9135,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, return; } - considerGuardedDevirtualization(call, ilOffset, isInterface, baseMethod, baseClass, pContextHandle); + considerGuardedDevirtualization(call, ilOffset, isInterface, baseMethod, baseClass, pContextHandle, + pResolvedToken); return; } @@ -9254,7 +9258,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call, return; } - considerGuardedDevirtualization(call, ilOffset, isInterface, baseMethod, objClass, pContextHandle); + considerGuardedDevirtualization(call, ilOffset, isInterface, baseMethod, objClass, pContextHandle, + pResolvedToken); return; } diff --git a/src/coreclr/jit/inline.h b/src/coreclr/jit/inline.h index 065f7de3eaaa20..6de8551d249f82 100644 --- a/src/coreclr/jit/inline.h +++ b/src/coreclr/jit/inline.h @@ -635,6 +635,7 @@ struct LateDevirtualizationInfo CORINFO_METHOD_HANDLE methodHnd; CORINFO_CONTEXT_HANDLE exactContextHnd; ILLocation ilLocation; + CORINFO_RESOLVED_TOKEN resolvedToken; }; // InlArgInfo describes inline candidate argument properties. diff --git a/src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs b/src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs index ac544ed02e3fea..9e23da8fb606b3 100644 --- a/src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs +++ b/src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs @@ -148,19 +148,19 @@ protected virtual MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefType return null; case DefaultInterfaceMethodResolution.DefaultImplementation: - if (dimMethod.OwningType.HasInstantiation || (declMethod != defaultInterfaceDispatchDeclMethod)) + if (declMethod != defaultInterfaceDispatchDeclMethod) { - // If we devirtualized into a default interface method on a generic type, we should actually return an - // instantiating stub but this is not happening. - // Making this work is tracked by https://github.com/dotnet/runtime/issues/9588 - - // In addition, we fail here for variant default interface dispatch + // Fail for variant default interface dispatch devirtualizationDetail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_DIM; return null; } else { impl = dimMethod; + if (originalDeclMethod.HasInstantiation) + { + impl = impl.GetMethodDefinition().MakeInstantiatedMethod(originalDeclMethod.Instantiation); + } } break; } @@ -219,13 +219,6 @@ protected virtual MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefType } } - if (impl != null && impl.HasInstantiation && impl.GetCanonMethodTarget(CanonicalFormKind.Specific).IsCanonicalMethod(CanonicalFormKind.Specific)) - { - // We don't support devirtualization of shared generic virtual methods yet. - devirtualizationDetail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON; - impl = null; - } - return impl; } diff --git a/src/coreclr/tools/Common/Compiler/GenericCycleDetection/GraphBuilder.ForEach.cs b/src/coreclr/tools/Common/Compiler/GenericCycleDetection/GraphBuilder.ForEach.cs index 6224974cd05155..e0b946b9341249 100644 --- a/src/coreclr/tools/Common/Compiler/GenericCycleDetection/GraphBuilder.ForEach.cs +++ b/src/coreclr/tools/Common/Compiler/GenericCycleDetection/GraphBuilder.ForEach.cs @@ -59,7 +59,11 @@ private static void ForEachEmbeddedGenericFormalWorker(TypeDesc type, Instantiat TypeDesc genericTypeDefinition = type.GetTypeDefinition(); Instantiation genericTypeParameters = genericTypeDefinition.Instantiation; Instantiation genericTypeArguments = type.Instantiation; - for (int i = 0; i < genericTypeArguments.Length; i++) + + // We have some negative IL tests that reference a generic type with more type arguments + // than the type definition has type parameters. + // Tolerate them as such a type can never load and therefore cannot form a cycle. + for (int i = 0; i < genericTypeArguments.Length && i < genericTypeParameters.Length; i++) { var genericTypeParameter = (EcmaGenericParameter)genericTypeParameters[i]; TypeDesc genericTypeArgument = genericTypeArguments[i]; diff --git a/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs b/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs index f70b278f2de695..99ec54082ac19a 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs @@ -124,6 +124,7 @@ public enum DictionaryEntryKind DispatchStubAddrSlot = 5, FieldDescSlot = 6, DeclaringTypeHandleSlot = 7, + DevirtualizedMethodDescSlot = 8, } public enum ReadyToRunFixupKind diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index d21377ed7d80da..1fd5881992b305 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -1483,6 +1483,90 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info) info->resolvedTokenDevirtualizedUnboxedMethod = default(CORINFO_RESOLVED_TOKEN); } + bool isArray = decl.OwningType.IsInterface && objType.IsArray; + bool isGenericVirtual = !isArray && decl.HasInstantiation; + MethodDesc instArgTarget = unboxingStub ? nonUnboxingImpl : impl; + bool requiresInstMethodDescArg = instArgTarget.RequiresInstMethodDescArg(); + bool requiresInstMethodTableArg = instArgTarget.RequiresInstMethodTableArg(); + + // For unboxing stubs whose unboxed entry needs a MethodTable inst arg, the boxed object supplies the exact MT. + // For MethodDesc cases we always need to supply the exact MD. + if (requiresInstMethodDescArg || (requiresInstMethodTableArg && !unboxingStub)) + { + if (originalImpl.OwningType.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + // If we end up with a shared MethodTable that is not exact, + // we can't devirtualize since it's not possible to compute the instantiation argument even as a runtime lookup. + info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON; + return false; + } + + if (originalImpl.IsRuntimeDeterminedExactMethod || originalImpl.IsSharedByGenericInstantiations) + { + if (info->pResolvedTokenVirtualMethod == null || unboxingStub) + { + info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON; + return false; + } + +#if READYTORUN + ComputeRuntimeLookupForSharedGenericToken( + Internal.ReadyToRunConstants.DictionaryEntryKind.DevirtualizedMethodDescSlot, + ref *info->pResolvedTokenVirtualMethod, + null, + originalImpl, + MethodBeingCompiled, + ref info->instParamLookup); +#else + // TODO: Implement generic virtual method devirtualization runtime lookup for NativeAOT + info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON; + return false; +#endif + } + } + + if (isArray) + { +#if READYTORUN + // Array interface devirt is not yet supported by R2R. + info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON; + return false; +#else + // NativeAOT handles arrays in a different way that doesn't require an instantiating stub. +#endif + } + + if (!info->instParamLookup.lookupKind.needsRuntimeLookup) + { + if (requiresInstMethodDescArg) + { + if (unboxingStub) + { + // Bail out for now. We need an unboxing stub that points to an instantiated method. + info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON; + return false; + } +#if READYTORUN + MethodWithToken originalImplWithToken = new MethodWithToken(originalImpl, methodWithTokenImpl.Token, null, false, null, null); + info->instParamLookup.constLookup = CreateConstLookupToSymbol(_compilation.SymbolNodeFactory.CreateReadyToRunHelper(ReadyToRunHelperId.MethodDictionary, originalImplWithToken)); +#else + info->instParamLookup.constLookup = CreateConstLookupToSymbol(_compilation.NodeFactory.MethodGenericDictionary(originalImpl)); +#endif + } + else if (requiresInstMethodTableArg) + { + if (!unboxingStub) + { +#if READYTORUN + info->instParamLookup.constLookup = CreateConstLookupToSymbol(_compilation.SymbolNodeFactory.CreateReadyToRunHelper(ReadyToRunHelperId.TypeDictionary, originalImpl.OwningType)); + +#else + info->instParamLookup.constLookup = CreateConstLookupToSymbol(_compilation.NodeFactory.ConstructedTypeSymbol(originalImpl.OwningType)); +#endif + } + } + } + #if READYTORUN // Testing has not shown that concerns about virtual matching are significant // Only generate verification for builds with the stress mode enabled @@ -1497,7 +1581,7 @@ private bool resolveVirtualMethod(CORINFO_DEVIRTUALIZATION_INFO* info) #endif info->detail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_SUCCESS; info->devirtualizedMethod = ObjectToHandle(impl); - info->tokenLookupContext = contextFromType(owningType); + info->tokenLookupContext = (isArray || isGenericVirtual) ? contextFromMethod(originalImpl) : contextFromType(owningType); return true; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DevirtualizationManager.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DevirtualizationManager.cs index 7d0275ff2eaaee..d6ebb33f99db18 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DevirtualizationManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DevirtualizationManager.cs @@ -45,8 +45,7 @@ protected override MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefTyp // // Result method checking // 1. Ensure that the resolved result versions with the code, or is the decl method - // 2. Devirtualizing to a default interface method is not currently considered to be useful, and how to check for version - // resilience has not yet been analyzed. + // 2. When devirtualizing to a default interface method, the resolved result method must version with the code. // 3. When checking that the resolved result versions with the code, validate that all of the types // From implType to the owning type of resolved result method also version with the code. @@ -163,6 +162,17 @@ protected override MethodDesc ResolveVirtualMethod(MethodDesc declMethod, DefTyp if (resolvedVirtualMethod != null) { + if (resolvedVirtualMethod.OwningType.IsInterface) + { + if (_compilationModuleGroup.VersionsWithMethodBody(resolvedVirtualMethod)) + { + return resolvedVirtualMethod; + } + + devirtualizationDetail = CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_BUBBLE; + return null; + } + // Validate that the inheritance chain for resolution is within version bubble // The rule is somewhat tricky here. // If the resolved method is the declMethod, then only types which derive from the diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 3f1bcab55b112f..2ce3602c5c57f0 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -2774,11 +2774,12 @@ private void ComputeRuntimeLookupForSharedGenericToken( break; case DictionaryEntryKind.MethodDescSlot: + case DictionaryEntryKind.DevirtualizedMethodDescSlot: case DictionaryEntryKind.MethodEntrySlot: case DictionaryEntryKind.ConstrainedMethodEntrySlot: case DictionaryEntryKind.DispatchStubAddrSlot: { - if (entryKind == DictionaryEntryKind.MethodDescSlot) + if (entryKind == DictionaryEntryKind.MethodDescSlot || entryKind == DictionaryEntryKind.DevirtualizedMethodDescSlot) { helperId = ReadyToRunHelperId.MethodHandle; } @@ -2806,21 +2807,38 @@ private void ComputeRuntimeLookupForSharedGenericToken( throw new NotImplementedException(entryKind.ToString()); } - object helperArg = GetRuntimeDeterminedObjectForToken(ref pResolvedToken); - if (helperArg is MethodDesc methodDesc) + object helperArg; + if (entryKind == DictionaryEntryKind.DevirtualizedMethodDescSlot) { - var methodIL = HandleToObject(pResolvedToken.tokenScope); - MethodDesc sharedMethod = methodIL.OwningMethod.GetSharedRuntimeFormMethodTarget(); - // We shouldn't be needing shared generics in resumption stubs - generics info should all be stored in the continuation - Debug.Assert(MethodBeingCompiled is not AsyncResumptionStub); - _compilation.NodeFactory.DetectGenericCycles(MethodBeingCompiled, sharedMethod); - helperArg = new MethodWithToken(methodDesc, HandleToModuleToken(ref pResolvedToken, out bool strippedInstantiation), constrainedType, unboxing: false, genericContextObject: sharedMethod, forceOwningTypeFromMethodDesc: strippedInstantiation); + Debug.Assert(templateMethod != null); + MethodDesc tokenMethod = (MethodDesc)GetRuntimeDeterminedObjectForToken(ref pResolvedToken); + if (tokenMethod.HasInstantiation) + { + templateMethod = _compilation.TypeSystemContext.GetInstantiatedMethod(templateMethod.GetMethodDefinition(), tokenMethod.Instantiation); + } + _compilation.NodeFactory.DetectGenericCycles(MethodBeingCompiled, templateMethod); + ModuleToken templateMethodToken = + _compilation.NodeFactory.Resolver.GetModuleTokenForMethod(templateMethod.GetTypicalMethodDefinition(), allowDynamicallyCreatedReference: false, throwIfNotFound: true); + helperArg = new MethodWithToken(templateMethod, templateMethodToken, constrainedType: null, unboxing: false, genericContextObject: null); } - else if (helperArg is FieldDesc fieldDesc) + else { - ModuleToken fieldToken = HandleToModuleToken(ref pResolvedToken, out bool strippedInstantiation); - Debug.Assert(!strippedInstantiation); - helperArg = new FieldWithToken(fieldDesc, fieldToken, forceOwningTypeNotDerivedFromToken: strippedInstantiation); + helperArg = GetRuntimeDeterminedObjectForToken(ref pResolvedToken); + if (helperArg is MethodDesc methodDesc) + { + var methodIL = HandleToObject(pResolvedToken.tokenScope); + MethodDesc sharedMethod = methodIL.OwningMethod.GetSharedRuntimeFormMethodTarget(); + // We shouldn't be needing shared generics in resumption stubs - generics info should all be stored in the continuation + Debug.Assert(MethodBeingCompiled is not AsyncResumptionStub); + _compilation.NodeFactory.DetectGenericCycles(MethodBeingCompiled, sharedMethod); + helperArg = new MethodWithToken(methodDesc, HandleToModuleToken(ref pResolvedToken, out bool strippedInstantiation), constrainedType, unboxing: false, genericContextObject: sharedMethod, forceOwningTypeFromMethodDesc: strippedInstantiation); + } + else if (helperArg is FieldDesc fieldDesc) + { + ModuleToken fieldToken = HandleToModuleToken(ref pResolvedToken, out bool strippedInstantiation); + Debug.Assert(!strippedInstantiation); + helperArg = new FieldWithToken(fieldDesc, fieldToken, forceOwningTypeNotDerivedFromToken: strippedInstantiation); + } } var methodContext = new GenericContext(callerHandle); diff --git a/src/coreclr/vm/genericdict.cpp b/src/coreclr/vm/genericdict.cpp index e9d15c0fa6bb63..326f5500e53f52 100644 --- a/src/coreclr/vm/genericdict.cpp +++ b/src/coreclr/vm/genericdict.cpp @@ -864,6 +864,7 @@ Dictionary::PopulateEntry( } case MethodDescSlot: + case DevirtualizedMethodDescSlot: case DispatchStubAddrSlot: case MethodEntrySlot: { @@ -1202,7 +1203,7 @@ Dictionary::PopulateEntry( } else { - _ASSERTE(kind == MethodDescSlot); + _ASSERTE(kind == MethodDescSlot || kind == DevirtualizedMethodDescSlot); result = (CORINFO_GENERIC_HANDLE)pMethod; } break; diff --git a/src/coreclr/vm/genericdict.h b/src/coreclr/vm/genericdict.h index be2a0af5569029..16bc515eb19998 100644 --- a/src/coreclr/vm/genericdict.h +++ b/src/coreclr/vm/genericdict.h @@ -58,6 +58,7 @@ enum DictionaryEntryKind DispatchStubAddrSlot = 5, FieldDescSlot = 6, DeclaringTypeHandleSlot = 7, + DevirtualizedMethodDescSlot = 8, }; enum DictionaryEntrySignatureSource : BYTE diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 1cb80773f3148a..6e39c59227a218 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -3209,11 +3209,19 @@ void CEEInfo::ComputeRuntimeLookupForSharedGenericToken(DictionaryEntryKind entr FALLTHROUGH; case MethodDescSlot: + case DevirtualizedMethodDescSlot: case MethodEntrySlot: case DispatchStubAddrSlot: { // Encode containing type - if (pResolvedToken->pTypeSpec != NULL) + if (entryKind == DevirtualizedMethodDescSlot) + { + // For shared GVM devirtualization use the devirtualized method owner type from pTemplateMD. + _ASSERTE(pTemplateMD != NULL); + sigBuilder.AppendElementType(ELEMENT_TYPE_INTERNAL); + sigBuilder.AppendPointer(pTemplateMD->GetMethodTable()); + } + else if (pResolvedToken->pTypeSpec != NULL) { SigPointer sigptr(pResolvedToken->pTypeSpec, pResolvedToken->cbTypeSpec); sigptr.ConvertToInternalExactlyOne(pModule, NULL, &sigBuilder); @@ -8770,15 +8778,6 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) info->detail = CORINFO_DEVIRTUALIZATION_FAILED_LOOKUP; return false; } - - // If we devirtualized into a default interface method on a generic type, we should actually return an - // instantiating stub but this is not happening. - // Making this work is tracked by https://github.com/dotnet/runtime/issues/9588 - if (pDevirtMD->GetMethodTable()->IsInterface() && pDevirtMD->HasClassInstantiation()) - { - info->detail = CORINFO_DEVIRTUALIZATION_FAILED_DIM; - return false; - } } else { @@ -8841,53 +8840,118 @@ bool CEEInfo::resolveVirtualMethodHelper(CORINFO_DEVIRTUALIZATION_INFO * info) bool isArray = false; bool isGenericVirtual = 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()) + if (pBaseMT->IsInterface() && pObjMT->IsArray()) { isArray = true; } - else + else if (!pApproxMT->IsInterface()) { pExactMT = pDevirtMD->GetExactDeclaringType(pObjMT); } + MethodDesc* pInstantiatedMD = pDevirtMD; + // This is generic virtual method devirtualization. if (!isArray && pBaseMD->HasMethodInstantiation()) { - MethodDesc* pPrimaryMD = pDevirtMD; + MethodDesc* pPrimaryMD = pDevirtMD->IsInstantiatingStub() ? pDevirtMD->GetWrappedMethodDesc() : pDevirtMD; + pDevirtMD = MethodDesc::FindOrCreateAssociatedMethodDesc( pPrimaryMD, pExactMT, pExactMT->IsValueType() && !pPrimaryMD->IsStatic(), pBaseMD->GetMethodInstantiation(), true); - if (pDevirtMD->IsSharedByGenericMethodInstantiations()) + + pInstantiatedMD = MethodDesc::FindOrCreateAssociatedMethodDesc( + pPrimaryMD, pExactMT, pExactMT->IsValueType() && !pPrimaryMD->IsStatic(), pBaseMD->GetMethodInstantiation(), false); + + isGenericVirtual = true; + } + + MethodDesc* pInstArgMD = pDevirtMD; + bool isUnboxingStubOfInstantiatingStub = false; + + if (pDevirtMD->IsUnboxingStub()) + { + // RequiresInstMethodDescArg and RequiresInstMethodTableArg are only valid for canonical instantiatins, + // use pDevirtMD instead of pInstantiatedMD for pInstArgMD. + // + pInstArgMD = pDevirtMD->GetWrappedMethodDesc(); + if (pInstArgMD->IsInstantiatingStub()) + { + isUnboxingStubOfInstantiatingStub = true; + } + } + else if (pDevirtMD->IsInstantiatingStub()) + { + pInstArgMD = pDevirtMD->GetWrappedMethodDesc(); + } + + if (pInstArgMD->RequiresInstMethodDescArg()) + { + if (TypeHandle::IsCanonicalSubtypeInstantiation(pInstantiatedMD->GetClassInstantiation())) { + // If we end up with a shared MethodTable that is not exact, + // we can't devirtualize since it's not possible to compute the instantiation argument even as a runtime lookup. info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CANON; return false; } - isGenericVirtual = true; - } + if (TypeHandle::IsCanonicalSubtypeInstantiation(pInstantiatedMD->GetMethodInstantiation())) + { + if (info->pResolvedTokenVirtualMethod == nullptr) + { + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CANON; + return false; + } - if (isArray || isGenericVirtual) - { - if (pDevirtMD->IsInstantiatingStub()) + ComputeRuntimeLookupForSharedGenericToken( + DevirtualizedMethodDescSlot, + info->pResolvedTokenVirtualMethod, + nullptr, + pInstantiatedMD->IsUnboxingStub() ? pInstantiatedMD->GetWrappedMethodDesc() : pInstantiatedMD, + m_pMethodBeingCompiled, + &info->instParamLookup); + } + + if (!info->instParamLookup.lookupKind.needsRuntimeLookup) { - info->instParamLookup.constLookup.handle = (CORINFO_GENERIC_HANDLE)pDevirtMD; + info->instParamLookup.constLookup.handle = (CORINFO_GENERIC_HANDLE) pInstantiatedMD; info->instParamLookup.constLookup.accessType = IAT_VALUE; } + } + else if (pInstArgMD->RequiresInstMethodTableArg()) + { + if (!pDevirtMD->IsUnboxingStub() && TypeHandle::IsCanonicalSubtypeInstantiation(pExactMT->GetInstantiation())) + { + // If we end up with a shared MethodTable that is not exact, + // we can't devirtualize since it's not possible to compute the instantiation argument even as a runtime lookup. + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CANON; + return false; + } - info->tokenLookupContext = MAKE_METHODCONTEXT((CORINFO_METHOD_HANDLE) pDevirtMD); - pDevirtMD = pDevirtMD->IsInstantiatingStub() ? pDevirtMD->GetWrappedMethodDesc() : pDevirtMD; + info->instParamLookup.constLookup.handle = (CORINFO_GENERIC_HANDLE) pExactMT; + info->instParamLookup.constLookup.accessType = IAT_VALUE; } - else + else if (isUnboxingStubOfInstantiatingStub) { - info->tokenLookupContext = MAKE_CLASSCONTEXT((CORINFO_CLASS_HANDLE) pExactMT); + if (TypeHandle::IsCanonicalSubtypeInstantiation(pInstantiatedMD->GetClassInstantiation()) || + TypeHandle::IsCanonicalSubtypeInstantiation(pInstantiatedMD->GetMethodInstantiation())) + { + // This is an unboxing stub that points to an instantiating stub that requires a runtime lookup. + // Bail out. + info->detail = CORINFO_DEVIRTUALIZATION_FAILED_CANON; + return false; + } + + // pInstArgMD is the wrapped instantiating stub in the unboxing stub. + // + info->instParamLookup.constLookup.handle = (CORINFO_GENERIC_HANDLE) pInstArgMD; + info->instParamLookup.constLookup.accessType = IAT_VALUE; } + pDevirtMD = pDevirtMD->IsInstantiatingStub() ? pDevirtMD->GetWrappedMethodDesc() : pDevirtMD; + info->tokenLookupContext = (isArray || isGenericVirtual) + ? MAKE_METHODCONTEXT((CORINFO_METHOD_HANDLE) pInstantiatedMD) + : MAKE_CLASSCONTEXT((CORINFO_CLASS_HANDLE) pExactMT); + // If we devirtualized into an unboxing stub, also hand back the unboxed entry // so the jit can perform the unboxing transformation. // @@ -14639,8 +14703,8 @@ BOOL LoadDynamicInfoEntry(Module *currentModule, } } - // Strip off method instantiation for comparison if the method is generic virtual. - if (pDeclMethod->HasMethodInstantiation()) + // Strip off method instantiation for comparison if the method is generic virtual or generic DIM. + if (pDeclMethod->HasMethodInstantiation() || pDeclMethod->IsInterface()) { if (pImplMethodRuntime != NULL) { diff --git a/src/tests/JIT/Generics/VirtualMethods/generic_virtual_methods.cs b/src/tests/JIT/Generics/VirtualMethods/generic_virtual_methods.cs index 7aba5abe26bab3..d2af6ea3d601db 100644 --- a/src/tests/JIT/Generics/VirtualMethods/generic_virtual_methods.cs +++ b/src/tests/JIT/Generics/VirtualMethods/generic_virtual_methods.cs @@ -141,7 +141,14 @@ public static void GenericInterfaceBase_GenericStructDerived_NoInliningVariants( [Fact] public static void RuntimeLookupDelegate() { - RuntimeLookupDelegateGenericVirtual.Test(); + RuntimeLookupDelegateGenericVirtual.TestGenericMethodOnNonGenericType(); + RuntimeLookupDelegateGenericVirtual.TestGenericMethodOnNonGenericType(); + RuntimeLookupDelegateGenericVirtual.TestGenericMethodOnGenericType(); + RuntimeLookupDelegateGenericVirtual.TestGenericMethodOnGenericType(); + RuntimeLookupDelegateGenericVirtual.TestGenericMethodOnGenericType(); + RuntimeLookupDelegateGenericVirtual.TestGenericMethodOnGenericType(); + RuntimeLookupDelegateGenericVirtual.TestGenericMethodOnStringType(); + RuntimeLookupDelegateGenericVirtual.TestGenericMethodOnStringType(); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -210,15 +217,103 @@ internal class RuntimeLookupDelegateGenericVirtual { internal static readonly List s_list = new(); - internal static void Test() + internal static void TestGenericMethodOnNonGenericType() { - var test = new Base(); - test.Foo>(); + Console.WriteLine("Testing {0}: {1}...", nameof(TestGenericMethodOnNonGenericType), typeof(T)); - var test2 = new Derived(); - Delegate m1 = test2.Foo>(); - Delegate m2 = test2.Foo>>; + Base test1 = new Base(); + test1.Foo>(); + Assert.Contains(typeof(List>>>>), s_list); + + s_list.Clear(); + + IBase test2 = new DerivedClassNoImpl(); + test2.Foo>(); + Assert.Contains(typeof(List>>>>), s_list); + + s_list.Clear(); + + IBase test3 = new DerivedStructNoImpl(); + test3.Foo>(); + Assert.Contains(typeof(List>>>>), s_list); + + s_list.Clear(); + + Base test4 = new DerivedClass(); + Delegate m1 = test4.Foo>(); + Delegate m2 = test4.Foo>>; + Assert.Equal(m1, m2); + + IBase test5 = new DerivedStruct(); + Delegate m3 = test5.Foo>(); + Delegate m4 = test5.Foo>>; + Assert.Equal(m3.Method, m4.Method); + } + + internal static void TestGenericMethodOnGenericType() + { + Console.WriteLine("Testing {0}: {1}, {2}...", nameof(TestGenericMethodOnGenericType), typeof(T), typeof(U)); + + Base test1 = new Base(); + test1.Foo>(); + Assert.Contains(typeof(List>>>>), s_list); + + s_list.Clear(); + + IBase test2 = new DerivedClassNoImpl(); + test2.Foo>(); + Assert.Contains(typeof(List>>>>), s_list); + + s_list.Clear(); + + IBase test3 = new DerivedStructNoImpl(); + test3.Foo>(); + Assert.Contains(typeof(List>>>>), s_list); + + s_list.Clear(); + + Base test4 = new DerivedClass(); + Delegate m1 = test4.Foo>(); + Delegate m2 = test4.Foo>>; + Assert.Equal(m1, m2); + + IBase test5 = new DerivedStruct(); + Delegate m3 = test5.Foo>(); + Delegate m4 = test5.Foo>>; + Assert.Equal(m3.Method, m4.Method); + } + + internal static void TestGenericMethodOnStringType() + { + Console.WriteLine("Testing {0}: {1}...", nameof(TestGenericMethodOnStringType), typeof(T)); + + Base test1 = new Base(); + test1.Foo>(); + Assert.Contains(typeof(List>>>>), s_list); + + s_list.Clear(); + + IBase test2 = new DerivedClassStringNoImpl(); + test2.Foo>(); + Assert.Contains(typeof(List>>>>), s_list); + + s_list.Clear(); + + IBase test3 = new DerivedStructStringNoImpl(); + test3.Foo>(); + Assert.Contains(typeof(List>>>>), s_list); + + s_list.Clear(); + + Base test4 = new DerivedClassString(); + Delegate m1 = test4.Foo>(); + Delegate m2 = test4.Foo>>; Assert.Equal(m1, m2); + + IBase test5 = new DerivedStructString(); + Delegate m3 = test5.Foo>(); + Delegate m4 = test5.Foo>>; + Assert.Equal(m3.Method, m4.Method); } } @@ -231,13 +326,98 @@ public virtual Delegate Foo() RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>)); RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>)); RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>>)); - RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>>>)); - RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>>>>)); return Foo; } } -internal class Derived : Base +internal class DerivedClass : Base +{ + public override Delegate Foo() + { + return Foo>; + } +} + +internal interface IBase +{ + public virtual Delegate Foo() + { + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(U)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>>)); + return Foo; + } +} + +internal class DerivedClassNoImpl : IBase +{ +} + +internal struct DerivedStructNoImpl : IBase +{ +} + +internal struct DerivedStruct : IBase +{ + public Delegate Foo() + { + return Foo>; + } +} + +internal class Base +{ + public virtual Delegate Foo() + { + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(U)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>>)); + return Foo; + } +} + +internal class DerivedClass : Base +{ + public override Delegate Foo() + { + return Foo>; + } +} + +internal interface IBase +{ + public virtual Delegate Foo() + { + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(U)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>)); + RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List>>>)); + return Foo; + } +} + +internal class DerivedClassNoImpl : IBase +{ +} + +internal struct DerivedStructNoImpl : IBase +{ +} + +internal struct DerivedStruct : IBase +{ + public Delegate Foo() + { + return Foo>; + } +} + +internal class DerivedClassString : Base { public override Delegate Foo() { @@ -245,6 +425,22 @@ public override Delegate Foo() } } +internal class DerivedClassStringNoImpl : IBase +{ +} + +internal struct DerivedStructStringNoImpl : IBase +{ +} + +internal struct DerivedStructString : IBase +{ + public Delegate Foo() + { + return Foo>; + } +} + internal static class IconContextBridgeNonShared { public static TMethod SameMethodSameClass(IBaseMethodCaller caller, TMethod value) diff --git a/src/tests/Loader/classloader/DefaultInterfaceMethods/devirtualization/dim_devirtualization.cs b/src/tests/Loader/classloader/DefaultInterfaceMethods/devirtualization/dim_devirtualization.cs new file mode 100644 index 00000000000000..b71856d12b2df5 --- /dev/null +++ b/src/tests/Loader/classloader/DefaultInterfaceMethods/devirtualization/dim_devirtualization.cs @@ -0,0 +1,278 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; +using Xunit; + +public class Program +{ + [Fact] + public static void TestEntryPoint() + { + GitHubIssue39419RegressionCase.Run(); + ConstrainedGenericInlineCase.Run(); + MultiInterfaceDefaultMethodCases.Run(); + NonGenericDefaultMethodCase.Run(); + OverriddenDefaultMethodCases.Run(); + ValueTypeArgumentDefaultMethodCase.Run(); + StructNonDefaultImplementationCase.Run(); + ContravariantDefaultMethodCase.Run(); + DirectStructMethodCase.Run(); + StaticGenericMethodCase.Run(); + } +} + +static class GitHubIssue39419RegressionCase +{ + interface IM + { + bool UseDefaultM + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => true; + } + + ValueTask M(T instance) => throw new NotImplementedException("M must be implemented if UseDefaultM is false"); + + static ValueTask DefaultM(T instance) + { + return default; + } + } + + struct M : IM + { + } + + public static void Run() + { + var m = new M(); + if (((IM)m).UseDefaultM) + { + IM.DefaultM(42); + return; + } + + ((IM)m).M(42); + throw new UnreachableException(); + } +} + +static class ConstrainedGenericInlineCase +{ + interface I where T : IComparable + { + T GetAt(int i, T[] tx) => tx[i]; + } + + class C : I + { + } + + private static readonly string[] s_values = new string[] { "test" }; + + public static void Run() + { + I c = new C(); + var dcs = c.GetAt(0, s_values); + Assert.Equal("test", dcs); + } +} + +static class MultiInterfaceDefaultMethodCases +{ + interface I + { + string DefaultTypeOf() => typeof(T).Name; + } + + class Dummy + { + } + + class ClassImplementation : I, I, I + { + string I.DefaultTypeOf() => "C.Dummy"; + } + + struct StructImplementation : I, I, I + { + string I.DefaultTypeOf() => "C.Dummy"; + } + + public static void Run() + { + RunClassImplementation(); + RunStructImplementation(); + } + + static void RunClassImplementation() + { + var c = new ClassImplementation(); + var dcs = ((I)c).DefaultTypeOf(); + Assert.Equal("String", dcs); + var dos = ((I)c).DefaultTypeOf(); + Assert.Equal("Object", dos); + var dds = ((I)c).DefaultTypeOf(); + Assert.Equal("C.Dummy", dds); + } + + static void RunStructImplementation() + { + var c = new StructImplementation(); + var dcs = ((I)c).DefaultTypeOf(); + Assert.Equal("String", dcs); + var dos = ((I)c).DefaultTypeOf(); + Assert.Equal("Object", dos); + var dds = ((I)c).DefaultTypeOf(); + Assert.Equal("C.Dummy", dds); + } +} + +static class NonGenericDefaultMethodCase +{ + interface I + { + string DefaultTypeOf() => typeof(string).Name; + } + + class C : I + { + } + + public static void Run() + { + var c = new C(); + var dcs = ((I)c).DefaultTypeOf(); + Assert.Equal("String", dcs); + } +} + +static class OverriddenDefaultMethodCases +{ + interface I + { + string DefaultTypeOf() => typeof(T).Name; + } + class C : I + { + public string DefaultTypeOf() => "C.String"; + } + + class GenericC : I + { + public string DefaultTypeOf() => "C." + typeof(T).Name; + } + + public static void Run() + { + RunNonGenericOverride(); + RunGenericOverride(); + } + + static void RunNonGenericOverride() + { + var c = new C(); + var dcs = ((I)c).DefaultTypeOf(); + Assert.Equal("C.String", dcs); + } + + static void RunGenericOverride() + { + var c = new GenericC(); + var dcs = ((I)c).DefaultTypeOf(); + Assert.Equal("C.String", dcs); + } +} + +static class ValueTypeArgumentDefaultMethodCase +{ + interface I + { + string DefaultTypeOf() => typeof(T).Name; + } + + class C : I + { + } + + public static void Run() + { + var c = new C(); + var dcs = ((I)c).DefaultTypeOf(); + Assert.Equal("Int32", dcs); + } +} + +static class StructNonDefaultImplementationCase +{ + interface I + { + string DefaultTypeOf(); + } + + struct C : I + { + public string DefaultTypeOf() => "C." + typeof(T).Name; + } + + public static void Run() + { + var c = new C(); + var dcs = ((I)c).DefaultTypeOf(); + Assert.Equal("C.String", dcs); + } +} + +static class ContravariantDefaultMethodCase +{ + interface I + { + string DefaultTypeOf() => typeof(T).Name; + } + + class C : I + { + } + + public static void Run() + { + var c = new C(); + var dcs = ((I)c).DefaultTypeOf(); + Assert.Equal("Object", dcs); + } +} + +static class DirectStructMethodCase +{ + struct C + { + [MethodImpl(MethodImplOptions.NoInlining)] + public string DefaultTypeOf() => typeof(T).Name; + } + + public static void Run() + { + var c = new C(); + var dcs = c.DefaultTypeOf(); + Assert.Equal("String", dcs); + } +} + +static class StaticGenericMethodCase +{ + class C + { + public static string DefaultTypeOf() => typeof(T).Name; + } + + public static void Run() + { + var dcs = C.DefaultTypeOf(); + Assert.Equal("String", dcs); + } +} + diff --git a/src/tests/Loader/classloader/DefaultInterfaceMethods/devirtualization/dim_devirtualization.csproj b/src/tests/Loader/classloader/DefaultInterfaceMethods/devirtualization/dim_devirtualization.csproj new file mode 100644 index 00000000000000..30d4804022360c --- /dev/null +++ b/src/tests/Loader/classloader/DefaultInterfaceMethods/devirtualization/dim_devirtualization.csproj @@ -0,0 +1,11 @@ + + + 1 + + + + + + + +