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

Inliner: don't give up on virtual calls if we can clarify their exact targets after inlining #85209

Open
EgorBo opened this issue Apr 23, 2023 · 2 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@EgorBo
Copy link
Member

EgorBo commented Apr 23, 2023

class ClassA
{
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public virtual void SayHello() => Console.WriteLine("Hello");
}

sealed class ClassB : ClassA {}

// Signature is abstract, but it always returns an exact class
ClassA GetClassA() => new ClassB();

void Test()
{
    GetClassA().SayHello(); // will SayHello() be inlined here?
}

Current codegen for Test():

; Assembly listing for method Prog:Test():this
       sub      rsp, 40
       mov      rcx, 0x7FF8DBB2FD60      ; Prog+ClassB
       call     CORINFO_HELP_NEWSFAST
       mov      rcx, rax
       call     [Prog+ClassA:SayHello():this]
       nop      
       add      rsp, 40
       ret      
; Total bytes of code 34

SayHello is not inlined despite being devirtualized and marked as AggressiveInlining. This demonstrates a fundamental problem in the JIT's inliner - it does inlining in, roughly, two stages: 1) Traverse all calls and set inline candidates + spill candidates to be top-level statements. 2) Inline all candidates we previosly found. During the first stage we give up on SayHello because the target is abstract (ClassA) - we don't know yet that if we inline GetClassA we'll get the exact target (ClassB). What is funny that Dynamic PGO can help us here - it will try to inline it as ClassB under a guard (GDV) and then we'll fold the guard once we clarify the target (#84661), so the same codegen when PGO is enabled looks like this:

; Assembly listing for method Prog:Test():this
       sub      rsp, 40
       mov      rcx, 0x23780209218      ; 'Hello'
       call     [System.Console:WriteLine(System.String)]
       nop      
       add      rsp, 40
       ret      
; Total bytes of code 26

No guards, call is inlined!

We should be able to do the same without PGO too (e.g. for NativeAOT). This limitation is the exact reason why APIs like Encoding.UTF8.X, Encoding.ASCII.X, ArrayPool.Shared.X etc are never inlined without PGO.

cc @AndyAyersMS (we discussed this case today in Discord)

@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 Apr 23, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Apr 23, 2023
@ghost
Copy link

ghost commented Apr 23, 2023

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

Issue Details
class ClassA
{
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public virtual void SayHello() => Console.WriteLine("Hello");
}

sealed class ClassB : ClassA {}

// Signature is abstract, but it always returns an exact class
ClassA GetClassA() => new ClassB();

void Test()
{
    GetClassA().SayHello(); // will SayHello() be inlined here?
}

Current codegen for Test():

; Assembly listing for method Prog:Test():this
       sub      rsp, 40
       mov      rcx, 0x7FF8DBB2FD60      ; Prog+ClassB
       call     CORINFO_HELP_NEWSFAST
       mov      rcx, rax
       call     [Prog+ClassA:SayHello():this]
       nop      
       add      rsp, 40
       ret      
; Total bytes of code 34

SayHello is not inlined despite being devirtualized and marked as AggressiveInlining. This demonstrates a fundamental problem in the JIT's inliner - it does inlining in, roughly, two stages: 1) Traverse all calls and set inline candidates + spill candidates to be top-level statements. 2) Inline all candidates we previosly recorded. During the first stage we give up on SayHello because the target is abstract (ClassA) - we don't know yet that if we inline GetClassA we'll get the exact target (ClassB). What is funny that Dynamic PGO can help us here - it will try to inline it as ClassB under a guard (GDV) and then we'll fold the guard once we clarify the target (#84661), so the same codegen when PGO is enabled looks like this:

; Assembly listing for method Prog:Test():this
       sub      rsp, 40
       mov      rcx, 0x23780209218      ; 'Hello'
       call     [System.Console:WriteLine(System.String)]
       nop      
       add      rsp, 40
       ret      
; Total bytes of code 26

No guards, call is inlined!

We should be able to do the same without PGO too (e.g. for NativeAOT). This limitation is the exact reason why APIs like Encoding.UTF8.X, Encoding.ASCII.X, ArrayPool.Shared.X etc are never inlined without PGO.

cc @AndyAyersMS

Author: EgorBo
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@EgorBo EgorBo removed the untriaged New issue has not been triaged by the area owner label Apr 23, 2023
@EgorBo EgorBo added this to the Future milestone Apr 23, 2023
@MichalPetryka
Copy link
Contributor

This also affects #85197.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

No branches or pull requests

2 participants