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.

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

Improve getExactClasses to support classes as base types #92440

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

MichalStrehovsky
Copy link
Member

@MichalStrehovsky MichalStrehovsky commented Sep 21, 2023

Fixes #88547

using System.Runtime.CompilerServices;

public class ClassA
{
    public virtual int GetValue() => 42;
}

public class ClassB : ClassA
{
    // we don't even need to override GetValue here
}

class MyClass
{
    static void Main()
    {
        Test(new ClassB());
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int Test(ClassA c) => c.GetValue();
}

Old codegen:

; Method MyClass:Test(ClassA):int (FullOpts)
       sub      rsp, 40
       mov      rax, qword ptr [rcx]
       call     [rax+30H]ClassA:GetValue():int:this
       nop
       add      rsp, 40
       ret
; Total bytes of code: 16

New codegen:

00007FF66DD8AFB0  cmp         dword ptr [rcx],ecx
00007FF66DD8AFB2  mov         eax,2Ah
00007FF66DD8AFB7  ret

Cc @dotnet/ilc-contrib @EgorBo

Fixes dotnet#88547

```csharp
using System.Runtime.CompilerServices;

public class ClassA
{
    public virtual int GetValue() => 42;
}

public class ClassB : ClassA
{
    // we don't even need to override GetValue here
}

class MyClass
{
    static void Main()
    {
        Test(new ClassB());
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int Test(ClassA c) => c.GetValue();
}
```

Old codegen:

```
; Method MyClass:Test(ClassA):int (FullOpts)
       sub      rsp, 40
       mov      rax, qword ptr [rcx]
       call     [rax+30H]ClassA:GetValue():int:this
       nop
       add      rsp, 40
       ret
; Total bytes of code: 16
```

New codegen:

```
00007FF66DD8AFB0  cmp         dword ptr [rcx],ecx
00007FF66DD8AFB2  mov         eax,2Ah
00007FF66DD8AFB7  ret
```
@ghost
Copy link

ghost commented Sep 21, 2023

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

Issue Details

Fixes #88547

using System.Runtime.CompilerServices;

public class ClassA
{
    public virtual int GetValue() => 42;
}

public class ClassB : ClassA
{
    // we don't even need to override GetValue here
}

class MyClass
{
    static void Main()
    {
        Test(new ClassB());
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static int Test(ClassA c) => c.GetValue();
}

Old codegen:

; Method MyClass:Test(ClassA):int (FullOpts)
       sub      rsp, 40
       mov      rax, qword ptr [rcx]
       call     [rax+30H]ClassA:GetValue():int:this
       nop
       add      rsp, 40
       ret
; Total bytes of code: 16

New codegen:

00007FF66DD8AFB0  cmp         dword ptr [rcx],ecx
00007FF66DD8AFB2  mov         eax,2Ah
00007FF66DD8AFB7  ret

Cc @dotnet/ilc-contrib @EgorBo

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

@MichalStrehovsky
Copy link
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@EgorBo
Copy link
Member

EgorBo commented Sep 22, 2023

Love it! It should also pick up other jit opts like if (obj is Class1) or multiple-guesses GDV, etc which currently only kick in for interfaces. LGTM if CI passes 🙂

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks

@jkotas jkotas merged commit e0a4bdd into dotnet:main Sep 22, 2023
121 of 123 checks passed
@MichalStrehovsky MichalStrehovsky deleted the moredevirt branch September 22, 2023 07:03
@ghost ghost locked as resolved and limited conversation to collaborators Oct 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NativeAOT: improve getExactClasses to support classes as base types
3 participants