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

Jit should avoid allocations for T1 is T2 pattern in generics #58554

Closed
EgorBo opened this issue Sep 2, 2021 · 4 comments
Closed

Jit should avoid allocations for T1 is T2 pattern in generics #58554

EgorBo opened this issue Sep 2, 2021 · 4 comments
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue
Milestone

Comments

@EgorBo
Copy link
Member

EgorBo commented Sep 2, 2021

Minimal repro from #58535

using System.Runtime.CompilerServices;

class Program
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    static bool Test<T1, T2>(T1 source) where T1 : struct where T2 : struct 
    {
        return source is T2; // box, issinst, ldnull
    }

    static void Main()
    {
        Test<(int, string), (int, string)>((42, "42"));
    }
}

Test is compiled as:

G_M19373_IG01:
       push     rdi
       push     rsi
       push     rbp
       push     rbx
       sub      rsp, 40
       mov      qword ptr [rsp+20H], rcx
       mov      rbx, rcx
       mov      rsi, rdx
						;; bbWeight=1    PerfScore 5.75
G_M19373_IG02:
       mov      rcx, qword ptr [rbx+56]
       mov      rcx, qword ptr [rcx]
       call     CORINFO_HELP_NEWSFAST
       mov      rbp, rax
       lea      rdi, bword ptr [rbp+8]
       call     CORINFO_HELP_ASSIGN_BYREF
       movsq    
       mov      rax, qword ptr [rbp]
       mov      rdx, qword ptr [rbx+56]
       cmp      rax, qword ptr [rdx+8]
       je       SHORT G_M19373_IG04
						;; bbWeight=1    PerfScore 14.75
G_M19373_IG03:
       xor      rbp, rbp
						;; bbWeight=0.12 PerfScore 0.03
G_M19373_IG04:
       test     rbp, rbp
       setne    al
       movzx    rax, al
						;; bbWeight=1    PerfScore 1.50
G_M19373_IG05:
       add      rsp, 40
       pop      rbx
       pop      rbp
       pop      rsi
       pop      rdi
       ret      
						;; bbWeight=1    PerfScore 3.25

while we could avoid the boxing here and compare generic contexts?

category:performance
theme:generics
skill-level:intermediate
cost:small
impact:small

@EgorBo EgorBo added the tenet-performance Performance related issue label Sep 2, 2021
@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 Sep 2, 2021
@ghost
Copy link

ghost commented Sep 2, 2021

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

Issue Details

Minimal repro from #58535

using System.Runtime.CompilerServices;

class Program
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    static bool Test<T1, T2>(T1 source) => source is T2;

    static void Main()
    {
        Test<(int, string), (int, string)>((42, "42"));
    }
}

Test is compiled as:

G_M19373_IG01:
       push     rdi
       push     rsi
       push     rbp
       push     rbx
       sub      rsp, 40
       mov      qword ptr [rsp+20H], rcx
       mov      rbx, rcx
       mov      rsi, rdx
						;; bbWeight=1    PerfScore 5.75
G_M19373_IG02:
       mov      rcx, qword ptr [rbx+56]
       mov      rcx, qword ptr [rcx]
       call     CORINFO_HELP_NEWSFAST
       mov      rbp, rax
       lea      rdi, bword ptr [rbp+8]
       call     CORINFO_HELP_ASSIGN_BYREF
       movsq    
       mov      rax, qword ptr [rbp]
       mov      rdx, qword ptr [rbx+56]
       cmp      rax, qword ptr [rdx+8]
       je       SHORT G_M19373_IG04
						;; bbWeight=1    PerfScore 14.75
G_M19373_IG03:
       xor      rbp, rbp
						;; bbWeight=0.12 PerfScore 0.03
G_M19373_IG04:
       test     rbp, rbp
       setne    al
       movzx    rax, al
						;; bbWeight=1    PerfScore 1.50
G_M19373_IG05:
       add      rsp, 40
       pop      rbx
       pop      rbp
       pop      rsi
       pop      rdi
       ret      
						;; bbWeight=1    PerfScore 3.25

while we could avoid the boxing here and compare generic contexts?

Author: EgorBo
Assignees: -
Labels:

tenet-performance, area-CodeGen-coreclr, untriaged

Milestone: -

@EgorBo EgorBo added this to the Future milestone Sep 2, 2021
@EgorBo EgorBo self-assigned this Sep 2, 2021
@EgorBo EgorBo removed the untriaged New issue has not been triaged by the area owner label Sep 2, 2021
@AndyAyersMS
Copy link
Member

Seems like another box pattern match case?

@acaly
Copy link

acaly commented Sep 3, 2021

Shouldn't this be better written as default(T1) is T2? JIT can optimize that properly.

I think unless we need the dynamic checking, we should prefer checking with default(T) instead of something with type T (in this case the parameter).

AndyAyersMS added a commit that referenced this issue Jul 1, 2024
Enable object stack allocation for ref classes and extend the support to include boxed value classes. Use a specialized unbox helper for stack allocated boxes, both to avoid apparent escape of the box by the helper, and to ensure all box field accesses are visible to the JIT. Update the local address visitor to rewrite trees involving address of stack allocated boxes in some cases to avoid address exposure. Disable old promotion for stack allocated boxes (since we have no field handles) and allow physical promotion to enregister the box method table and/or payload as appropriate. In OSR methods handle the fact that the stack allocation may actually have been a heap allocation by the Tier0 method.

The analysis TP cost is around 0.4-0.7% (notes below). Boxes are much less likely to escape than ref classes (roughly ~90% of boxes escape, ~99.8% of ref classes escape). Codegen impact is diminished somewhat because many of the boxes are dead and were already getting optimized away.
 
Fixes #4584, #9118, #10195, #11192, #53585, #58554, #85570

---------

Co-authored-by: Jakob Botsch Nielsen <jakob.botsch.nielsen@gmail.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@AndyAyersMS
Copy link
Member

Fixed by #103361

@github-actions github-actions bot locked and limited conversation to collaborators Aug 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests

3 participants