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

Methods with struct parameters are not inlined #53783

Open
atynagano opened this issue Jun 6, 2021 · 3 comments
Open

Methods with struct parameters are not inlined #53783

atynagano opened this issue Jun 6, 2021 · 3 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@atynagano
Copy link

atynagano commented Jun 6, 2021

Description

Based on x64 in sharplab:

https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWwB8ABABgAJiBGAOgCUBXAOwwEt8YaBhCfAB1YAbGFADKIgG6swMXAG4AsAChlxAEzku5AN7Ly5PfsopyAMSoAKAGaCI2DOSsBKcgF4AfGbXWnipUeMva1t7RxcPMwBmHz8A4hNTaJs7B2c3TwBZGIDDfVzyAG0MmAwACwgAEwBJfkELYrLKmr5BAHk+NggmXBoAOQgqpkFWJhGAcycAXXz48izk0LTtcgBfZRWgA=

The following:

using System.Runtime.CompilerServices;

class C {
  
    void F1(float f) => F2(f);
    void F2(float f) => F3(f);
    void F3(float f) => M(f);    
    
    [MethodImpl(MethodImplOptions.NoInlining)]
    void M(float f) { }
}

is compiled as:

; Core CLR v5.0.621.22011 on amd64

C..ctor()
    L0000: ret

C.F1(Single)
    L0000: vzeroupper
    L0003: jmp C.M(Single)

C.F2(Single)
    L0000: vzeroupper
    L0003: jmp C.M(Single)

C.F3(Single)
    L0000: vzeroupper
    L0003: jmp C.M(Single)

C.M(Single)
    L0000: ret

Methods with primitive parameters are inlined, and F1, F2, and F3 output the same code.

https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWwB8ABABgAJiBGAOgCUBXAOwwEt8YaBhCfAB1YAbGFADKIgG6swMXAG4AsAChlxAEzku5AN7Ly+8noO4MUBmAzkAYoIjYM2ygGZyAM1v3yANWyCGMOXIAXyN9UMoUayoAChs7S1cASnIAXgA+azVopMUlAwjM2I8E5PTrJ2zE3PziSKsKuM8k1IyAWUrAg3DwgG1WmAwACwgAEwBJfkFo/qHRib5BAHk+NggmXBoAOQgxpkFWJgOAc0SAXXDa8nbGkp1g5SCgA

But this sample:

using System.Runtime.CompilerServices;

class C {
    
    struct Float{ public float Value; }
    
    void F1(Float f) => F2(f);
    void F2(Float f) => F3(f);
    void F3(Float f) => M(f);    
    
    [MethodImpl(MethodImplOptions.NoInlining)]
    void M(Float f) { }
}

is compiled as:

; Core CLR v5.0.621.22011 on amd64

C..ctor()
    L0000: ret

C.F1(Float)
    L0000: push rax
    L0001: mov [rsp+0x18], rdx
    L0006: mov edx, [rsp+0x18]
    L000a: mov [rsp], edx
    L000d: mov edx, [rsp]
    L0010: add rsp, 8
    L0014: jmp C.M(Float)

C.F2(Float)
    L0000: push rax
    L0001: mov [rsp+0x18], rdx
    L0006: mov edx, [rsp+0x18]
    L000a: mov [rsp], edx
    L000d: mov edx, [rsp]
    L0010: add rsp, 8
    L0014: jmp C.M(Float)

C.F3(Float)
    L0000: mov [rsp+0x10], rdx
    L0005: mov edx, [rsp+0x10]
    L0009: jmp C.M(Float)

C.M(Float)
    L0000: ret

F1 and F2 are longer than F3 and seem to consume unnecessary stack. These methods are expected to work the same, so why this difference?

category:cq
theme:inlining
skill-level:intermediate
cost:medium
impact:medium

@dotnet-issue-labeler dotnet-issue-labeler bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Jun 6, 2021
@EgorBo
Copy link
Member

EgorBo commented Jun 6, 2021

Should be inlined with #52708

@EgorBo EgorBo self-assigned this Jun 6, 2021
@EgorBo EgorBo added this to the 6.0.0 milestone Jun 6, 2021
@EgorBo EgorBo added this to Needs Triage in .NET Core CodeGen via automation Jun 6, 2021
@EgorBo EgorBo moved this from Needs Triage to Optimizations in .NET Core CodeGen Jun 6, 2021
@EgorBo EgorBo removed the untriaged New issue has not been triaged by the area owner label Jun 6, 2021
@EgorBo
Copy link
Member

EgorBo commented Jun 7, 2021

Oops, I misinterpreted the issue - it's not inlining related since all F1-F2 are inlined just fine.
So it's struct-related, my guess - structs with float fields are not promotable in JIT yet.

@EgorBo EgorBo modified the milestones: 6.0.0, Future Jun 7, 2021
@EgorBo EgorBo removed their assignment Jun 7, 2021
@EgorBo EgorBo moved this from Optimizations to Backlog (General) in .NET Core CodeGen Jun 7, 2021
@hez2010
Copy link
Contributor

hez2010 commented Jun 9, 2021

I think it's #43867

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
Status: Backlog (General)
.NET Core CodeGen
  
Backlog (General)
Development

No branches or pull requests

3 participants