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: Expand address peeling in block morphing to handle array indexing expressions #86755

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

Comments

@jakobbotsch
Copy link
Member

The optimization done in #85889 does not recognize the IR produced for array indexing. Example from #6534:

public struct Cards3
{
    public byte C0, C1, C2;
}

class Program
{
    static void Main()
    {           
        Run3();
    }

    private static Cards3[] cards3 = new Cards3[1];

    [MethodImpl(MethodImplOptions.NoInlining)]
    public static int Run3()
    {
        var c = cards3[0];
        return c.C0 - c.C1;
    }
}

Produces:

G_M62886_IG03:  ;; offset=000DH
       mov      rax, 0x1A5EF001D08      ; data for Program:cards3
       mov      rax, gword ptr [rax]
       cmp      dword ptr [rax+08H], 0
       jbe      SHORT G_M62886_IG06
       add      rax, 16
       movzx    rcx, byte  ptr [rax]
       movzx    rax, byte  ptr [rax+01H]
       sub      ecx, eax
       mov      eax, ecx

Ideally that add rax, 16 would not be there. But block morphing sees:

               [000010] DACXGO-----                           STORE_LCL_VAR struct<Cards3, 3>(P) V00 loc0         
                                                                ubyte  V00.Cards3:C0 (offs=0x00) -> V02 tmp1         
                                                                ubyte  V00.Cards3:C1 (offs=0x01) -> V03 tmp2         
                                                                ubyte  V00.Cards3:C2 (offs=0x02) -> V04 tmp3          (last use)
               [000009] nACXG+-----                         └──▌  BLK       struct<Cards3, 3>
               [000034] -ACXG+-----                            └──▌  COMMA     byref 
               [000021] DACXG+-----                               ├──▌  STORE_LCL_VAR ref    V05 tmp4         
               [000006] --CXG+-----                                 └──▌  COMMA     ref   
               [000005] H-CXG+-----                                    ├──▌  CALL help long   CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE
               [000003] -----+----- arg0 in rcx                          ├──▌  CNS_INT   long   0x7ff7ef1aff68
               [000004] -----+----- arg1 in rdx                          └──▌  CNS_INT   int    3
               [000001] I---G+-----                                    └──▌  IND       ref   
               [000000] H----+-----                                       └──▌  CNS_INT(h) long   0x1bc92001d08 static Fseq[cards3]
               [000033] ---X-+-----                               └──▌  COMMA     byref 
               [000026] ---X-+-----                                  ├──▌  BOUNDS_CHECK_Rng void  
               [000007] -----+-----                                    ├──▌  CNS_INT   int    0
               [000025] ---X-+-----                                    └──▌  ARR_LENGTH int   
               [000022] -----+-----                                       └──▌  LCL_VAR   ref    V05 tmp4         
               [000032] -----+-----                                  └──▌  ARR_ADDR  byref Cards3[]
               [000031] -----+-----                                     └──▌  ADD       byref 
               [000023] -----+-----                                        ├──▌  LCL_VAR   ref    V05 tmp4         
               [000030] -----+-----                                        └──▌  CNS_INT   long   16

So we both need to handle the commas and also the wrapping ARR_ADDR annotation node.

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

ghost commented May 25, 2023

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

Issue Details

The optimization done in #85889 does not recognize the IR produced for array indexing. Example from #6534:

public struct Cards3
{
    public byte C0, C1, C2;
}

class Program
{
    static void Main()
    {           
        Run3();
    }

    private static Cards3[] cards3 = new Cards3[1];

    [MethodImpl(MethodImplOptions.NoInlining)]
    public static int Run3()
    {
        var c = cards3[0];
        return c.C0 - c.C1;
    }
}

Produces:

G_M62886_IG03:  ;; offset=000DH
       mov      rax, 0x1A5EF001D08      ; data for Program:cards3
       mov      rax, gword ptr [rax]
       cmp      dword ptr [rax+08H], 0
       jbe      SHORT G_M62886_IG06
       add      rax, 16
       movzx    rcx, byte  ptr [rax]
       movzx    rax, byte  ptr [rax+01H]
       sub      ecx, eax
       mov      eax, ecx

Ideally that add rax, 16 would not be there. But block morphing sees:

               [000010] DACXGO-----                           STORE_LCL_VAR struct<Cards3, 3>(P) V00 loc0         
                                                                ubyte  V00.Cards3:C0 (offs=0x00) -> V02 tmp1         
                                                                ubyte  V00.Cards3:C1 (offs=0x01) -> V03 tmp2         
                                                                ubyte  V00.Cards3:C2 (offs=0x02) -> V04 tmp3          (last use)
               [000009] nACXG+-----                         └──▌  BLK       struct<Cards3, 3>
               [000034] -ACXG+-----                            └──▌  COMMA     byref 
               [000021] DACXG+-----                               ├──▌  STORE_LCL_VAR ref    V05 tmp4         
               [000006] --CXG+-----                                 └──▌  COMMA     ref   
               [000005] H-CXG+-----                                    ├──▌  CALL help long   CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE
               [000003] -----+----- arg0 in rcx                          ├──▌  CNS_INT   long   0x7ff7ef1aff68
               [000004] -----+----- arg1 in rdx                          └──▌  CNS_INT   int    3
               [000001] I---G+-----                                    └──▌  IND       ref   
               [000000] H----+-----                                       └──▌  CNS_INT(h) long   0x1bc92001d08 static Fseq[cards3]
               [000033] ---X-+-----                               └──▌  COMMA     byref 
               [000026] ---X-+-----                                  ├──▌  BOUNDS_CHECK_Rng void  
               [000007] -----+-----                                    ├──▌  CNS_INT   int    0
               [000025] ---X-+-----                                    └──▌  ARR_LENGTH int   
               [000022] -----+-----                                       └──▌  LCL_VAR   ref    V05 tmp4         
               [000032] -----+-----                                  └──▌  ARR_ADDR  byref Cards3[]
               [000031] -----+-----                                     └──▌  ADD       byref 
               [000023] -----+-----                                        ├──▌  LCL_VAR   ref    V05 tmp4         
               [000030] -----+-----                                        └──▌  CNS_INT   long   16

So we both need to handle the commas and also the wrapping ARR_ADDR annotation node.

Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@jakobbotsch jakobbotsch removed the untriaged New issue has not been triaged by the area owner label May 25, 2023
@jakobbotsch jakobbotsch added this to the Future milestone May 25, 2023
@SingleAccretion
Copy link
Contributor

SingleAccretion commented May 25, 2023

Note that when implementing this care needs to be taken to make sure ARR_ADDR remains valid, by, e. g., adding a field to it that indicates how much of the offset was "taken away" and consuming this information in VN.

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 optimization
Projects
None yet
Development

No branches or pull requests

2 participants