Stack allocate unescaped boxes #103361

AndyAyersMS · 2024-06-12T18:15:05Z

Enable object stack allocation for ref classes and extend the support to include boxed value classes. Use a specialized unbox helper for stack allocated boxes, both to avoid apparent escape of the box by the helper, and to ensure all box field accesses are visible to the JIT. Update the local address visitor to rewrite trees involving address of stack allocated boxes in some cases to avoid address exposure. Disable old promotion for stack allocated boxes (since we have no field handles) and allow physical promotion to enregister the box method table and/or payload as appropriate. In OSR methods handle the fact that the stack allocation may actually have been a heap allocation by the Tier0 method.

The analysis TP cost is around 0.4-0.7% (notes below). Boxes are much less likely to escape than ref classes (roughly ~90% of boxes escape, ~99.8% of ref classes escape). Codegen impact is diminished somewhat because many of the boxes are dead and were already getting optimized away.

Fixes #4584, #9118, #10195, #11192, #58535, #58554, #85570

Still todo:

settle on the right new type for a stack allocated box (currently KeyValuePair)
R2R/NAOT support (done in Enable "Allocate on stack" for NAOT/R2R #104411)
continue trying to do a realistic assessment of impact

…-lclmorph Minor fixes

AndyAyersMS · 2024-06-12T18:30:49Z

For case from #9118:

; Assembly listing for method C:Main():int (FullOpts)
; Emitting BLENDED_CODE for X64 with AVX - Windows
; FullOpts code
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; 1 inlinees with PGO data; 3 single block inlinees; 1 inlinees without PGO data
; Final local variable assignments
;
;  V00 OutArgs      [V00    ] (  1,  1   )  struct (32) [rsp+0x00]  do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;* V01 tmp1         [V01,T00] (  0,  0   )     ref  ->  zero-ref    class-hnd exact single-def "NewObj constructor temp" <ObjectEqualityComparer`1[int]>
;* V02 tmp2         [V02    ] (  0,  0   )   ubyte  ->  zero-ref    "Inline return value spill temp"
;* V03 tmp3         [V03    ] (  0,  0   )     int  ->  zero-ref    ld-addr-op "Inlining Arg"
;* V04 tmp4         [V04    ] (  0,  0   )    long  ->  zero-ref    class-hnd exact "Single-def Box Helper" <System.Int32>
;* V05 tmp5         [V05    ] (  0,  0   )   ubyte  ->  zero-ref    "Inline return value spill temp"
;* V06 tmp6         [V06    ] (  0,  0   )    long  ->  zero-ref    class-hnd exact "Inlining Arg" <System.Int32>
;* V07 tmp7         [V07    ] (  0,  0   )  struct (16) zero-ref    do-not-enreg[SF] "stack allocated boxed value class temp" <System.Collections.Generic.KeyValuePair`2[long,int]>
;* V08 tmp8         [V08    ] (  0,  0   )    long  ->  zero-ref    "V07.[000..008)"
;* V09 tmp9         [V09    ] (  0,  0   )     int  ->  zero-ref    "V07.[008..012)"
;
; Lcl frame size = 40

G_M46144_IG01:  ;; offset=0x0000
       sub      rsp, 40
                                                ;; size=4 bbWeight=1 PerfScore 0.25
G_M46144_IG02:  ;; offset=0x0004
       mov      eax, 100
                                                ;; size=5 bbWeight=1 PerfScore 0.25
G_M46144_IG03:  ;; offset=0x0009
       add      rsp, 40
       ret
                                                ;; size=5 bbWeight=1 PerfScore 1.25

; Total bytes of code 14, prolog size 4, PerfScore 1.75, instruction count 4, allocated bytes for code 14 (MethodHash=cd934bbf) for method C:Main():int (FullOpts)

AndyAyersMS · 2024-06-12T18:32:51Z

Current code for #10195

; Assembly listing for method Program:Pattern[Dog](Dog) (FullOpts)
; Emitting BLENDED_CODE for X86 with AVX512 - Windows
; FullOpts code
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; 0 inlinees with PGO data; 2 single block inlinees; 0 inlinees without PGO data
; Final local variable assignments
;
;  V00 arg0         [V00,T00] (  3,  3   )  struct ( 8) rcx         single-def <Dog>
;* V01 loc0         [V01    ] (  0,  0   )    long  ->  zero-ref    class-hnd exact <Dog>
;# V02 OutArgs      [V02    ] (  1,  1   )  struct ( 0) [rsp+0x00]  do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;* V03 tmp1         [V03    ] (  0,  0   )    long  ->  zero-ref    class-hnd exact "Single-def Box Helper" <Dog>
;* V04 tmp2         [V04    ] (  0,  0   )    long  ->  zero-ref    "Inlining Arg"
;  V05 tmp3         [V05    ] (  2,  2   )  struct (16) [rsp+0x08]  do-not-enreg[XSF] must-init addr-exposed "stack allocated boxed value class temp" <System.Collections.Generic.KeyValuePair`2[long,Dog]>
;
; Lcl frame size = 24

G_M46299_IG01:  ;; offset=0x0000
       sub      rsp, 24
       xor      eax, eax
       mov      qword ptr [rsp+0x08], rax
       mov      qword ptr [rsp+0x10], rax
						;; size=16 bbWeight=1 PerfScore 2.50
G_M46299_IG02:  ;; offset=0x0010
       mov      rax, 0x7FFEDABE4C08      ; Dog
       mov      qword ptr [rsp+0x08], rax
       mov      byte  ptr [rsp+0x10], cl
						;; size=19 bbWeight=1 PerfScore 2.25
G_M46299_IG03:  ;; offset=0x0023
       add      rsp, 24
       ret      
						;; size=5 bbWeight=1 PerfScore 1.25

; Total bytes of code 40, prolog size 16, PerfScore 6.00, instruction count 9, allocated bytes for code 40 (MethodHash=a9204b24) for method Program:Pattern[Dog](Dog) (FullOpts)
; ============================================================

No more boxing, but room for improvement...

jakobbotsch · 2024-06-12T18:45:50Z

Current code for #10195

; Assembly listing for method Program:Pattern[Dog](Dog) (FullOpts)
; Emitting BLENDED_CODE for X86 with AVX512 - Windows
; FullOpts code
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; 0 inlinees with PGO data; 2 single block inlinees; 0 inlinees without PGO data
; Final local variable assignments
;
;  V00 arg0         [V00,T00] (  3,  3   )  struct ( 8) rcx         single-def <Dog>
;* V01 loc0         [V01    ] (  0,  0   )    long  ->  zero-ref    class-hnd exact <Dog>
;# V02 OutArgs      [V02    ] (  1,  1   )  struct ( 0) [rsp+0x00]  do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;* V03 tmp1         [V03    ] (  0,  0   )    long  ->  zero-ref    class-hnd exact "Single-def Box Helper" <Dog>
;* V04 tmp2         [V04    ] (  0,  0   )    long  ->  zero-ref    "Inlining Arg"
;  V05 tmp3         [V05    ] (  2,  2   )  struct (16) [rsp+0x08]  do-not-enreg[XSF] must-init addr-exposed "stack allocated boxed value class temp" <System.Collections.Generic.KeyValuePair`2[long,Dog]>
;
; Lcl frame size = 24

G_M46299_IG01:  ;; offset=0x0000
       sub      rsp, 24
       xor      eax, eax
       mov      qword ptr [rsp+0x08], rax
       mov      qword ptr [rsp+0x10], rax
						;; size=16 bbWeight=1 PerfScore 2.50
G_M46299_IG02:  ;; offset=0x0010
       mov      rax, 0x7FFEDABE4C08      ; Dog
       mov      qword ptr [rsp+0x08], rax
       mov      byte  ptr [rsp+0x10], cl
						;; size=19 bbWeight=1 PerfScore 2.25
G_M46299_IG03:  ;; offset=0x0023
       add      rsp, 24
       ret      
						;; size=5 bbWeight=1 PerfScore 1.25

; Total bytes of code 40, prolog size 16, PerfScore 6.00, instruction count 9, allocated bytes for code 40 (MethodHash=a9204b24) for method Program:Pattern[Dog](Dog) (FullOpts)
; ============================================================

No more boxing, but room for improvement...

Any idea why we end up with address exposure for V05?

AndyAyersMS · 2024-06-12T18:49:59Z

Any idea why we end up with address exposure for V05?

I think it's that local morph needs to deal with a nullcheck... will know shortly

jkotas · 2024-06-12T18:57:37Z

src/coreclr/vm/jitinterface.cpp

+        TypeHandle mt(CoreLibBinder::GetElementType(ELEMENT_TYPE_I));
+        TypeHandle boxedFields[2] = {mt, VMClsHnd};
+        Instantiation boxedFieldsInst((TypeHandle*)&boxedFields, 2);
+        TypeHandle genericKvp = CoreLibBinder::GetClass(CLASS__KEYVALUEPAIRGENERIC);


This won't work well. KeyValuePair is auto-layout when the type contains references. auto-layout moves references to the front.

For example, KeyValuePair<IntPtr, string> has string at offset 0 and IntPtr at offset 8.

I guess it probably works ok for valuetypes currently, but it is fragile.

Thanks... we were trying to find a suitable type without needing too much from the runtime.

I think we should introduce dedicated internal helper type for this.

We can also try and do this entirely within the jit (that's where I started), but it would be nice to have class and field handles and the like.

I also am still not sure how effective any of this will be, so I want to do some more analysis. Earlier studies suggest we can prove maybe 1% of boxes do not escape (static, not dynamic) but were based on SPMI which can't replay a bunch of possibly interesting cases. Given the cost of the analysis I would like to see if we can get that number a bit higher.

That 1% is a static measure, that is, 1% of the box call sites, not necessarily 1% of boxes allocated.

AndyAyersMS · 2024-06-12T19:21:21Z

Yep, that was the reason...

; Assembly listing for method Program:Pattern[Dog](Dog) (FullOpts)
; Emitting BLENDED_CODE for X86 with AVX512 - Windows
; FullOpts code
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; 0 inlinees with PGO data; 2 single block inlinees; 0 inlinees without PGO data
; Final local variable assignments
;
;* V00 arg0         [V00    ] (  0,  0   )  struct ( 8) zero-ref    single-def <Dog>
;* V01 loc0         [V01    ] (  0,  0   )    long  ->  zero-ref    class-hnd exact <Dog>
;# V02 OutArgs      [V02    ] (  1,  1   )  struct ( 0) [rsp+0x00]  do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;* V03 tmp1         [V03    ] (  0,  0   )    long  ->  zero-ref    class-hnd exact "Single-def Box Helper" <Dog>
;* V04 tmp2         [V04    ] (  0,  0   )    long  ->  zero-ref    "Inlining Arg"
;* V05 tmp3         [V05    ] (  0,  0   )  struct (16) zero-ref    do-not-enreg[SF] "stack allocated boxed value class temp" <System.Collections.Generic.KeyValuePair`2[long,Dog]>
;
; Lcl frame size = 0

G_M46299_IG01:  ;; offset=0x0000
						;; size=0 bbWeight=1 PerfScore 0.00
G_M46299_IG02:  ;; offset=0x0000
       ret      
						;; size=1 bbWeight=1 PerfScore 1.00

; Total bytes of code 1, prolog size 0, PerfScore 1.00, instruction count 1, allocated bytes for code 1 (MethodHash=a9204b24) for method Program:Pattern[Dog](Dog) (FullOpts)
; ============================================================

AndyAyersMS · 2024-06-12T19:50:28Z

For #85570 one box gets optimized, but the other box is passed to an unbox helper. This helper causes the box to escape if the type tests fail.

There is some preliminary helper escape logic in #103148 but it is too aggressive for unboxing. We would need some kind of partial escape analysis and a different helper pattern for this.

AndyAyersMS · 2024-06-12T20:29:21Z

We stack allocate in #58554 but the local box is exposed, possibly because of a ?: -- need to review if we get conservative handling these in local morph.

So asm is not that pretty yet

; Assembly listing for method Program:Test[System.ValueTuple`2[int,System.__Canon],System.ValueTuple`2[int,System.__Canon]](System.ValueTuple`2[int,System.__Canon]):ubyte (FullOpts)
; Emitting BLENDED_CODE for X86 with AVX512 - Windows
; FullOpts code
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; Final local variable assignments
;
;  V00 TypeCtx      [V00,T01] (  4,  4   )    long  ->  rcx         single-def
;  V01 arg0         [V01,T00] (  3,  6   )   byref  ->  rdx         single-def
;# V02 OutArgs      [V02    ] (  1,  1   )  struct ( 0) [rsp+0x00]  do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;* V03 tmp1         [V03    ] (  0,  0   )    long  ->  zero-ref    class-hnd exact "Single-def Box Helper" <System.ValueTuple`2[int,System.__Canon]>
;* V04 tmp2         [V04    ] (  0,  0   )   byref  ->  zero-ref    "ISINST eval op1"
;  V05 tmp3         [V05,T02] (  2,  4   )   byref  ->  rdx         class-hnd "spilling qmarkNull" <System.ValueTuple`2[int,System.__Canon]>
;  V06 tmp4         [V06    ] (  4,  4   )  struct (24) [rsp+0x08]  do-not-enreg[XSF] must-init addr-exposed "stack allocated boxed value class temp" <System.Collections.Generic.KeyValuePair`2[long,System.ValueTuple`2[int,System.__Canon]]>
;* V07 tmp5         [V07    ] (  0,  0   )     ref  ->  zero-ref    "field V01.Item2 (fldOffset=0x0)" P-INDEP
;* V08 tmp6         [V08    ] (  0,  0   )     int  ->  zero-ref    "field V01.Item1 (fldOffset=0x8)" P-INDEP
;* V09 tmp7         [V09    ] (  0,  0   )  struct (16) zero-ref    "Promoted implicit byref" <System.ValueTuple`2[int,System.__Canon]>
;  V10 cse0         [V10,T03] (  3,  3   )    long  ->  rax         "CSE #01: aggressive"
;
; Lcl frame size = 40

G_M33081_IG01:  ;; offset=0x0000
       sub      rsp, 40
       vxorps   xmm4, xmm4, xmm4
       vmovdqu  xmmword ptr [rsp+0x08], xmm4
       xor      eax, eax
       mov      qword ptr [rsp+0x18], rax
       mov      qword ptr [rsp+0x20], rcx
						;; size=26 bbWeight=1 PerfScore 4.83
G_M33081_IG02:  ;; offset=0x001A
       mov      rax, qword ptr [rcx+0x40]
       mov      rcx, qword ptr [rax]
       mov      qword ptr [rsp+0x08], rcx
						;; size=12 bbWeight=1 PerfScore 5.00
G_M33081_IG03:  ;; offset=0x0026
       vmovdqu  xmm0, xmmword ptr [rdx]
       vmovdqu  xmmword ptr [rsp+0x10], xmm0
						;; size=10 bbWeight=1 PerfScore 5.00
G_M33081_IG04:  ;; offset=0x0030
       mov      rcx, qword ptr [rsp+0x08]
       lea      rdx, bword ptr [rsp+0x08]
       xor      r8, r8
       cmp      rcx, qword ptr [rax+0x08]
       cmovne   rdx, r8
       test     rdx, rdx
       setne    al
       movzx    rax, al
						;; size=30 bbWeight=1 PerfScore 6.50
G_M33081_IG05:  ;; offset=0x004E
       add      rsp, 40
       ret      
						;; size=5 bbWeight=1 PerfScore 1.25

; Total bytes of code 83, prolog size 26, PerfScore 22.58, instruction count 21, allocated bytes for code 83 (MethodHash=0b7f7ec6) for method Program:Test[System.ValueTuple`2[int,System.__Canon],System.ValueTuple`2[int,System.__Canon]](System.ValueTuple`2[int,System.__Canon]):ubyte (FullOpts)
; ============================================================

AndyAyersMS · 2024-06-12T20:30:03Z

Will run mihu bot once we're a bit further along in testing, to get a sense of diffs.

jakobbotsch · 2024-06-12T22:18:13Z

src/coreclr/jit/lclmorph.cpp

+                }
+                INDEBUG(TopValue(0).Consume());
+                PopValue();


This has to call EscapeValue in the non-transformed case (that might be the reason for the asserts)

jakobbotsch · 2024-06-13T02:11:20Z

We stack allocate in #58554 but the local box is exposed, possibly because of a ?: -- need to review if we get conservative handling these in local morph.

So asm is not that pretty yet

; Assembly listing for method Program:Test[System.ValueTuple`2[int,System.__Canon],System.ValueTuple`2[int,System.__Canon]](System.ValueTuple`2[int,System.__Canon]):ubyte (FullOpts)
; Emitting BLENDED_CODE for X86 with AVX512 - Windows
; FullOpts code
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; Final local variable assignments
;
;  V00 TypeCtx      [V00,T01] (  4,  4   )    long  ->  rcx         single-def
;  V01 arg0         [V01,T00] (  3,  6   )   byref  ->  rdx         single-def
;# V02 OutArgs      [V02    ] (  1,  1   )  struct ( 0) [rsp+0x00]  do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;* V03 tmp1         [V03    ] (  0,  0   )    long  ->  zero-ref    class-hnd exact "Single-def Box Helper" <System.ValueTuple`2[int,System.__Canon]>
;* V04 tmp2         [V04    ] (  0,  0   )   byref  ->  zero-ref    "ISINST eval op1"
;  V05 tmp3         [V05,T02] (  2,  4   )   byref  ->  rdx         class-hnd "spilling qmarkNull" <System.ValueTuple`2[int,System.__Canon]>
;  V06 tmp4         [V06    ] (  4,  4   )  struct (24) [rsp+0x08]  do-not-enreg[XSF] must-init addr-exposed "stack allocated boxed value class temp" <System.Collections.Generic.KeyValuePair`2[long,System.ValueTuple`2[int,System.__Canon]]>
;* V07 tmp5         [V07    ] (  0,  0   )     ref  ->  zero-ref    "field V01.Item2 (fldOffset=0x0)" P-INDEP
;* V08 tmp6         [V08    ] (  0,  0   )     int  ->  zero-ref    "field V01.Item1 (fldOffset=0x8)" P-INDEP
;* V09 tmp7         [V09    ] (  0,  0   )  struct (16) zero-ref    "Promoted implicit byref" <System.ValueTuple`2[int,System.__Canon]>
;  V10 cse0         [V10,T03] (  3,  3   )    long  ->  rax         "CSE #01: aggressive"
;
; Lcl frame size = 40

G_M33081_IG01:  ;; offset=0x0000
       sub      rsp, 40
       vxorps   xmm4, xmm4, xmm4
       vmovdqu  xmmword ptr [rsp+0x08], xmm4
       xor      eax, eax
       mov      qword ptr [rsp+0x18], rax
       mov      qword ptr [rsp+0x20], rcx
						;; size=26 bbWeight=1 PerfScore 4.83
G_M33081_IG02:  ;; offset=0x001A
       mov      rax, qword ptr [rcx+0x40]
       mov      rcx, qword ptr [rax]
       mov      qword ptr [rsp+0x08], rcx
						;; size=12 bbWeight=1 PerfScore 5.00
G_M33081_IG03:  ;; offset=0x0026
       vmovdqu  xmm0, xmmword ptr [rdx]
       vmovdqu  xmmword ptr [rsp+0x10], xmm0
						;; size=10 bbWeight=1 PerfScore 5.00
G_M33081_IG04:  ;; offset=0x0030
       mov      rcx, qword ptr [rsp+0x08]
       lea      rdx, bword ptr [rsp+0x08]
       xor      r8, r8
       cmp      rcx, qword ptr [rax+0x08]
       cmovne   rdx, r8
       test     rdx, rdx
       setne    al
       movzx    rax, al
						;; size=30 bbWeight=1 PerfScore 6.50
G_M33081_IG05:  ;; offset=0x004E
       add      rsp, 40
       ret      
						;; size=5 bbWeight=1 PerfScore 1.25

; Total bytes of code 83, prolog size 26, PerfScore 22.58, instruction count 21, allocated bytes for code 83 (MethodHash=0b7f7ec6) for method Program:Test[System.ValueTuple`2[int,System.__Canon],System.ValueTuple`2[int,System.__Canon]](System.ValueTuple`2[int,System.__Canon]):ubyte (FullOpts)
; ============================================================

With this PR + #103391 the disassembly for this function becomes

; Assembly listing for method C:Test[System.ValueTuple`2[int,System.__Canon],System.ValueTuple`2[int,System.__Canon]](System.ValueTuple`2[int,System.__Canon]):ubyte (FullOpts)
; Emitting BLENDED_CODE for X64 with AVX - Windows
; FullOpts code
; optimized code
; rsp based frame
; partially interruptible
; No PGO data
; Final local variable assignments
;
;  V00 TypeCtx      [V00,T00] (  4,  4   )    long  ->  rcx         single-def
;* V01 arg0         [V01    ] (  0,  0   )   byref  ->  zero-ref    single-def
;# V02 OutArgs      [V02    ] (  1,  1   )  struct ( 0) [rsp+0x00]  do-not-enreg[XS] addr-exposed "OutgoingArgSpace"
;* V03 tmp1         [V03    ] (  0,  0   )    long  ->  zero-ref    class-hnd exact "Single-def Box Helper" <System.ValueTuple`2[int,System.__Canon]>
;* V04 tmp2         [V04    ] (  0,  0   )   byref  ->  zero-ref    "ISINST eval op1"
;* V05 tmp3         [V05    ] (  0,  0   )     int  ->  zero-ref    "spilling qmarkNull"
;* V06 tmp4         [V06    ] (  0,  0   )  struct (24) zero-ref    do-not-enreg[SF] "stack allocated boxed value class temp" <System.Collections.Generic.KeyValuePair`2[long,System.ValueTuple`2[int,System.__Canon]]>
;* V07 tmp5         [V07    ] (  0,  0   )     ref  ->  zero-ref    "field V01.Item2 (fldOffset=0x0)" P-INDEP
;* V08 tmp6         [V08    ] (  0,  0   )     int  ->  zero-ref    "field V01.Item1 (fldOffset=0x8)" P-INDEP
;  V09 tmp7         [V09,T02] (  2,  2   )    long  ->  rcx         "V06.[000..008)"
;* V10 tmp8         [V10    ] (  0,  0   )     ref  ->  zero-ref    "V06.[008..016)"
;* V11 tmp9         [V11    ] (  0,  0   )     int  ->  zero-ref    "V06.[016..020)"
;* V12 tmp10        [V12    ] (  0,  0   )  struct (16) zero-ref    "Promoted implicit byref" <System.ValueTuple`2[int,System.__Canon]>
;  V13 cse0         [V13,T01] (  3,  3   )    long  ->  rax         "CSE #01: aggressive"
;
; Lcl frame size = 8

G_M43406_IG01:  ;; offset=0x0000
       push     rax
       mov      qword ptr [rsp], rcx
                                                ;; size=5 bbWeight=1 PerfScore 2.00
G_M43406_IG02:  ;; offset=0x0005
       mov      rax, qword ptr [rcx+0x40]
       mov      rcx, qword ptr [rax]
       cmp      rcx, qword ptr [rax+0x08]
       setne    al
       movzx    rax, al
                                                ;; size=17 bbWeight=1 PerfScore 8.25
G_M43406_IG03:  ;; offset=0x0016
       add      rsp, 8
       ret
                                                ;; size=5 bbWeight=1 PerfScore 1.25

; Total bytes of code 27, prolog size 5, PerfScore 11.50, instruction count 9, allocated bytes for code 27 (MethodHash=e76e5671) for method C:Test[System.ValueTuple`2[int,System.__Canon],System.ValueTuple`2[int,System.__Canon]](System.ValueTuple`2[int,System.__Canon]):ubyte (FullOpts)
; ============================================================

jakobbotsch · 2024-06-28T14:58:44Z

src/coreclr/jit/objectalloc.cpp

+                if (lhsType != newType)
+                {
+                    lhs->ChangeType(newType);
+                }
+
+                if (rhsType != newType)
+                {
+                    rhs->ChangeType(newType);
+                }
+
+                parent->ChangeType(newType);


It still seems odd to me to forcefully retype an arbitrary sibling node of an ancestor, but maybe all TYP_REF JIT IR nodes really do have equivalently typed TYP_BYREF nodes.

For example, this will change TYP_REF LCL_VAR(local whose LclVarDsc is TYP_REF) to TYP_BYREF LCL_VAR(local whose LclVarDsc is TYP_REF)... That will hit this assert:

runtime/src/coreclr/jit/lclvars.cpp

Lines 4691 to 4694 in fcc2f58

// Check that the LCL_VAR node has the same type as the underlying variable, save a few mismatches we allow.

assert(tree->TypeIs(varDsc->TypeGet(), genActualType(varDsc)) ||

(tree->TypeIs(TYP_BYREF) && (varDsc->TypeGet() == TYP_I_IMPL)) || // Created by inliner substitution.

(tree->TypeIs(TYP_INT) && (varDsc->TypeGet() == TYP_LONG))); // Created by "optNarrowTree".

Not sure if it will actually cause issues downstream, but it feels like we should insert some form of cast to maintain IR invariants. Potentially in quite a few places in object stack allocation. Anyway, something to think about, we don't need to block on this given that QMARKs have very restricted shapes.

Happy to reconsider ... I added this because of assertion failures but don't recall the specific case and now can't reproduce it. Not clear to me how the JIT could create a TYP_REF GT_COLON, at least not one that this code would see.

Here's the motivating example -- from the object stack allocation tests

[000035] DA-X------- * STORE_LCL_VAR ref V03 tmp3 [000034] ---X------- \--* QMARK ref [000025] ----------- if +--* EQ int [000024] ----------- | +--* LCL_VAR ref V02 tmp2 [000023] ----------- | \--* CNS_INT ref null [000033] ---X------- if \--* COLON ref [000031] ---X------- else +--* QMARK ref [000027] ---X------- if | +--* NE int [000026] #--X------- | | +--* IND long [000022] ----------- | | | \--* LCL_VAR ref V02 tmp2 [000021] H---------- | | \--* CNS_INT(h) long 0x7ffe34d5e970 class ObjectStackAllocation.SimpleClassB [000030] ----------- if | \--* COLON ref [000028] ----------- else | +--* LCL_VAR ref V02 tmp2 [000029] ----------- then | \--* CNS_INT ref null [000032] ----------- then \--* CNS_INT ref null

We stack allocate the ref class, and so retype tmp2 to byref. This propagates up and if we don't retype sibling [000029] we hit asserts.

I think it may be the case that the mistyped sibling must be null... I can check for that.

jakobbotsch

LGTM, a comment on why the retyping feels wrong to me.

AndyAyersMS · 2024-06-28T15:49:01Z

@MichalStrehovsky any feedback? Think you got added as a reviewer automatically.

AndyAyersMS · 2024-06-28T17:15:10Z

LGTM, a comment on why the retyping feels wrong to me.

Turns out we only need to retype nulls, hopefully that's a bit more palatable.

AndyAyersMS · 2024-06-28T19:30:23Z

CI seems stuck...

AndyAyersMS · 2024-06-28T21:39:05Z

@dotnet/dnceng is CI stuck? No evident progress here in 2 hours.

mmitche · 2024-06-28T21:40:09Z

@dotnet/dnceng is CI stuck? No evident progress here in 2 hours.

GitHub had an outage.

hez2010 · 2024-06-30T15:47:58Z

btw I'm seeing this PR closes #11192 as well, but how about const sized arrays and strings?
this could be super helpful as some existing code with params T[] can be optimized without need of introducing a new overload params ROS<T>
and as for strings, it has the potential of eliminating extra allocation due to usage to string.Split and etc.

MichalStrehovsky · 2024-07-01T05:21:34Z

@MichalStrehovsky any feedback? Think you got added as a reviewer automatically.

I made myself "owner" of files under tools\common\JitInterface because I don't want to miss PRs that change it. This looks good to me, there's not much to review in the areas I know about.

Is crossgen2/nativeAOT implementation tracked somewhere? Do you have plans for it?

AndyAyersMS · 2024-07-01T13:46:34Z

Is crossgen2/nativeAOT implementation tracked somewhere? Do you have plans for it?

I added a note to #11192. Jan says it should not be hard, so perhaps I can try and get it implemented soon, though it does not do much right now.

For NAOT: once we have proper interprocedural support, it's possible that this will find a lot of unescaped objects.

jkotas · 2024-07-03T06:53:41Z

I added a note to #11192. Jan says it should not be hard, so perhaps I can try and get it implemented soon,

I have opened #104350 to have this tracked since #11192 is closed.

#103361 resurrected this long-deleted ifdef (crossgen1-specific code).

AndyAyersMS and others added 9 commits June 10, 2024 15:35

wip

e333c8f

post rebase fixes

5deb676

wip

877927a

revert layout changes, rely on KVP instead

0a7f3d4

trying to enable promotion

56500c3

Small fixes

f9e6e47

Update src/coreclr/jit/lclmorph.cpp

67402d4

Merge pull request #6 from jakobbotsch/stack-allocate-unescaped-boxes…

5a08c84

…-lclmorph Minor fixes

format

350d7ca

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 12, 2024

dotnet-policy-service bot assigned AndyAyersMS Jun 12, 2024

jkotas reviewed Jun 12, 2024

View reviewed changes

avoid escaping nullcheck addrs

0328665

AndyAyersMS mentioned this pull request Jun 12, 2024

JIT should support custom ClassLayout instances with GC refs #103362

Open

jakobbotsch reviewed Jun 12, 2024

View reviewed changes

AndyAyersMS added 3 commits June 12, 2024 19:39

fix nullcheck handling

315916d

fix colon retyping

91b83d3

metrics, enable for ref classes too

bcc7c3a

jakobbotsch reviewed Jun 28, 2024

View reviewed changes

jakobbotsch approved these changes Jun 28, 2024

View reviewed changes

more surgical sibling type update for qmark/colon

7af3d98

AndyAyersMS closed this Jun 28, 2024

AndyAyersMS reopened this Jun 28, 2024

mmitche closed this Jun 28, 2024

mmitche reopened this Jun 28, 2024

build-analysis bot mentioned this pull request Jun 29, 2024

The "RestoreTask" task returned false but did not log an error dotnet/dnceng#3100

Open

3 tasks

AndyAyersMS merged commit 53a8a01 into dotnet:main Jul 1, 2024
97 of 101 checks passed

This was referenced Jul 1, 2024

JIT: optimizations for multi-use boxes #9118

Closed

Undoing boxing doesn't work with C# 7 pattern matching #10195

Closed

Jit should avoid allocations for T1 is T2 pattern in generics #58554

Closed

jkotas mentioned this pull request Jul 3, 2024

[NativeAOT] Enable object stack allocation #104350

Closed

EgorBo mentioned this pull request Jul 4, 2024

Enable "Allocate on stack" for NAOT/R2R #104411

Merged

jkotas mentioned this pull request Jul 6, 2024

Avoid boxing in System.ObjectModel.KeydCollection during startup #104504

Closed

AndyAyersMS mentioned this pull request Jul 8, 2024

A simple method that performs unboxing is considered unprofitable inlinee, preventing box stack allocation #104479

Open

MichalStrehovsky added a commit that referenced this pull request Jul 11, 2024

Delete FEATURE_READYTORUN_COMPILER ifdef

b9e6cc0

#103361 resurrected this long-deleted ifdef (crossgen1-specific code).

MichalStrehovsky mentioned this pull request Jul 11, 2024

Delete FEATURE_READYTORUN_COMPILER ifdef #104727

Merged

MichalStrehovsky added a commit that referenced this pull request Jul 12, 2024

Delete FEATURE_READYTORUN_COMPILER ifdef (#104727)

c09ec65

#103361 resurrected this long-deleted ifdef (crossgen1-specific code).

AndyAyersMS mentioned this pull request Jul 16, 2024

Stack Allocation Enhancements #104936

Open

20 tasks

github-actions bot locked and limited conversation to collaborators Aug 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stack allocate unescaped boxes #103361

Stack allocate unescaped boxes #103361

AndyAyersMS commented Jun 12, 2024 •

edited by EgorBo

Loading

AndyAyersMS commented Jun 12, 2024

AndyAyersMS commented Jun 12, 2024

jakobbotsch commented Jun 12, 2024

AndyAyersMS commented Jun 12, 2024

jkotas Jun 12, 2024

jkotas Jun 12, 2024

AndyAyersMS Jun 12, 2024

jkotas Jun 12, 2024

AndyAyersMS Jun 12, 2024

AndyAyersMS Jun 12, 2024

AndyAyersMS commented Jun 12, 2024

AndyAyersMS commented Jun 12, 2024

AndyAyersMS commented Jun 12, 2024

AndyAyersMS commented Jun 12, 2024

jakobbotsch Jun 12, 2024

jakobbotsch commented Jun 13, 2024 •

edited

Loading

jakobbotsch Jun 28, 2024

jakobbotsch Jun 28, 2024 •

edited

Loading

AndyAyersMS Jun 28, 2024

AndyAyersMS Jun 28, 2024

jakobbotsch left a comment

AndyAyersMS commented Jun 28, 2024

AndyAyersMS commented Jun 28, 2024

AndyAyersMS commented Jun 28, 2024

AndyAyersMS commented Jun 28, 2024

mmitche commented Jun 28, 2024

hez2010 commented Jun 30, 2024

MichalStrehovsky commented Jul 1, 2024

AndyAyersMS commented Jul 1, 2024

jkotas commented Jul 3, 2024

	// Check that the LCL_VAR node has the same type as the underlying variable, save a few mismatches we allow.
	assert(tree->TypeIs(varDsc->TypeGet(), genActualType(varDsc)) \|\|
	(tree->TypeIs(TYP_BYREF) && (varDsc->TypeGet() == TYP_I_IMPL)) \|\| // Created by inliner substitution.
	(tree->TypeIs(TYP_INT) && (varDsc->TypeGet() == TYP_LONG))); // Created by "optNarrowTree".

Stack allocate unescaped boxes #103361

Stack allocate unescaped boxes #103361

Conversation

AndyAyersMS commented Jun 12, 2024 • edited by EgorBo Loading

AndyAyersMS commented Jun 12, 2024

AndyAyersMS commented Jun 12, 2024

jakobbotsch commented Jun 12, 2024

AndyAyersMS commented Jun 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndyAyersMS commented Jun 12, 2024

AndyAyersMS commented Jun 12, 2024

AndyAyersMS commented Jun 12, 2024

AndyAyersMS commented Jun 12, 2024

Choose a reason for hiding this comment

jakobbotsch commented Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

jakobbotsch Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakobbotsch left a comment

Choose a reason for hiding this comment

AndyAyersMS commented Jun 28, 2024

AndyAyersMS commented Jun 28, 2024

AndyAyersMS commented Jun 28, 2024

AndyAyersMS commented Jun 28, 2024

mmitche commented Jun 28, 2024

hez2010 commented Jun 30, 2024

MichalStrehovsky commented Jul 1, 2024

AndyAyersMS commented Jul 1, 2024

jkotas commented Jul 3, 2024

AndyAyersMS commented Jun 12, 2024 •

edited by EgorBo

Loading

jakobbotsch commented Jun 13, 2024 •

edited

Loading

jakobbotsch Jun 28, 2024 •

edited

Loading