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: Lowering's insertion of PUTARG_* nodes is questionable #103300

Closed
jakobbotsch opened this issue Jun 11, 2024 · 2 comments · Fixed by #103301
Closed

JIT: Lowering's insertion of PUTARG_* nodes is questionable #103300

jakobbotsch opened this issue Jun 11, 2024 · 2 comments · Fixed by #103301
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@jakobbotsch
Copy link
Member

When we lower arguments to calls we create PUTARG_* nodes and insert them right after their corresponding argument definitions:

// Insert the putarg/copy into the block
BlockRange().InsertAfter(arg, putArgOrBitcast);

This insertion location does not make sense, since conceptually the definitions of the argument nodes can be arbitrarily far away from the call and there can even be interfering calls between the argument and consuming call.

The choice was probably made because the argument placement order is decided during morph when the call is in tree order, and the expectation is that args are placed in the early/late order decided there.

A few potential fixes:

  • Insert PUTARG nodes during rationalization when we can depend on tree order and the early/late args call invariant. Seems to move something that is conceptually ABI lowering into rationalization which is unfortunate.
  • Insert the PUTARG nodes right before the call node. Unfortunately has large regressions.
  • Check for call interference between the arg and call and place the argument early on no interference and late on interference. Probably costs some TP.
  • Move arg lowering from morph to lowering with appropriate interference checking (similar to what morph has to do today anyway). Probably best long term solution, but takes the most work.

cc @dotnet/jit-contrib

@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 Jun 11, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jun 11, 2024
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 Author

jakobbotsch commented Jun 11, 2024

We do have cases today where we end up with interfering CALL nodes between a PUTARG_STK and a call, specifically because of the BULKWRITEBARRIER transformation introducing a call between a temp store for a late argument and its corresponding call.

For example:

****** START compiling Microsoft.CodeAnalysis.Compilation:SerializePeToStream(Microsoft.CodeAnalysis.Emit.CommonPEModuleBuilder,Microsoft.CodeAnalysis.DiagnosticBag,Microsoft.CodeAnalysis.CommonMessageProvider,System.Func`1[System.IO.Stream],System.Func`1[System.IO.Stream],System.Func`1[System.IO.Stream],Microsoft.Cci.PdbWriter,System.String,Microsoft.CodeAnalysis.RebuildData,ubyte,ubyte,ubyte,ubyte,System.Nullable`1[System.Security.Cryptography.RSAParameters],System.Threading.CancellationToken):ubyte (MethodHash=c9b17aac)

...

N001 (  3,  2) [000037] -----------                   t37 =    LCL_VAR   ref    V06 arg6         u:1 (last use) $86
                                                            ┌──▌  t37    ref    
               [000270] -----------                           PUTARG_STK [+0x20] void  
N002 (  3,  2) [000038] -----------                   t38 =    LCL_VAR   ref    V07 arg7         u:1 (last use) $87
                                                            ┌──▌  t38    ref    
               [000271] -----------                           PUTARG_STK [+0x28] void  
N003 (  1,  1) [000228] -----------                  t228 =    LCL_VAR   int    V29 cse0         u:1 $181
                                                            ┌──▌  t228   int    
               [000272] -----------                           PUTARG_STK [+0x30] void  
N004 (  3,  2) [000040] -----------                   t40 =    LCL_VAR   int    V17 loc2         u:2 (last use) $18a
                                                            ┌──▌  t40    int    
               [000273] -----------                           PUTARG_STK [+0x38] void  
N005 (  3,  2) [000041] -----------                   t41 =    LCL_VAR   int    V12 arg12        u:1 (last use) $c3
                                                            ┌──▌  t41    int    
N006 (  4,  4) [000200] -----------                  t200 =   CAST      int <- ubyte <- int $18e
                                                            ┌──▌  t200   int    
               [000274] -----------                           PUTARG_STK [+0x40] void  
               [000261] D----------                  t261 =    LCL_ADDR  byref  V28 tmp10        [+0]
                                                            ┌──▌  t261   byref  
               [000267] -----------                  t267 =   PUTARG_REG byref  REG rcx
N007 (  1,  1) [000042] -----------                   t42 =    LCL_VAR   byref  V13 arg13        u:1 $100
                                                            ┌──▌  t42    byref  
               [000268] -----------                  t268 =   PUTARG_REG byref  REG rdx
               [000262] -----------                  t262 =    CNS_INT   long   72
                                                            ┌──▌  t262   long   
               [000269] -----------                  t269 =   PUTARG_REG long   REG r8
                                                            ┌──▌  t267   byref  arg0 in rcx
                                                            ├──▌  t268   byref  arg1 in rdx
                                                            ├──▌  t269   long   arg2 in r8
N004 ( 17, 11) [000266] --CXG------                           CALL help void   CORINFO_HELP_BULK_WRITEBARRIER
N009 ( 17, 12) [000202] -----------                            NOP       void  
N010 (  3,  2) [000043] -----------                   t43 =    LCL_VAR   ref    V25 tmp7         u:1 $89
                                                            ┌──▌  t43    ref    
               [000275] -----------                           PUTARG_STK [+0x50] void  
N011 (  3,  3) [000203] -----------                  t203 =    LCL_ADDR  long   V28 tmp10        [+0] $280
                                                            ┌──▌  t203   long   
               [000276] -----------                           PUTARG_STK [+0x48] void  
N012 (  3,  3) [000033] -----------                   t33 =    LCL_ADDR  long   V21 tmp3         [+0] $281
                                                            ┌──▌  t33    long   
               [000277] -----------                  t277 =   PUTARG_REG long   REG rcx
N013 (  1,  1) [000034] -----------                   t34 =    LCL_VAR   ref    V02 arg2         u:1 $82
                                                            ┌──▌  t34    ref    
               [000278] -----------                  t278 =   PUTARG_REG ref    REG rdx
N014 (  1,  1) [000035] -----------                   t35 =    LCL_VAR   ref    V03 arg3         u:1 (last use) $83
                                                            ┌──▌  t35    ref    
               [000279] -----------                  t279 =   PUTARG_REG ref    REG r8
N015 (  3,  2) [000036] -----------                   t36 =    LCL_VAR   ref    V05 arg5         u:1 (last use) $85
                                                            ┌──▌  t36    ref    
               [000280] -----------                  t280 =   PUTARG_REG ref    REG r9
N001 (  3, 10) [000281] Hc---------                  t281 =    CNS_INT(h) long   0x7fff099345a0 ftn
                                                            ┌──▌  t281   long   
N002 (  5, 12) [000282] nc--G------                  t282 =   IND       long   REG NA
                                                            ┌──▌  t277   long   arg0 in rcx
                                                            ├──▌  t278   ref    arg1 in rdx
                                                            ├──▌  t279   ref    arg2 in r8
                                                            ├──▌  t280   ref    arg3 in r9
                                                            ├──▌  t282   long   control expr
N016 ( 80, 45) [000044] -ACXG------                   t44 =   CALL      int    <unknown method> $2c0

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

Successfully merging a pull request may close this issue.

2 participants