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] Inefficient in-place readonly record struct updates with "with" keyword #111076

Open
dellamonica opened this issue Jan 3, 2025 · 2 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

@dellamonica
Copy link

Description

Consider the following code snippet:

public readonly record struct X(int A, int B, int C, int D, int E);

public record struct Y(int A, int B, int C, int D, int E);

public class C
{
    public void M(ref X x)
    {
        x = x with { C = 12 };
    }
    
    public void N(ref Y y)
    {
        y.C = 12;
    }
}

The function M uses "with" to change the reference x in place, this should be equivalent to just setting C = 12 directly. However, on sharplab we get:

C.M(X ByRef)
    L0000: sub esp, 0x14
    L0003: cmp [edx], dl
    L0005: vmovdqu xmm0, [edx]
    L0009: vmovdqu [esp], xmm0
    L000e: mov eax, [edx+0x10]
    L0011: mov [esp+0x10], eax
    L0015: vmovdqu xmm0, [esp]
    L001a: vmovdqu [edx], xmm0
    L001e: mov eax, [esp+0x10]
    L0022: mov [edx+0x10], eax
    L0025: mov dword ptr [edx+8], 0xc
    L002c: add esp, 0x14
    L002f: ret

C.N(Y ByRef)
    L0000: mov dword ptr [edx+8], 0xc
    L0007: ret

Configuration

Sharplab / C# / x64 / JIT ASM / Release

@dellamonica dellamonica added the tenet-performance Performance related issue label Jan 3, 2025
@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 Jan 3, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 3, 2025
Copy link
Contributor

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

@jakobbotsch
Copy link
Member

After physical promotion we do end up with IR in a shape that is close to optimizable with a simple pattern match:

***** BB01 [0000]
STMT00000 ( 0x000[E-] ... 0x007 )
               [000017] -A-XG------                           COMMA     void  
               [000016] ---XG------                         ├──▌  IND       byte  
               [000015] -----------                           └──▌  LCL_VAR   byref  V01 arg1         
               [000003] DA-XG------                         └──▌  STORE_LCL_VAR struct<X, 20> V02 loc0         
               [000002] ---XG------                            └──▌  BLK       struct<X, 20>
               [000001] -----------                               └──▌  LCL_VAR   byref  V01 arg1         

***** BB01 [0000]
STMT00002 ( 0x011[--] ... ??? )
               [000024] -A-XG------                           COMMA     void  
               [000008] -A-XG------                         ├──▌  STORE_BLK struct<X, 20> (copy)
               [000019] -----------                           ├──▌  LCL_VAR   byref  V01 arg1         
               [000007] -----------                           └──▌  LCL_VAR   struct<X, 20> V02 loc0         
               [000023] -A-XG------                         └──▌  STOREIND  int   
               [000022] -----------                            ├──▌  ADD       byref 
               [000000] -----------                              ├──▌  LCL_VAR   byref  V01 arg1         
               [000021] -----------                              └──▌  CNS_INT   long   8
               [000011] -----------                            └──▌  CNS_INT   int    12

Seems like if we had just forward substituted [000003] to [000007] then morph would probably be able to elide the block copy. But [000007] is not recognized as a last use, so we cannot do that.
Also physical promotion introduces the null check [000016] which is unnecessary here.

@jakobbotsch jakobbotsch self-assigned this Jan 6, 2025
@jakobbotsch jakobbotsch added this to the Future milestone Jan 6, 2025
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Jan 6, 2025
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 tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests

2 participants