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

Dead stores generated from arguments to the helper call that was dead #36663

Open
kunalspathak opened this issue May 18, 2020 · 2 comments
Open
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

@kunalspathak
Copy link
Member

kunalspathak commented May 18, 2020

While investigating a test failure for #35675, I noticed that we don't remove the arguments to a dead call completely and end up with unnecessary loads to registers.

Below is the IR that populates the arguments to helper READYTORUN_ISINSTANCEOF.

N003 (  1,  1) [000052] ------------        t52 =    LCL_VAR   ref    V01 arg1         u:1 $c0
                                                  /--*  t52    ref    
N005 (  3,  3) [000328] -c----------       t328 = *  LEA(b+16) byref 
                                                  /--*  t328   byref  
N006 (  4,  3) [000201] n---GO------       t201 = *  IND       ref    <l:$328, c:$327>
                                                  /--*  t201   ref    
N008 (  6,  5) [000326] -c----------       t326 = *  LEA(b+16) byref 
                                                  /--*  t326   byref  
N009 (  7,  5) [000055] ---XGO------        t55 = *  IND       ref    <l:$334, c:$333>
                                                  /--*  t55    ref    
               [000420] ---XGO------       t420 = *  PUTARG_REG ref    REG x0
N010 (  1,  1) [000324] ------------       t324 =    CNS_INT(h) long   0x1f2f272c2f0 ftn REG x11 $18d
                                                  /--*  t324   long   
               [000421] ------------       t421 = *  PUTARG_REG long   REG x11
N001 (  1,  1) [000422] -c----------       t422 =    PHYSREG   long   x11
                                                  /--*  t422   long   
N002 (  4,  3) [000423] ------------       t423 = *  IND       long  
                                                  /--*  t420   ref    arg1 in x0
                                                  +--*  t421   long   arg0 in x11
                                                  +--*  t423   long   control expr
N011 ( 22, 10) [000056] --CXGO------        t56 = *  CALL help r2r_ind ref    HELPER.CORINFO_HELP_READYTORUN_ISINSTANCEOF <l:$338, c:$337>

During liveness, we remove dead call to that helper and remove some of the nodes related to them (few of them related to the R2R indirect param handled in #35675).

Removing dead call:
N011 ( 22, 10) [000056] --CXGO------              *  CALL help r2r_ind ref    HELPER.CORINFO_HELP_READYTORUN_ISINSTANCEOF <l:$338, c:$337>
Removing dead node:
N002 (  4,  3) [000423] ------------              *  IND       long  
Removing dead node:
N001 (  1,  1) [000422] ------------              *  PHYSREG   long   x11
Removing dead node:
               [000421] ------------              *  PUTARG_REG long   REG x11
Removing dead node:
N010 (  1,  1) [000324] ------------              *  CNS_INT(h) long   0x1f2f272c2f0 ftn REG x11 $18d
Removing dead node:
               [000420] ---XGO------              *  PUTARG_REG ref    REG x0

However it doesn't remove nodes like t201 and t55 and so we generate code for it.

Generating: N281 (  1,  1) [000052] ------------        t52 =    LCL_VAR   ref    V01 arg1         u:1 x20 REG x20 $c0
                                                              /--*  t52    ref    
Generating: N283 (  3,  3) [000328] -c----------       t328 = *  LEA(b+16) byref  REG NA
                                                              /--*  t328   byref  
Generating: N285 (  4,  3) [000201] n---GO------       t201 = *  IND       ref    REG x2 <l:$328, c:$327>
IN003b:                           ldr     x2, [x20,#16]
							GC regs: 180000 {x19 x20} => 180004 {x2 x19 x20}
                                                              /--*  t201   ref    
Generating: N287 (  6,  5) [000326] -c----------       t326 = *  LEA(b+16) byref  REG NA
                                                              /--*  t326   byref  
Generating: N289 (  7,  5) [000055] ---XGO------        t55 = *  IND       ref    REG x2 <l:$334, c:$333>
							GC regs: 180004 {x2 x19 x20} => 180000 {x19 x20}
IN003c:                           ldr     x2, [x2,#16]
							GC regs: 180000 {x19 x20} => 180004 {x2 x19 x20}
							GC regs: 180004 {x2 x19 x20} => 180000 {x19 x20}

The final code looks like this:

IN003b: 000118                    ldr     x2, [x20,#16]
IN003c: 00011C                    ldr     x2, [x2,#16]
IN003d: 000120                    ldr     x20, [x20,#8]
IN003e: 000124                    ldr     x2, [x19,#8]
IN003f: 000128                    add     x4, fp, #24	// [V18 tmp9]

Here, we perform dead stores to x2 and should be removed. Similar observation can be seen for x64:

IN002d: 0000BE mov      r8, gword ptr [rdi+16]
IN002e: 0000C2 mov      r8, gword ptr [r8+16]
IN002f: 0000C6 mov      rdi, gword ptr [rdi+8]
IN0030: 0000CA lea      r8, [V18 rsp+30H]
IN0031: 0000CF mov      qword ptr [V09+0x20 rsp+20H], r8

category:cq
theme:liveness
skill-level:expert
cost:medium
impact:small

@kunalspathak kunalspathak added tenet-performance Performance related issue area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels May 18, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label May 18, 2020
@kunalspathak
Copy link
Member Author

//cc @dotnet/jit-contrib

@BruceForstall BruceForstall added this to the 5.0 milestone May 19, 2020
@BruceForstall BruceForstall removed the untriaged New issue has not been triaged by the area owner label May 19, 2020
@BruceForstall
Copy link
Member

Since this is an optimization, and not a correctness issue, I'm going to move this to milestone Future. We can bring it back if we have time.

@BruceForstall BruceForstall modified the milestones: 5.0, Future Jun 1, 2020
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@BruceForstall BruceForstall removed the JitUntriaged CLR JIT issues needing additional triage label Nov 7, 2020
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

3 participants