NativeAOT: Guarded devirtualization #64497

EgorBo · 2022-01-29T21:02:37Z

Contributes to #64242

This PR introduces getExactClasses JIT-EE API, VM may give JIT a hint that a specific virtual call will always be of one of the specific classes and no other class can be loaded/added so we can skip "fallback" path.

Current implementation only handles single-impl/subclass/struct case, will be extended to multiple with #59604

Example:

public interface IFoo
{
    void SayHello();
}

public class FooImpl : IFoo
{
    public void SayHello() => Console.WriteLine("Hello!");
}

class Test
{
    static void Main()
    {
        Test(new FooImpl());
        Console.ReadKey();
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static void Test(IFoo f) => f.SayHello();
}

Codegen for Test:

; Method MyType:Test(IFoo)
G_M19269_IG01:
       sub      rsp, 40
G_M19269_IG02:
       cmp      dword ptr [rcx], ecx
       mov      rcx, 0xD1FFAB1E      ; "Hello!"
       mov      rcx, gword ptr [rcx]
       call     System.Console:WriteLine(System.String) ; <-- SayHello was devirtualized & inlined
       nop      ; <-- no fallbacks as with PGO on CoreCLR
G_M19269_IG03:
       add      rsp, 40
       ret      
; Total bytes of code: 30

(I was using JIT for demo, getExactClasses is not implemented on VM side yet, @MichalStrehovsky any pointers how to get all possible implementations/subclasses for a specific type? (see https://github.com/dotnet/runtime/pull/64497/files#diff-132a77bcd3f74cf0e0b04fbccda246c97c91e40562d78cb01fff61cf69403573R2930)

TODO:

Implement VM's side
Remove "must be inlineable" requirement for GDV candidates

ghost · 2022-01-29T21:02:44Z

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR introduces getExactClasses JIT-EE API, VM may give JIT a hint that a specific virtual call will always be of one of the specific classes and no other class can be loaded/added so we can skip "fallback" path.

Current implementation only handles single-impl/subclasses/class case, will be extended to multiple with #59604

Example:

public interface IFoo
{
    void SayHello();
}

public class FooImpl : IFoo
{
    public void SayHello() => Console.WriteLine("Hello!");
}

class Test
{
    static void Main()
    {
        Test(new FooImpl());
        Console.ReadKey();
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static void Test(IFoo f) => f.SayHello();
}

Codegen for Test:

; Method MyType:Test(IFoo)
G_M19269_IG01:
       sub      rsp, 40
G_M19269_IG02:
       cmp      dword ptr [rcx], ecx
       mov      rcx, 0xD1FFAB1E      ; "Hello!"
       mov      rcx, gword ptr [rcx]
       call     System.Console:WriteLine(System.String) ; <-- SayHello was devirtualized & inlined
       nop      ; <-- no fallbacks as with PGO on CoreCLR
G_M19269_IG03:
       add      rsp, 40
       ret      
; Total bytes of code: 30

(I was using JIT for demo, getExactClasses is not implemented on VM side yet, @MichalStrehovsky any pointers how to get all possible implementations/subclasses for a specific type? (see )

Author:	EgorBo
Assignees:	EgorBo
Labels:	`area-CodeGen-coreclr`
Milestone:	-

MichalStrehovsky · 2022-01-31T00:11:21Z

Spicy pull request!

@MichalStrehovsky any pointers how to get all possible implementations/subclasses for a specific type?

Here's a blueprint of what should work:

https://github.com/dotnet/runtime/blob/bc6d349ecd72eec162f0532e6b6218fdbaa07ddb/src/coreclr/tools/Common/Compiler/DevirtualizationManager.cs - add a new virtual method to this, say public virtual TypeDesc[] GetImplementingClasses(TypeDesc type) => null.
The interesting override of the virtual method will be in this class:

runtime/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs

Line 365 in bc6d349

private class ScannedDevirtualizationManager : DevirtualizationManager
In the constructor of the class we already do a foreach over all the types that we've seen as instantiated. Filter out the abstract ones (the constructor already checks for that in a spot) and put them into some data structure based on their .RuntimeInterfaces and .BaseType.
Then override GetImplementingClasses and give answers based on the data collected in the constructor.
Add a method to the Compilation class that forwards to DevirtualizationManager (available as _devirtualizationManager):

runtime/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs

Line 23 in bc6d349

public abstract class Compilation : ICompilation
_compilation is available in the JitInterface implementation, so you can just call that and give the result to RyuJIT.

We'll have to be a little careful about generic classes/interfaces because new instantiations of those can happen at runtime (e.g. if we compiled/saw List<__Canon>/List<string>, we could still end up seeing a List<object> at runtime because they can be created after compilation with MakeGenericType). Maybe for now it might be easiest to disallow generics - if an interface/base class (even non-generic) is implemented by a generic type that has a shared form, forget about devirtualizing the interface/base type.

…iveaot # Conflicts: # src/coreclr/inc/jiteeversionguid.h # src/coreclr/tools/Common/JitInterface/CorInfoBase.cs # src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs # src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h # src/coreclr/vm/jitinterface.cpp

…iveaot

EgorBo · 2022-03-28T10:38:04Z

Ping myself

kant2002 · 2022-04-22T05:15:48Z

@EgorBo friendly ping.

MichalStrehovsky · 2022-07-15T21:36:02Z

Is the remaining work on the guarded devirtualization on the VM side or jit side? I could help pick up the VM side. This would be a very cool thing to have

EgorBo · 2022-07-16T13:29:01Z

@MichalStrehovsky @kant2002 ah, completely forgot about this one - I'll finish it tomorrow 🙂

# Conflicts: # src/coreclr/inc/jiteeversionguid.h # src/coreclr/jit/importer.cpp # src/coreclr/tools/Common/JitInterface/CorInfoBase.cs # src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs # src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs

EgorBo · 2022-08-12T12:01:41Z

@MichalStrehovsky could you please help me to figure out how to incorporate changes from #73218 (I fixed the merge conflict with "using mine")

MichalStrehovsky · 2022-08-12T12:09:15Z

@MichalStrehovsky could you please help me to figure out how to incorporate changes from #73218 (I fixed the merge conflict with "using mine")

Another conflict is incoming in #73839 and that one is a fix for a blocking bug. We can try after

MichalStrehovsky · 2022-08-12T22:52:30Z

/azp run runtime-extra-platforms

azure-pipelines · 2022-08-12T22:52:52Z

Azure Pipelines successfully started running 1 pipeline(s).

MichalStrehovsky · 2022-08-13T05:31:18Z

The NativeAOT runs look good. The failures exist in the baseline, but if I don't baseline them apparently nobody else will... (I'm looking into baselining them)

MichalStrehovsky · 2022-08-15T00:42:11Z

src/coreclr/jit/lclvars.cpp

@@ -3174,6 +3174,16 @@ void Compiler::lvaSetClass(unsigned varNum, CORINFO_CLASS_HANDLE clsHnd, bool is
        return;
    }

+    if (clsHnd != NO_CLASS_HANDLE && !isExact && IsTargetAbi(CORINFO_NATIVEAOT_ABI))


Is there a meaningful perf saving from skipping getExactClasses call outside NATIVEAOT_ABI? I assume it would just be a no-op. Not doing this outside NativeAOT is a policy decision and JIT generally tries to leave those to the EE side.

Could we instead key this off of JitEnableDevirtualization (or maybe a new config switch specifically for this)? Just so we have some way to see the impact of this or disable it if it misbehaves. On ilc side we have --noscan that would disable this, but it also disables a bunch of other things because it's for the whole program analysis in general.

jakobbotsch

LGTM, are there any concerns about a JIT-EE GUID change this close to the snap if we take it?

EgorBo · 2022-08-15T18:32:46Z

LGTM, are there any concerns about a JIT-EE GUID change this close to the snap if we take it?

I personally don't see any

Merging now, CI is green (except the spmi jobs which fail due to jit-ee guid update)

BruceForstall · 2022-08-15T18:38:27Z

I personally don't see any

Any further "last minute" .NET 7 PRs will fail SPMI jobs until the collection happens, which will complete after the fork.

EgorBo · 2022-08-15T18:42:13Z

I personally don't see any

Any further "last minute" .NET 7 PRs will fail SPMI jobs until the collection happens, which will complete after the fork.

Right, which is why merging this last minute is fine, new PRs which will be rebased won't make it to green anyway (20 minutes left) - am I right?

AndyAyersMS · 2022-08-15T18:48:20Z

Snap is at 4pm PDT, so there are 4+ hours left....

EgorBo · 2022-08-15T18:56:50Z

Snap is at 4pm PDT, so there are 4+ hours left....

Oops, I thought it's noon 🙁

TIHan · 2022-08-16T23:45:33Z

@EgorBo I think this change is causing SuperPMI collection to fail even on a simple "hello world" app - GetExactClasses is null when it is trying to be used in recGetExactClasses.

EgorBo added 2 commits January 29, 2022 23:40

Initial implementation

17c3b8c

basic getExactClasses impl

e0a25e0

EgorBo requested a review from MichalStrehovsky as a code owner January 29, 2022 21:02

ghost assigned EgorBo Jan 29, 2022

dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jan 29, 2022

EgorBo added 9 commits February 25, 2022 13:18

fix merge conflicts

9643359

fix merge conflicts

10ed069

fix merge conflicts

84b0b50

fix merge conflicts

ee67913

Implement GetImplementingClasses

07f053e

Merge branch 'main' of https://github.com/dotnet/runtime into gdv-nat…

29c0f93

…iveaot

Make checks more conservative

ec97ddb

Another attempt

0685c07

EgorBo added 4 commits July 18, 2022 01:02

update guid

cda167d

Merge branch 'main' of github.com:dotnet/runtime into gdv-nativeaot

527b302

Fix importer logic

0e8adab

runfoapp bot mentioned this pull request Jul 19, 2022

system.net.quic.functional.tests failing with stack buffer overflow #72429

Closed

Update importer.cpp

a1617c6

runfoapp bot mentioned this pull request Jul 19, 2022

System.Net.Http.Functional.Tests.SyncHttpHandler_DiagnosticsTest.SendAsync_SuppressedGlobalStaticPropagationEnvVar failing in CI #72428

Closed

EgorBo added 3 commits July 21, 2022 15:14

Merge branch 'main' of github.com:dotnet/runtime into gdv-nativeaot

9ada528

Don't use GDV

46f84a5

Clean up

f87300f

EgorBo added 4 commits August 12, 2022 12:58

Merge branch 'main' into gdv-nativeaot

19d6690

Merge branch 'main' of github.com:dotnet/runtime into gdv-nativeaot

e19f094

address feedback

7559015

fix build

e4eca63

MichalStrehovsky added 3 commits August 13, 2022 06:00

Merge branch 'main' into gdv-nativeaot

cb74fc9

Restore old logic

d170052

One more lost diff

6bf87db

MichalStrehovsky reviewed Aug 15, 2022

View reviewed changes

EgorBo added 3 commits August 15, 2022 18:26

Merge branch 'main' of github.com:dotnet/runtime into gdv-nativeaot

6728e6a

add JitEnableExactDevirtualization

6cd67e6

fix build

292d1d8

JulieLeeMSFT added this to the 7.0.0 milestone Aug 15, 2022

jakobbotsch approved these changes Aug 15, 2022

View reviewed changes

EgorBo merged commit a85b782 into dotnet:main Aug 15, 2022

EgorBo deleted the gdv-nativeaot branch August 15, 2022 18:42

MichalStrehovsky mentioned this pull request Aug 16, 2022

NativeAOT codegen optimization opportunities #64242

Closed

15 tasks

TIHan mentioned this pull request Aug 17, 2022

SuperPMI collection fix by instantiating 'GetExactClasses' if it is null #74037

Merged

AndyAyersMS mentioned this pull request Aug 18, 2022

Convert more jit phases to opt into common post phase checks #74041

Merged

ghost locked as resolved and limited conversation to collaborators Sep 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NativeAOT: Guarded devirtualization #64497

NativeAOT: Guarded devirtualization #64497

EgorBo commented Jan 29, 2022 •

edited

Loading

ghost commented Jan 29, 2022

MichalStrehovsky commented Jan 31, 2022

EgorBo commented Mar 28, 2022

kant2002 commented Apr 22, 2022

MichalStrehovsky commented Jul 15, 2022

EgorBo commented Jul 16, 2022

EgorBo commented Aug 12, 2022

MichalStrehovsky commented Aug 12, 2022

MichalStrehovsky commented Aug 12, 2022

azure-pipelines bot commented Aug 12, 2022

MichalStrehovsky commented Aug 13, 2022

MichalStrehovsky Aug 15, 2022

jakobbotsch left a comment

EgorBo commented Aug 15, 2022

BruceForstall commented Aug 15, 2022

EgorBo commented Aug 15, 2022 •

edited

Loading

AndyAyersMS commented Aug 15, 2022

EgorBo commented Aug 15, 2022

TIHan commented Aug 16, 2022

NativeAOT: Guarded devirtualization #64497

NativeAOT: Guarded devirtualization #64497

Conversation

EgorBo commented Jan 29, 2022 • edited Loading

ghost commented Jan 29, 2022

MichalStrehovsky commented Jan 31, 2022

EgorBo commented Mar 28, 2022

kant2002 commented Apr 22, 2022

MichalStrehovsky commented Jul 15, 2022

EgorBo commented Jul 16, 2022

EgorBo commented Aug 12, 2022

MichalStrehovsky commented Aug 12, 2022

MichalStrehovsky commented Aug 12, 2022

azure-pipelines bot commented Aug 12, 2022

MichalStrehovsky commented Aug 13, 2022

MichalStrehovsky Aug 15, 2022

Choose a reason for hiding this comment

jakobbotsch left a comment

Choose a reason for hiding this comment

EgorBo commented Aug 15, 2022

BruceForstall commented Aug 15, 2022

EgorBo commented Aug 15, 2022 • edited Loading

AndyAyersMS commented Aug 15, 2022

EgorBo commented Aug 15, 2022

TIHan commented Aug 16, 2022

EgorBo commented Jan 29, 2022 •

edited

Loading

EgorBo commented Aug 15, 2022 •

edited

Loading