Enable inlining of virtual method calls (#3773)

13 files changed, 155 insertions, 44 deletions

diff --git a/src/ILCompiler.Compiler/src/Compiler/Compilation.cs b/src/ILCompiler.Compiler/src/Compiler/Compilation.cs
index a9770431a..2d69417b1 100644
--- a/src/ILCompiler.Compiler/src/Compiler/Compilation.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/Compilation.cs
@@ -176,6 +176,11 @@ namespace ILCompiler
             return intrinsicMethod;
         }
 
+        public bool HasFixedSlotVTable(TypeDesc type)
+        {
+            return NodeFactory.VTable(type).HasFixedSlots;
+        }
+
         void ICompilation.Compile(string outputFile, ObjectDumper dumper)
         {
             if (dumper != null)
diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs
index 007ee65ed..acb7072b7 100644
--- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs
@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System;
 using System.Diagnostics;
 
 using Internal.Runtime;
@@ -13,8 +12,8 @@ namespace ILCompiler.DependencyAnalysis
 {
     public class InterfaceDispatchCellNode : ObjectNode, ISymbolDefinitionNode
     {
-        MethodDesc _targetMethod;
-        string _callSiteIdentifier;
+        private readonly MethodDesc _targetMethod;
+        private readonly string _callSiteIdentifier;
 
         public InterfaceDispatchCellNode(MethodDesc targetMethod, string callSiteIdentifier)
         {
@@ -49,7 +48,22 @@ namespace ILCompiler.DependencyAnalysis
         public override ObjectNodeSection Section => ObjectNodeSection.DataSection;
 
         public override bool StaticDependenciesAreComputed => true;
-        
+
+        protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFactory factory)
+        {
+            DependencyList result = new DependencyList();
+
+            if (!factory.VTable(_targetMethod.OwningType).HasFixedSlots)
+            {
+                result.Add(factory.VirtualMethodUse(_targetMethod), "Interface method use");
+            }
+
+            // TODO: https://github.com/dotnet/corert/issues/3224 
+            result.Add(factory.ReflectableMethod(_targetMethod), "Abstract reflectable method");
+
+            return result;
+        }
+
         public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
         {
             ObjectDataBuilder objData = new ObjectDataBuilder(factory, relocsOnly);
diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs
index f5f788ee1..6f91ff373 100644
--- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs
@@ -49,6 +49,8 @@ namespace ILCompiler.DependencyAnalysis
                     return factory.GenericLookup.MethodDictionary((MethodDesc)target);
                 case ReadyToRunHelperId.VirtualCall:
                     return factory.GenericLookup.VirtualCall((MethodDesc)target);
+                case ReadyToRunHelperId.VirtualDispatchCell:
+                    return factory.GenericLookup.VirtualMethodAddress((MethodDesc)target);
                 case ReadyToRunHelperId.ResolveVirtualFunction:
                     return factory.GenericLookup.VirtualMethodAddress((MethodDesc)target);
                 case ReadyToRunHelperId.MethodEntry:
diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs
index 1ab4a73f4..c3bfaf83c 100644
--- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs
@@ -31,6 +31,7 @@ namespace ILCompiler.DependencyAnalysis
         FieldHandle,
         MethodDictionary,
         MethodEntry,
+        VirtualDispatchCell,
         DefaultConstructor,
     }
 
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 c49d5eb8f..e3b4ac7b9 100644
--- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/Target_X64/X64ReadyToRunGenericHelperNode.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/Target_X64/X64ReadyToRunGenericHelperNode.cs
@@ -210,6 +210,7 @@ namespace ILCompiler.DependencyAnalysis
                 case ReadyToRunHelperId.VirtualCall:
                 case ReadyToRunHelperId.ResolveVirtualFunction:
                 case ReadyToRunHelperId.MethodEntry:
+                case ReadyToRunHelperId.VirtualDispatchCell:
                 case ReadyToRunHelperId.DefaultConstructor:
                     {
                         EmitDictionaryLookup(factory, ref encoder, contextRegister, encoder.TargetRegister.Result, _lookupSignature, relocsOnly);
diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/Target_X64/X64ReadyToRunHelperNode.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/Target_X64/X64ReadyToRunHelperNode.cs
index 82247a54b..8e7fbab71 100644
--- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/Target_X64/X64ReadyToRunHelperNode.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/Target_X64/X64ReadyToRunHelperNode.cs
@@ -31,27 +31,22 @@ namespace ILCompiler.DependencyAnalysis
                     {
                         MethodDesc targetMethod = (MethodDesc)Target;
 
-                        if (targetMethod.OwningType.IsInterface)
-                        {
-                            encoder.EmitLEAQ(Register.R10, factory.InterfaceDispatchCell((MethodDesc)Target));
-                            AddrMode jmpAddrMode = new AddrMode(Register.R10, null, 0, 0, AddrModeSize.Int64);
-                            encoder.EmitJmpToAddrMode(ref jmpAddrMode);
-                        }
-                        else
-                        {
-                            if (relocsOnly)
-                                break;
+                        Debug.Assert(!targetMethod.OwningType.IsInterface);
 
-                            AddrMode loadFromThisPtr = new AddrMode(encoder.TargetRegister.Arg0, null, 0, 0, AddrModeSize.Int64);
-                            encoder.EmitMOV(encoder.TargetRegister.Result, ref loadFromThisPtr);
+                        AddrMode loadFromThisPtr = new AddrMode(encoder.TargetRegister.Arg0, null, 0, 0, AddrModeSize.Int64);
+                        encoder.EmitMOV(encoder.TargetRegister.Result, ref loadFromThisPtr);
 
-                            int pointerSize = factory.Target.PointerSize;
+                        int pointerSize = factory.Target.PointerSize;
 
-                            int slot = VirtualMethodSlotHelper.GetVirtualMethodSlot(factory, targetMethod);
+                        int slot = 0;
+                        if (!relocsOnly)
+                        {
+                            slot = VirtualMethodSlotHelper.GetVirtualMethodSlot(factory, targetMethod);
                             Debug.Assert(slot != -1);
-                            AddrMode jmpAddrMode = new AddrMode(encoder.TargetRegister.Result, null, EETypeNode.GetVTableOffset(pointerSize) + (slot * pointerSize), 0, AddrModeSize.Int64);
-                            encoder.EmitJmpToAddrMode(ref jmpAddrMode);
                         }
+
+                        AddrMode jmpAddrMode = new AddrMode(encoder.TargetRegister.Result, null, EETypeNode.GetVTableOffset(pointerSize) + (slot * pointerSize), 0, AddrModeSize.Int64);
+                        encoder.EmitJmpToAddrMode(ref jmpAddrMode);
                     }
                     break;
 
diff --git a/src/ILCompiler.Compiler/src/IL/ILImporter.Scanner.cs b/src/ILCompiler.Compiler/src/IL/ILImporter.Scanner.cs
index eeaebf363..a44116f4f 100644
--- a/src/ILCompiler.Compiler/src/IL/ILImporter.Scanner.cs
+++ b/src/ILCompiler.Compiler/src/IL/ILImporter.Scanner.cs
@@ -592,32 +592,26 @@ namespace Internal.IL
 
                 _dependencies.Add(GetHelperEntrypoint(ReadyToRunHelper.GVMLookupForSlot), reason);
             }
-            else
+            else if (method.OwningType.IsInterface)
             {
-                ReadyToRunHelperId helper;
-                if (opcode == ILOpcode.ldvirtftn)
+                if (exactContextNeedsRuntimeLookup)
                 {
-                    helper = ReadyToRunHelperId.ResolveVirtualFunction;
+                    _dependencies.Add(GetGenericLookupHelper(ReadyToRunHelperId.VirtualDispatchCell, runtimeDeterminedMethod), reason);
                 }
                 else
                 {
-                    Debug.Assert(opcode == ILOpcode.callvirt);
-                    helper = ReadyToRunHelperId.VirtualCall;
+                    _dependencies.Add(_factory.InterfaceDispatchCell(method), reason);
                 }
-
-                if (exactContextNeedsRuntimeLookup && targetMethod.OwningType.IsInterface)
-                {
-                    _dependencies.Add(GetGenericLookupHelper(helper, runtimeDeterminedMethod), reason);
-                }
-                else
-                {
-                    // Get the slot defining method to make sure our virtual method use tracking gets this right.
-                    // For normal C# code the targetMethod will always be newslot.
-                    MethodDesc slotDefiningMethod = targetMethod.IsNewSlot ?
+            }
+            else if (_compilation.HasFixedSlotVTable(method.OwningType))
+            {
+                // No dependencies: virtual call through the vtable
+            }
+            else
+            {
+                MethodDesc slotDefiningMethod = targetMethod.IsNewSlot ?
                         targetMethod : MetadataVirtualMethodAlgorithm.FindSlotDefiningMethodForVirtualMethod(targetMethod);
-
-                    _dependencies.Add(_factory.ReadyToRunHelper(helper, slotDefiningMethod), reason);
-                }
+                _dependencies.Add(_factory.VirtualMethodUse(slotDefiningMethod), reason);
             }
         }
 
diff --git a/src/ILCompiler.CppCodeGen/src/CppCodeGen/ILToCppImporter.cs b/src/ILCompiler.CppCodeGen/src/CppCodeGen/ILToCppImporter.cs
index cac63cb1e..dce48157a 100644
--- a/src/ILCompiler.CppCodeGen/src/CppCodeGen/ILToCppImporter.cs
+++ b/src/ILCompiler.CppCodeGen/src/CppCodeGen/ILToCppImporter.cs
@@ -990,7 +990,6 @@ namespace Internal.IL
 
             if (callViaInterfaceDispatch)
             {
-                _dependencies.Add(_nodeFactory.ReadyToRunHelper(ReadyToRunHelperId.VirtualCall, method));
                 ExpressionEntry v = (ExpressionEntry)_stack[_stack.Top - (methodSignature.Length + 1)];
 
                 string typeDefName = _writer.GetCppMethodName(method);
diff --git a/src/JitInterface/src/CorInfoImpl.cs b/src/JitInterface/src/CorInfoImpl.cs
index 0e483a666..a13855c08 100644
--- a/src/JitInterface/src/CorInfoImpl.cs
+++ b/src/JitInterface/src/CorInfoImpl.cs
@@ -797,10 +797,66 @@ namespace Internal.JitInterface
 
         private CORINFO_MODULE_STRUCT_* getMethodModule(CORINFO_METHOD_STRUCT_* method)
         { throw new NotImplementedException("getMethodModule"); }
+
         private void getMethodVTableOffset(CORINFO_METHOD_STRUCT_* method, ref uint offsetOfIndirection, ref uint offsetAfterIndirection)
-        { throw new NotImplementedException("getMethodVTableOffset"); }
-        private CORINFO_METHOD_STRUCT_* resolveVirtualMethod(CORINFO_METHOD_STRUCT_* virtualMethod, CORINFO_CLASS_STRUCT_* implementingClass, CORINFO_CONTEXT_STRUCT* ownerType)
-        { throw new NotImplementedException("resolveVirtualMethod"); }
+        {
+            MethodDesc methodDesc = HandleToObject(method);
+            int pointerSize = _compilation.TypeSystemContext.Target.PointerSize;
+            offsetOfIndirection = (uint)CORINFO_VIRTUALCALL_NO_CHUNK.Value;
+
+            // Normalize to the slot defining method. We don't have slot information for the overrides.
+            methodDesc = MetadataVirtualMethodAlgorithm.FindSlotDefiningMethodForVirtualMethod(methodDesc);
+
+            int slot = VirtualMethodSlotHelper.GetVirtualMethodSlot(_compilation.NodeFactory, methodDesc);
+            Debug.Assert(slot != -1);
+
+            offsetAfterIndirection = (uint)(EETypeNode.GetVTableOffset(pointerSize) + slot * pointerSize);
+        }
+
+        private CORINFO_METHOD_STRUCT_* resolveVirtualMethod(CORINFO_METHOD_STRUCT_* baseMethod, CORINFO_CLASS_STRUCT_* derivedClass, CORINFO_CONTEXT_STRUCT* ownerType)
+        {
+            TypeDesc implType = HandleToObject(derivedClass);
+
+            // __Canon cannot be devirtualized
+            if (implType.IsCanonicalDefinitionType(CanonicalFormKind.Any))
+            {
+                return null;
+            }
+
+            implType = implType.GetClosestDefType();
+
+            MethodDesc decl = HandleToObject(baseMethod);
+            Debug.Assert(decl.IsVirtual);
+            Debug.Assert(!decl.HasInstantiation);
+
+            MethodDesc impl;
+
+            TypeDesc declOwningType = decl.OwningType;
+            if (declOwningType.IsInterface)
+            {
+                // Interface call devirtualization.
+
+                if (implType.IsValueType)
+                {
+                    // TODO: this ends up asserting RyuJIT - why?
+                    return null;
+                }
+
+                if (implType.IsCanonicalSubtype(CanonicalFormKind.Any))
+                {
+                    // TODO: attempt to devirtualize methods on canonical interfaces
+                    return null;
+                }
+
+                impl = implType.ResolveInterfaceMethodTarget(decl);
+            }
+            else
+            {
+                impl = implType.GetClosestDefType().FindVirtualFunctionTargetMethodOnObjectType(decl);
+            }
+
+            return impl != null ? ObjectToHandle(impl) : null;
+        }
 
         private void expandRawHandleIntrinsic(ref CORINFO_RESOLVED_TOKEN pResolvedToken, ref CORINFO_GENERICHANDLE_RESULT pResult)
         {
@@ -3019,6 +3075,39 @@ namespace Internal.JitInterface
                 // move that assert to some place later though.
                 targetIsFatFunctionPointer = true;
             }
+            else if ((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_LDFTN) == 0
+                && targetMethod.OwningType.IsInterface)
+            {
+                pResult.kind = CORINFO_CALL_KIND.CORINFO_VIRTUALCALL_STUB;
+
+                if (pResult.exactContextNeedsRuntimeLookup)
+                {
+                    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)
+                    {
+                        pResult.codePointerOrStubLookup.lookupKind.runtimeLookupKind = GetGenericRuntimeLookupKind(contextMethod);
+                        pResult.codePointerOrStubLookup.lookupKind.runtimeLookupFlags = (ushort)ReadyToRunHelperId.VirtualDispatchCell;
+                        pResult.codePointerOrStubLookup.lookupKind.runtimeLookupArgs = (void*)ObjectToHandle(GetRuntimeDeterminedObjectForToken(ref pResolvedToken));
+                    }
+                }
+                else
+                {
+                    pResult.codePointerOrStubLookup.lookupKind.needsRuntimeLookup = false;
+                    pResult.codePointerOrStubLookup.constLookup.accessType = InfoAccessType.IAT_PVALUE;
+                    pResult.codePointerOrStubLookup.constLookup.addr = (void*)ObjectToHandle(_compilation.NodeFactory.InterfaceDispatchCell(targetMethod));
+                }
+            }
+            else if ((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_LDFTN) == 0
+                && _compilation.HasFixedSlotVTable(targetMethod.OwningType))
+            {
+                pResult.kind = CORINFO_CALL_KIND.CORINFO_VIRTUALCALL_VTABLE;
+                pResult.nullInstanceCheck = true;
+            }
             else
             {
                 ReadyToRunHelperId helperId;
diff --git a/src/JitInterface/src/CorInfoTypes.cs b/src/JitInterface/src/CorInfoTypes.cs
index 53bb1f721..513def773 100644
--- a/src/JitInterface/src/CorInfoTypes.cs
+++ b/src/JitInterface/src/CorInfoTypes.cs
@@ -1021,6 +1021,10 @@ namespace Internal.JitInterface
         CORINFO_VIRTUALCALL_VTABLE
     };
 
+    public enum CORINFO_VIRTUALCALL_NO_CHUNK : uint
+    {
+        Value = 0xFFFFFFFF,
+    }
 
     public unsafe struct CORINFO_CALL_INFO
     {
diff --git a/src/System.Private.Jit/src/Internal/Runtime/JitSupport/JitCompilation.cs b/src/System.Private.Jit/src/Internal/Runtime/JitSupport/JitCompilation.cs
index 52bc25701..212aa98b9 100644
--- a/src/System.Private.Jit/src/Internal/Runtime/JitSupport/JitCompilation.cs
+++ b/src/System.Private.Jit/src/Internal/Runtime/JitSupport/JitCompilation.cs
@@ -97,6 +97,11 @@ namespace ILCompiler
             return intrinsicMethod;
         }
 
+        public bool HasFixedSlotVTable(TypeDesc type)
+        {
+            return true;
+        }
+
         public DelegateCreationInfo GetDelegateCtor(TypeDesc delegateType, MethodDesc target, bool followVirtualDispatch)
         {
             return DelegateCreationInfo.Create(delegateType, target, NodeFactory, followVirtualDispatch);
diff --git a/src/System.Private.TypeLoader/src/System.Private.TypeLoader.csproj b/src/System.Private.TypeLoader/src/System.Private.TypeLoader.csproj
index 5491c32c6..fa220b956 100644
--- a/src/System.Private.TypeLoader/src/System.Private.TypeLoader.csproj
+++ b/src/System.Private.TypeLoader/src/System.Private.TypeLoader.csproj
@@ -52,6 +52,7 @@
     <NativeFormatCommonPath>..\..\Common\src\Internal\NativeFormat</NativeFormatCommonPath>
   </PropertyGroup>
   <ItemGroup Condition="'$(JitSupport)' == 'true'">
+    <Compile Include="..\..\Common\src\TypeSystem\Common\CastingHelper.cs" />
     <Compile Include="..\..\Common\src\TypeSystem\RuntimeDetermined\ArrayType.RuntimeDetermined.cs" />
     <Compile Include="..\..\Common\src\TypeSystem\RuntimeDetermined\ByRefType.RuntimeDetermined.cs" />
     <Compile Include="..\..\Common\src\TypeSystem\RuntimeDetermined\DefType.RuntimeDetermined.cs" />
diff --git a/tests/src/Simple/Generics/Generics.cs b/tests/src/Simple/Generics/Generics.cs
index 213400efe..6ad140ae8 100644
--- a/tests/src/Simple/Generics/Generics.cs
+++ b/tests/src/Simple/Generics/Generics.cs
@@ -820,7 +820,8 @@ class Program
                 new DerivedClass2<string>().GVMethod2<string>("string", "string2");
                 new DerivedClass2<string>().GVMethod3<string>("string", "string2");
                 new DerivedClass2<string>().GVMethod4<string>("string", "string2");
-                ((IFace<string>)new BaseClass<string>()).IFaceMethod1("string");
+                Func<IFace<string>> f = () => new BaseClass<string>(); // Hack to prevent devirtualization
+                f().IFaceMethod1("string");
                 ((IFace<string>)new BaseClass<string>()).IFaceGVMethod1<string>("string1", "string2");
 
                 MethodInfo m1 = typeof(BaseClass<string>).GetTypeInfo().GetDeclaredMethod("Method1");