JIT: Fix invalid last use propagation for implicit byrefs

[jakobbotsch]

Fix #80488

[ghost]

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

Issue Details

---

Fix #80488

Author:	jakobbotsch
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[jakobbotsch]

A lot of the regressions look like:

@@ -7,7 +7,7 @@
 ; 0 inlinees with PGO data; 1 single block inlinees; 0 inlinees without PGO data
 ; Final local variable assignments
 ;
-;  V00 arg0         [V00,T00] (  4,  8   )   byref  ->  rcx         ld-addr-op single-def
+;  V00 arg0         [V00,T00] (  4,  8   )   byref  ->   r8         ld-addr-op single-def
 ;  V01 OutArgs      [V01    ] (  1,  1   )  lclBlk (32) [rsp+00H]   "OutgoingArgSpace"
 ;* V02 tmp1         [V02    ] (  0,  0   )  struct (16) zero-ref    "impAppendStmt"
 ;* V03 tmp2         [V03    ] (  0,  0   )     ref  ->  zero-ref    V07._items(offs=0x00) P-INDEP "field V00._items (fldOffset=0x0)"
@@ -15,27 +15,35 @@
 ;* V05 tmp4         [V05    ] (  0,  0   )     ref  ->  zero-ref    V02._items(offs=0x00) P-INDEP "field V02._items (fldOffset=0x0)"
 ;* V06 tmp5         [V06    ] (  0,  0   )     int  ->  zero-ref    V02._length(offs=0x08) P-INDEP "field V02._length (fldOffset=0x8)"
 ;* V07 tmp6         [V07    ] (  0,  0   )  struct (16) zero-ref    "Promoted implicit byref"
+;  V08 tmp7         [V08    ] (  2,  4   )  struct (16) [rsp+28H]   do-not-enreg[XS] addr-exposed "by-value struct argument"
 ;
-; Lcl frame size = 40
+; Lcl frame size = 56
 
 G_M45936_IG01:        ; bbWeight=1, gcrefRegs=00000000 {}, byrefRegs=00000000 {}, byref, nogc <-- Prolog IG
-       sub      rsp, 40
-						;; size=4 bbWeight=1 PerfScore 0.25
-G_M45936_IG02:        ; bbWeight=1, gcrefRegs=00000000 {}, byrefRegs=00000002 {rcx}, byref
-       ; byrRegs +[rcx]
-       mov      r8d, dword ptr [rcx+08H]
+       sub      rsp, 56
+       vzeroupper 
+       mov      r8, rcx
+       ; byrRegs +[r8]
+						;; size=10 bbWeight=1 PerfScore 1.50
+G_M45936_IG02:        ; bbWeight=1, gcrefRegs=00000000 {}, byrefRegs=00000100 {r8}, byref, nogc
+       vmovdqu  xmm0, xmmword ptr [r8]
+       vmovdqu  xmmword ptr [rsp+28H], xmm0
+						;; size=11 bbWeight=1 PerfScore 5.00
+G_M45936_IG03:        ; bbWeight=1, extend
+       lea      rcx, [rsp+28H]
+       mov      r8d, dword ptr [r8+08H]
+       ; byrRegs -[r8]
        xor      edx, edx
        call     [Microsoft.CodeAnalysis.Collections.SegmentedArray:Reverse[ubyte](Microsoft.CodeAnalysis.Collections.SegmentedArray`1[ubyte],int,int)]
-       ; byrRegs -[rcx]
        ; gcr arg pop 0
        nop      
-						;; size=13 bbWeight=1 PerfScore 5.50
-G_M45936_IG03:        ; bbWeight=1, epilog, nogc, extend
-       add      rsp, 40
+						;; size=18 bbWeight=1 PerfScore 6.00
+G_M45936_IG04:        ; bbWeight=1, epilog, nogc, extend
+       add      rsp, 56
        ret      
 						;; size=5 bbWeight=1 PerfScore 1.25
 
-; Total bytes of code 22, prolog size 4, PerfScore 9.20, instruction count 7, allocated bytes for code 22 (MethodHash=d0364c8f) for method Microsoft.CodeAnalysis.Collections.SegmentedArray:Reverse[ubyte](Microsoft.CodeAnalysis.Collections.SegmentedArray`1[ubyte])
+; Total bytes of code 44, prolog size 7, PerfScore 18.15, instruction count 12, allocated bytes for code 44 (MethodHash=d0364c8f) for method Microsoft.CodeAnalysis.Collections.SegmentedArray:Reverse[ubyte](Microsoft.CodeAnalysis.Collections.SegmentedArray`1[ubyte])
 ; ============================================================

Notice that we have another later argument accessing a field. It would still be legal to omit the copy here though, but it would take some more analysis to prove. It seems doable though.

[jakobbotsch]

Diffs. Regressions as expected due to fewer omitted copies.

cc @dotnet/jit-contrib PTAL @AndyAyersMS

[sebastienros]

With these commits (2ca7cf7...6aaaaaa) aspnet apps crash at runtime in the benchmarks CI. I am flagging the PR which had the potential to explain it.

Crank command if you can run with and without this PR changes:

crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/src/BenchmarksApps/Mvc/benchmarks.certapi.yml  --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/build/ci.profile.yml --scenario NoMvcAuth --profile intel-win-app --profile intel-load2-load --profile intel-db-db --application.framework net8.0 --application.aspNetCoreVersion 8.0.0-alpha.1.23062.23 --application.runtimeVersion 8.0.0-alpha.1.23061.12 --application.sdkVersion 8.0.100-alpha.1.23061.8

[jakobbotsch]

cc @tannergooding, can you check if this is due to #80242? I think it is unlikely this PR is the cause, and I currently don't have access to run crank so I cannot double check.

[sebastienros]

We isolated the issue in the GC changes, please disregard my comment.