Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GDV: don't emit fallback call if classes are "exact" #87055

Merged
merged 9 commits into from Jun 19, 2023
Merged

GDV: don't emit fallback call if classes are "exact" #87055

merged 9 commits into from Jun 19, 2023

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Jun 2, 2023 •
edited

Contributes to #86769 and closes #86235

using System;
using System.Runtime.CompilerServices;
using System.Threading;

public interface IAnimal
{
    void MakeSound();
}

public class Cat : IAnimal
{
    public void MakeSound() => Console.WriteLine("Meow!");
}

public class Dog : IAnimal
{
    public void MakeSound() => Console.WriteLine("Woof!");
}

public class Cow : IAnimal
{
    public void MakeSound() => Console.WriteLine("Moo!");
}

// This class is not used and will be trimmed out
public class Lion : IAnimal
{
    public void MakeSound() => Console.WriteLine("Roar!");
}

public class Program
{
    public static void Main(string[] args)
    {
        Test(new Cat());
        Test(new Dog());
        Test(new Cow());
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static void Test(IAnimal a) => a.MakeSound();
}

NativeAOT codegen for Test:

; Method Program:Test(IAnimal)
       sub      rsp, 40
       lea      rax, [(reloc 0x4000000000423000)]
       cmp      qword ptr [rcx], rax                         ;; is it 'Cat'?
       jne      SHORT G_M31564_IG04
       lea      rcx, gword ptr [(reloc 0x4000000000423028)]  ;; '"Meow!"'
       call     System.Console:WriteLine(System.String)
       jmp      SHORT G_M31564_IG07
G_M31564_IG04:
       lea      rax, [(reloc 0x4000000000423010)]
       cmp      qword ptr [rcx], rax                         ;; is it 'Dog'?
       jne      SHORT G_M31564_IG06
       lea      rcx, gword ptr [(reloc 0x4000000000422f98)]  ;; '"Woof!"'
       call     System.Console:WriteLine(System.String)
       jmp      SHORT G_M31564_IG07
G_M31564_IG06:
       lea      rcx, gword ptr [(reloc 0x4000000000422f70)]  ;; '"Moo!"'
       call     System.Console:WriteLine(System.String)
G_M31564_IG07:
       nop      
       add      rsp, 40
       ret      
; Total bytes of code: 74

Virtual callback is not emitted since we took care of all possible classes implementing the given interface ("closed world" on NativeAOT). We might still emit it if we e.g. fail to inline one of the candidates (we currently don't do GDV just to devirtualize a call, we only do that when we're able to inline it) -- we might want to do that for cases where interface calls are expensive but that's a different task in #86769

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 2, 2023
@ghost ghost assigned EgorBo Jun 2, 2023
@ghost
Copy link

ghost commented Jun 2, 2023

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

Issue Details 
using System;
using System.Runtime.CompilerServices;
using System.Threading;

public interface IAnimal
{
    void MakeSound();
}

public class Cat : IAnimal
{
    public void MakeSound() => Console.WriteLine("Meow!");
}

public class Dog : IAnimal
{
    public void MakeSound() => Console.WriteLine("Woof!");
}

public class Cow : IAnimal
{
    public void MakeSound() => Console.WriteLine("Moo!");
}

public class Program
{
    public static void Main(string[] args)
    {
        Test(new Cat());
        Test(new Dog());
        Test(new Cow());
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static void Test(IAnimal a) => a.MakeSound();
}

NativeAOT codegen for Test:

; Method Program:Test(IAnimal)
       sub      rsp, 40
       lea      rax, [(reloc 0x4000000000423000)]
       cmp      qword ptr [rcx], rax                         ;; is it 'Cat'?
       jne      SHORT G_M31564_IG04
       lea      rcx, gword ptr [(reloc 0x4000000000423028)]  ;; '"Meow!"'
       call     System.Console:WriteLine(System.String)
       jmp      SHORT G_M31564_IG07
G_M31564_IG04:  ;; offset=001EH
       lea      rax, [(reloc 0x4000000000423010)]
       cmp      qword ptr [rcx], rax                         ;; is it 'Dog'?
       jne      SHORT G_M31564_IG06
       lea      rcx, gword ptr [(reloc 0x4000000000422f98)]  ;; '"Woof!"'
       call     System.Console:WriteLine(System.String)
       jmp      SHORT G_M31564_IG07
G_M31564_IG06:  ;; offset=0038H
       lea      rcx, gword ptr [(reloc 0x4000000000422f70)]  ;; '"Moo!"'
       call     System.Console:WriteLine(System.String)
G_M31564_IG07:  ;; offset=0044H
       nop      
       add      rsp, 40
       ret      
; Total bytes of code: 74

Virtual callback is not emitted since we took care of all possible classes implementing the given interface ("closed world" on NativeAOT). We might still emit it if we e.g. fail to inline one of the candidates (we currently don't do GDV just to devirtualize a call, we only do that when we're able to inline it).

Author: EgorBo
Assignees: -
Labels:

area-CodeGen-coreclr
Milestone: -

@EgorBo EgorBo mentioned this pull request Jun 2, 2023
Open
14 tasks
@EgorBo
Copy link
Member Author

EgorBo commented Jun 2, 2023

For reference, codegen for JIT:

static void Main()
{
    for (int i = 0; i < 200; i++)
    {
        Test(new Cat());
        Test(new Dog());
        Test(new Cow());
        Thread.Sleep(16);
    }
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void Test(IAnimal a) => a.MakeSound();
; Assembly listing for method Program:Test(IAnimal)
       push     rsi
       sub      rsp, 32
       mov      rsi, qword ptr [rcx]
       mov      rax, 0x7FFF53C41C88      ; Cat
       cmp      rsi, rax
       jne      SHORT G_M31564_IG04
       mov      rcx, 0x26780208F80      ; 'Meow!'
       call     [System.Console:WriteLine(System.String)]
       jmp      SHORT G_M31564_IG07
G_M31564_IG04:
       mov      rax, 0x7FFF53C41E48      ; Dog
       cmp      rsi, rax
       jne      SHORT G_M31564_IG06
       mov      rcx, 0x26780209128      ; 'Woof!'
       call     [System.Console:WriteLine(System.String)]
       jmp      SHORT G_M31564_IG07
G_M31564_IG06:
       mov      rax, 0x7FFF53C42008      ; Cow
       cmp      rsi, rax
       jne      SHORT G_M31564_IG09
       mov      rcx, 0x26780209148      ; 'Moo!'
       call     [System.Console:WriteLine(System.String)]
G_M31564_IG07:
       nop      
       add      rsp, 32
       pop      rsi
       ret      
G_M31564_IG09:
       mov      r11, 0x7FFF53130280
       call     [r11]IAnimal:MakeSound():this
       jmp      SHORT G_M31564_IG07

(PGO-driven)

@EgorBo
Copy link
Member Author

EgorBo commented Jun 2, 2023

/azp run runtime-extra-platforms, runtime-coreclr pgo

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@EgorBo
Copy link
Member Author

EgorBo commented Jun 2, 2023

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 2, 2023
Closed
Open
@EgorBo
Copy link
Member Author

EgorBo commented Jun 3, 2023

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@EgorBo EgorBo marked this pull request as draft June 4, 2023 00:30
@EgorBo
Copy link
Member Author

EgorBo commented Jun 4, 2023

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@EgorBo
Copy link
Member Author

EgorBo commented Jun 8, 2023

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 9, 2023
Closed
Closed
Open
Closed
Closed
Closed
@build-analysis build-analysis bot mentioned this pull request Jun 9, 2023
Closed
@EgorBo
Copy link
Member Author

EgorBo commented Jun 10, 2023

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@EgorBo
Copy link
Member Author

EgorBo commented Jun 10, 2023

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@EgorBo
Copy link
Member Author

EgorBo commented Jun 11, 2023

@AndyAyersMS PTAL, it passes NativeAOT outerloop tests + 3 type checks

@EgorBo EgorBo requested a review from AndyAyersMS June 11, 2023 10:34
@EgorBo EgorBo marked this pull request as ready for review June 11, 2023 11:40
EgorBo
EgorBo commented Jun 11, 2023
View reviewed changes
src/coreclr/jit/likelyclass.cpp
@@ -268,7 +273,7 @@ static unsigned getLikelyClassesOrMethods(LikelyClassMethodRecord*

// Distribute the rounding error and just apply it to the first entry.
// Assume that there is no error If we have unknown handles.
if (numberOfClasses == h.m_totalCount)
if (!containsUnknownHandles)
Copy link
Member Author

@EgorBo EgorBo Jun 11, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an unrelated change, it's just that I used a wrong check in #86965 so it didn't work properly

@EgorBo EgorBo mentioned this pull request Jun 11, 2023
Merged
@EgorBo EgorBo merged commit ce61999 into dotnet:main Jun 19, 2023
127 checks passed
@EgorBo EgorBo deleted the gdv-omit-fallback branch June 19, 2023 07:50
@dotnet dotnet locked as resolved and limited conversation to collaborators Jul 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@AndyAyersMS AndyAyersMS AndyAyersMS approved these changes

Assignees

@EgorBo EgorBo

Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Guarded devirtualization based on whole program view
2 participants
@EgorBo @AndyAyersMS