Skip to content
New issue

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

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

Already on GitHub? # to your account

Multiple GDV: ignore non-inlineable candidates #86835

Merged
merged 2 commits into from
May 27, 2023

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented May 27, 2023

Contributes to #86769

The current logic used to give up on all of them if at least one of the candidates was non-inlineable (for any reason)
E.g.:

public interface IValue
{
    int GetValue();
}

public class MyClass1 : IValue
{
    public int GetValue() => 10;
}

public class MyClass2 : IValue
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    public int GetValue() => 50;
}

public class MyClass3 : IValue
{
    public int GetValue() => 100;
}

Was:

; Assembly listing for method Program:Test(IValue):int
       sub      rsp, 40
       mov      r11, 0x7FFBC9420318
       call     [r11]IValue:GetValue():int:this
       nop      
       add      rsp, 40
       ret      
; Total bytes of code 23

Now:

; Assembly listing for method Program:Test(IValue):int
       sub      rsp, 40
       mov      r11, 0x7FFBC9FE8B40
       cmp      qword ptr [rcx], r11       ;; is it MyClass1 ?
       jne      SHORT G_M7592_IG04
       mov      eax, 10
       jmp      SHORT G_M7592_IG07
G_M7592_IG04:
       mov      rax, 0x7FFBC9FE8ED0
       cmp      qword ptr [rcx], rax       ;; is it MyClass3 ?
       jne      SHORT G_M7592_IG06
       mov      eax, 100
       jmp      SHORT G_M7592_IG07
G_M7592_IG06:
       mov      r11, 0x7FFBC9400318
       call     [r11]IValue:GetValue():int:this  ;; no longer cold, we have MyClass2
G_M7592_IG07:
       nop      
       add      rsp, 40
       ret      
; Total bytes of code 67

@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 May 27, 2023
@ghost ghost assigned EgorBo May 27, 2023
@ghost
Copy link

ghost commented May 27, 2023

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

Issue Details

Contributes to #86769

The current logic used to give on all of them if at least one of the candidates were non inlineable (for any reason)
E.g.:

public interface IValue
{
    int GetValue();
}

public class MyClass1 : IValue
{
    public int GetValue() => 10;
}

public class MyClass2 : IValue
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    public int GetValue() => 50;
}

public class MyClass3 : IValue
{
    public int GetValue() => 100;
}
Author: EgorBo
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@EgorBo EgorBo changed the title Multiple GDV: don't give up if one of the candidates are non-inlineable Multiple GDV: ignore non-inlineable candidates May 27, 2023
@EgorBo EgorBo requested a review from AndyAyersMS May 27, 2023 13:38
@EgorBo
Copy link
Member Author

EgorBo commented May 27, 2023

@AndyAyersMS PTAL

@EgorBo
Copy link
Member Author

EgorBo commented May 27, 2023

@AndyAyersMS Thanks! Good idea, not sure why I decided to introduce those struct copies. Addressed via 2d3395e - it also simplified the code.

Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

Looks good

@EgorBo EgorBo merged commit d58754b into dotnet:main May 27, 2023
@EgorBo EgorBo deleted the gdv-dont-ignore-failed-inlines branch May 27, 2023 18:38
@ghost ghost locked as resolved and limited conversation to collaborators Jun 26, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants