diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/GenericDictionaryNode.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/GenericDictionaryNode.cs index 8b83b227a33..a66288e4ae2 100644 --- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/GenericDictionaryNode.cs +++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/GenericDictionaryNode.cs @@ -112,8 +112,8 @@ public override IEnumerable GetConditionalStaticDep // that use the same dictionary layout. foreach (var method in _owningType.GetAllMethods()) { - // Static and generic methods have their own generic dictionaries - if (method.Signature.IsStatic || method.HasInstantiation) + // Generic methods have their own generic dictionaries + if (method.HasInstantiation) continue; // Abstract methods don't have a body diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/GenericLookupResult.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/GenericLookupResult.cs index 76a470d8c62..5760d328bb0 100644 --- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/GenericLookupResult.cs +++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/GenericLookupResult.cs @@ -128,6 +128,11 @@ internal sealed class VirtualDispatchGenericLookupResult : GenericLookupResult public VirtualDispatchGenericLookupResult(MethodDesc method) { Debug.Assert(method.IsRuntimeDeterminedExactMethod); + Debug.Assert(method.IsVirtual); + + // Normal virtual methods don't need a generic lookup. + Debug.Assert(method.OwningType.IsInterface || method.HasInstantiation); + _method = method; } @@ -146,6 +151,42 @@ public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilde public override string ToString() => $"VirtualCall: {_method}"; } + /// + /// Generic lookup result that points to a virtual function address load stub. + /// + internal sealed class VirtualResolveGenericLookupResult : GenericLookupResult + { + private MethodDesc _method; + + public VirtualResolveGenericLookupResult(MethodDesc method) + { + Debug.Assert(method.IsRuntimeDeterminedExactMethod); + Debug.Assert(method.IsVirtual); + + // Normal virtual methods don't need a generic lookup. + Debug.Assert(method.OwningType.IsInterface || method.HasInstantiation); + + _method = method; + } + + public override ISymbolNode GetTarget(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation) + { + MethodDesc instantiatedMethod = _method.InstantiateSignature(typeInstantiation, methodInstantiation); + + // https://github.com/dotnet/corert/issues/2342 - we put a pointer to the virtual call helper into the dictionary + // but this should be something that will let us compute the target of the dipatch (e.g. interface dispatch cell). + return factory.ReadyToRunHelper(ReadyToRunHelperId.VirtualCall, instantiatedMethod); + } + + public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb) + { + sb.Append("VirtualResolve_"); + sb.Append(nameMangler.GetMangledMethodName(_method)); + } + + public override string ToString() => $"VirtualResolve: {_method}"; + } + /// /// Generic lookup result that points to the non-GC static base of a type. /// diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/NodeFactory.GenericLookups.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/NodeFactory.GenericLookups.cs index 36a227f487f..decf899d185 100644 --- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/NodeFactory.GenericLookups.cs +++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/NodeFactory.GenericLookups.cs @@ -42,6 +42,11 @@ private void CreateNodeCaches() return new VirtualDispatchGenericLookupResult(method); }); + _virtualResolveHelpers = new NodeCache(method => + { + return new VirtualResolveGenericLookupResult(method); + }); + _typeGCStaticBaseSymbols = new NodeCache(type => { return new TypeGCStaticBaseGenericLookupResult(type); @@ -88,6 +93,13 @@ public GenericLookupResult VirtualCall(MethodDesc method) return _virtualCallHelpers.GetOrAdd(method); } + private NodeCache _virtualResolveHelpers; + + public GenericLookupResult VirtualMethodAddress(MethodDesc method) + { + return _virtualResolveHelpers.GetOrAdd(method); + } + private NodeCache _methodEntrypoints; public GenericLookupResult MethodEntry(MethodDesc method) diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs index 98e36e587c8..e6885423582 100644 --- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs +++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs @@ -40,6 +40,8 @@ private static GenericLookupResult GetLookupSignature(NodeFactory factory, Ready return factory.GenericLookup.MethodDictionary((MethodDesc)target); case ReadyToRunHelperId.VirtualCall: return factory.GenericLookup.VirtualCall((MethodDesc)target); + case ReadyToRunHelperId.ResolveVirtualFunction: + return factory.GenericLookup.VirtualMethodAddress((MethodDesc)target); case ReadyToRunHelperId.MethodEntry: return factory.GenericLookup.MethodEntry((MethodDesc)target); default: diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/Target_X64/X64ReadyToRunGenericHelperNode.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/Target_X64/X64ReadyToRunGenericHelperNode.cs index b3859b87bb9..9b5bba78bd2 100644 --- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/Target_X64/X64ReadyToRunGenericHelperNode.cs +++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/Target_X64/X64ReadyToRunGenericHelperNode.cs @@ -103,6 +103,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref X64Emitter enco case ReadyToRunHelperId.TypeHandle: case ReadyToRunHelperId.MethodDictionary: case ReadyToRunHelperId.VirtualCall: + case ReadyToRunHelperId.ResolveVirtualFunction: case ReadyToRunHelperId.MethodEntry: { EmitDictionaryLookup(factory, ref encoder, encoder.TargetRegister.Arg0, encoder.TargetRegister.Result, _lookupSignature, relocsOnly); diff --git a/src/JitInterface/src/CorInfoImpl.cs b/src/JitInterface/src/CorInfoImpl.cs index 2f4e6606811..99ccaad4eb3 100644 --- a/src/JitInterface/src/CorInfoImpl.cs +++ b/src/JitInterface/src/CorInfoImpl.cs @@ -2683,13 +2683,39 @@ private void getCallInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_RESO (void*)ObjectToHandle(_compilation.NodeFactory.ReadyToRunHelper(ReadyToRunHelperId.ResolveGenericVirtualMethod, targetMethod)); pResult.nullInstanceCheck = false; } - else if ((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_LDFTN) != 0) + else if((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_LDFTN) != 0) { pResult.kind = CORINFO_CALL_KIND.CORINFO_VIRTUALCALL_LDVIRTFTN; - pResult.codePointerOrStubLookup.constLookup.accessType = InfoAccessType.IAT_VALUE; - pResult.codePointerOrStubLookup.constLookup.addr = - (void*)ObjectToHandle(_compilation.NodeFactory.ReadyToRunHelper(ReadyToRunHelperId.ResolveVirtualFunction, targetMethod)); + // If this is a non-interface/non-GVM call, we actually don't need a runtime lookup to find the target. + // We don't even need to keep track of the runtime-determined method being called because the system ensures + // that if e.g. Foo<__Canon>.GetHashCode is needed and we're generating a dictionary for Foo, + // Foo.GetHashCode is needed too. + if (pResult.exactContextNeedsRuntimeLookup && + (targetMethod.HasInstantiation || targetMethod.OwningType.IsInterface)) + { + pResult.codePointerOrStubLookup.lookupKind.needsRuntimeLookup = true; + pResult.codePointerOrStubLookup.runtimeLookup.indirections = CORINFO.USEHELPER; + + // Do not bother computing the runtime lookup if we are inlining. The JIT is going + // to abort the inlining attempt anyway. + MethodDesc contextMethod = methodFromContext(pResolvedToken.tokenContext); + if (contextMethod != MethodBeingCompiled) + { + return; + } + + pResult.codePointerOrStubLookup.lookupKind.runtimeLookupKind = GetGenericRuntimeLookupKind(contextMethod); + pResult.codePointerOrStubLookup.lookupKind.runtimeLookupFlags = (ushort)ReadyToRunHelperId.ResolveVirtualFunction; + } + else + { + pResult.exactContextNeedsRuntimeLookup = false; + pResult.codePointerOrStubLookup.constLookup.accessType = InfoAccessType.IAT_VALUE; + + pResult.codePointerOrStubLookup.constLookup.addr = + (void*)ObjectToHandle(_compilation.NodeFactory.ReadyToRunHelper(ReadyToRunHelperId.ResolveVirtualFunction, targetMethod)); + } // The current CoreRT ReadyToRun helpers do not handle null thisptr - ask the JIT to emit explicit null checks // TODO: Optimize this @@ -2701,10 +2727,14 @@ private void getCallInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_RESO // call that should not be inlined. pResult.kind = CORINFO_CALL_KIND.CORINFO_CALL_CODE_POINTER; - if (pResult.exactContextNeedsRuntimeLookup) + // If this is a non-interface/non-GVM call, we actually don't need a runtime lookup to find the target. + // We don't even need to keep track of the runtime-determined method being called because the system ensures + // that if e.g. Foo<__Canon>.GetHashCode is needed and we're generating a dictionary for Foo, + // Foo.GetHashCode is needed too. + if (pResult.exactContextNeedsRuntimeLookup && + (targetMethod.HasInstantiation || targetMethod.OwningType.IsInterface)) { pResult.codePointerOrStubLookup.lookupKind.needsRuntimeLookup = true; - pResult.codePointerOrStubLookup.lookupKind.runtimeLookupFlags = 0; pResult.codePointerOrStubLookup.runtimeLookup.indirections = CORINFO.USEHELPER; // Do not bother computing the runtime lookup if we are inlining. The JIT is going @@ -2720,6 +2750,7 @@ private void getCallInfo(ref CORINFO_RESOLVED_TOKEN pResolvedToken, CORINFO_RESO } else { + pResult.exactContextNeedsRuntimeLookup = false; pResult.codePointerOrStubLookup.constLookup.accessType = InfoAccessType.IAT_VALUE; pResult.codePointerOrStubLookup.constLookup.addr = diff --git a/tests/src/Simple/Generics/Generics.cs b/tests/src/Simple/Generics/Generics.cs index 859a6b98ca9..d181130ac66 100644 --- a/tests/src/Simple/Generics/Generics.cs +++ b/tests/src/Simple/Generics/Generics.cs @@ -14,6 +14,8 @@ static int Main() TestDelegateFatFunctionPointers.Run(); TestVirtualMethodUseTracking.Run(); TestSlotsInHierarchy.Run(); + TestDelegateVirtualMethod.Run(); + TestDelegateInterfaceMethod.Run(); TestNameManglingCollisionRegression.Run(); TestUnusedGVMsDoNotCrashCompiler.Run(); @@ -142,6 +144,63 @@ public static void Run() } } + class TestDelegateVirtualMethod + { + static void Generic() + { + Base o = new Derived(); + Func f = o.Do; + if (f() != "Derived") + throw new Exception(); + + o = new Base(); + f = o.Do; + if (f() != "Base") + throw new Exception(); + } + + public static void Run() + { + Generic(); + } + + class Base + { + public virtual string Do() => "Base"; + } + + class Derived : Base + { + public override string Do() => "Derived"; + } + } + + class TestDelegateInterfaceMethod + { + static void Generic() + { + IFoo o = new Foo(); + Func f = o.Do; + if (f() != "Foo") + throw new Exception(); + } + + public static void Run() + { + Generic(); + } + + interface IFoo + { + string Do(); + } + + class Foo : IFoo + { + public string Do() => "Foo"; + } + } + /// /// Tests RyuJIT's initThisClass. ///