Interface dispatch support #626

nattress · 2016-01-12T00:59:49Z

Emit the supporting data structures for interface dispatch:

Interface dispatch cell for each call site providing the resolution function entry point as well as the interface type and interface method slot number
Interface map on EETypes specifying the list of interfaces they implement
Dispatch map that provides, for each type, the lookup rows for interface type, interface method slot, and implementing method's VTable slot
Dispatch map table that provides dispatch map pointers (the index into this table is stored on an EEType instead of a pointer to the map for space savings)

Added a work-around in the runtime to provide access to the dispatch map table until full module headers are implemented

nattress · 2016-01-12T01:02:16Z

@jkotas @davidwrighton @smosier Please take a look. David, I removed a couple type system asserts in TypeSystemContext::GetMethodForInstantiatedType. For interface method impls, they had instantiations in my testing and removing the asserts seemed to work fine :)

Scott: I added you to make sure you're okay with my temporary workaround for no ModuleHeader support. I plumbed the dispatch map table in via a RuntimeInstance helper.

jkotas · 2016-01-12T04:14:34Z

src/ILCompiler/repro/Program.cs

@@ -7,7 +7,76 @@ internal class Program
 {
    private static void Main(string[] args)
    {
-        Console.WriteLine("Hello world");
+        MyInterface i1 = new Foo(false, 1, "Foo implementation") as MyInterface;


I would keep the Program.cs in repro as is. Instead, we should add a new smoke test under tests\src\simple that has what you got here.

jkotas · 2016-01-12T04:29:21Z

For Unix, could you please just fix the build breaks (e.g. by adding dummy methods to portable.cpp)? We can deal with porting the assembly code in separate change.

jkotas · 2016-01-12T04:29:31Z

LGTM otherwise

MichalStrehovsky · 2016-01-12T15:39:56Z

src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/EETypeNode.cs

        public override ObjectData GetData(NodeFactory factory, bool relocsOnly)
        {
            ObjectDataBuilder objData = new ObjectDataBuilder(factory);
            objData.Alignment = 16;
            objData.DefinedSymbols.Add(this);
-
+            


Nit: can you do a sweep of all the whitespace you're checking in?

nattress · 2016-01-13T01:58:04Z

@jkotas @MichalStrehovsky @davidwrighton Updated based on your feedback.

Also, I split TypeDesc.MethodImpls.cs into two (based on the type names stored in it): MetadataType.MethodImpls.cs, InstantiatedType.MethodImpls.cs

I added a stub .S for the cross-plat assembly code needed for dispatch. Portable.cpp should already have the missing entry points (we'll see when CI finishes).

manu-st · 2016-01-13T02:10:39Z

@nattress Could you add stub for Linux arm and arm64 as well?

jkotas · 2016-01-13T02:21:39Z

split TypeDesc.MethodImpls.cs into two (based on the type names stored in it)

The split files need to be in the common directory - you have added them to the compiler directory.

We have been going back and forth on whether the files should be split for cases like this or not. I believe that we have been leaning towards keeping them together. Splitting these into multiple files rigorously leads to way too many little files.

jkotas · 2016-01-13T02:25:43Z

src/Native/Runtime/CachedInterfaceDispatch.cpp

@@ -545,6 +546,10 @@ COOP_PINVOKE_HELPER(PTR_Code, RhpResolveInterfaceMethodCacheMiss, (Object * pObj
                                                                   PInvokeTransitionFrame * pTransitionFrame))
 {
    CID_COUNTER_INC(CacheMisses);
+#ifdef CORERT
+    return RhpCidResolve(pObject, pCell);


This needs @TODO: CORERT: Use ManagedCallout2.

MichalStrehovsky · 2016-01-13T18:20:49Z

src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/EETypeNode.cs

+
+                    foreach (MethodDesc interfaceMethod in interfaceType.GetMethods())
+                    {
+                        Debug.Assert(interfaceMethod.IsVirtual);


Interfaces are allowed to have static methods. Might need to skip them here.

Instead of asserting the method is virtual, we should probably throw (unless this is guarded in some other place).

Emit the supporting data structures for interface dispatch: - Interface dispatch cell for each call site providing the resolution function entry point as well as the interface type and interface method slot number - Interface map on EETypes specifying the list of interfaces they implement - Dispatch map that provides, for each type, the lookup rows for interface type, interface method slot, and implementing method's VTable slot - Dispatch map table that provides dispatch map pointers (the index into this table is stored on an EEType instead of a pointer to the map for space savings) Added a work-around in the runtime to provide access to the dispatch map table until full module headers are implemented

Interface dispatch support

When interface dispatch was initially implemented in CoreRT, we took a number of shortcuts (#626). Skipping the size on disk optimization around generating runs of interface dispatch cells with the same slot number was one of the shortcuts (each cell represented its own run). With this, I'm implementing the same optimization that binder does. For PureNative shared library, this saves 100 kB. I didn't measure savings for apps, but I expect them to be similar. [tfs-changeset: 1708487]

dnfclas added the cla-already-signed label Jan 12, 2016

jkotas reviewed Jan 12, 2016
View reviewed changes

MichalStrehovsky reviewed Jan 12, 2016
View reviewed changes

nattress force-pushed the interface_dispatch branch from da170fc to 83fd293 Compare January 13, 2016 01:54

jkotas reviewed Jan 13, 2016
View reviewed changes

nattress force-pushed the interface_dispatch branch 2 times, most recently from d0bb358 to 0ffd71f Compare January 13, 2016 03:11

MichalStrehovsky reviewed Jan 13, 2016
View reviewed changes

nattress force-pushed the interface_dispatch branch from 0ffd71f to 69d12d9 Compare January 14, 2016 02:13

nattress added a commit that referenced this pull request Jan 14, 2016

Merge pull request #626 from nattress/interface_dispatch

811a323

Interface dispatch support

nattress merged commit 811a323 into dotnet:master Jan 14, 2016

nattress deleted the interface_dispatch branch January 14, 2016 03:04

MichalStrehovsky mentioned this pull request Feb 24, 2022

Reduce alignment of dispatch maps dotnet/runtime#65823

Merged

MichalStrehovsky mentioned this pull request Oct 16, 2023

Fix analysis of interface methods on generic types dotnet/runtime#93540

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Interface dispatch support #626

Interface dispatch support #626

nattress commented Jan 12, 2016

nattress commented Jan 12, 2016

jkotas Jan 12, 2016

jkotas commented Jan 12, 2016

jkotas commented Jan 12, 2016

MichalStrehovsky Jan 12, 2016

nattress commented Jan 13, 2016

manu-st commented Jan 13, 2016

jkotas commented Jan 13, 2016

jkotas Jan 13, 2016

MichalStrehovsky Jan 13, 2016

MichalStrehovsky Jan 13, 2016

Interface dispatch support #626

Interface dispatch support #626

Conversation

nattress commented Jan 12, 2016

nattress commented Jan 12, 2016

jkotas Jan 12, 2016

Choose a reason for hiding this comment

jkotas commented Jan 12, 2016

jkotas commented Jan 12, 2016

MichalStrehovsky Jan 12, 2016

Choose a reason for hiding this comment

nattress commented Jan 13, 2016

manu-st commented Jan 13, 2016

jkotas commented Jan 13, 2016

jkotas Jan 13, 2016

Choose a reason for hiding this comment

MichalStrehovsky Jan 13, 2016

Choose a reason for hiding this comment

MichalStrehovsky Jan 13, 2016

Choose a reason for hiding this comment