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

Minor fixes #6

Conversation

jakobbotsch
Copy link

Switch local morph to use its semantic reasoning about addresses in the GT_EQ/GT_NE cases. Add a missing return false.

@jakobbotsch
Copy link
Author

With this we manage to reduce the case in dotnet#9118 to

; 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)
; ============================================================

Looks like we don't manage to remove the frame entirely since we don't get rid of the heap allocation until post-lower DCE, so there is still a helper call during lowering when we're figuring out how much outgoing arg space we need.

@AndyAyersMS AndyAyersMS merged commit 5a08c84 into AndyAyersMS:StackAllocateUnescapedBoxes Jun 12, 2024
@jakobbotsch jakobbotsch deleted the stack-allocate-unescaped-boxes-lclmorph branch June 12, 2024 18:15
AndyAyersMS pushed a commit that referenced this pull request Sep 7, 2024
* bug #1: don't allow for values out of the SerializationRecordType enum range

* bug #2: throw SerializationException rather than KeyNotFoundException when the referenced record is missing or it points to a record of different type

* bug #3: throw SerializationException rather than FormatException when it's being thrown by BinaryReader (or sth else that we use)

* bug #4: document the fact that IOException can be thrown

* bug #5: throw SerializationException rather than OverflowException when parsing the decimal fails

* bug #6: 0 and 17 are illegal values for PrimitiveType enum

* bug dotnet#7: throw SerializationException when a surrogate character is read (so far an ArgumentException was thrown)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants