Repository navigation
JIT: Support bitwise field extractions from parameter registers - #112740
Conversation
Recent work now allows us to finally add support for the backend to
extract fields out of parameters without spilling them to stack.
Previously this was only supported when the fields mapped cleanly to
registers.
A win-x64 example:
```csharp
static int Foo(int? foo)
{
return foo.HasValue ? foo.Value : 0;
}
```
```diff
; Method Program:Foo(System.Nullable`1[int]):int (FullOpts)
G_M19236_IG01: ;; offset=0x0000
- mov qword ptr [rsp+0x08], rcx
- ;; size=5 bbWeight=0.50 PerfScore 0.50
+ ;; size=0 bbWeight=0.50 PerfScore 0.00
-G_M19236_IG02: ;; offset=0x0005
- movzx rcx, cl
- xor eax, eax
- test ecx, ecx
- cmovne eax, dword ptr [rsp+0x0C]
- ;; size=12 bbWeight=0.50 PerfScore 1.38
+G_M19236_IG02: ;; offset=0x0000
+ movzx rax, cl
+ shr rcx, 32
+ xor edx, edx
+ test eax, eax
+ mov eax, edx
+ cmovne eax, ecx
+ ;; size=16 bbWeight=0.50 PerfScore 0.88
-G_M19236_IG03: ;; offset=0x0011
+G_M19236_IG03: ;; offset=0x0010
ret
;; size=1 bbWeight=0.50 PerfScore 0.50
-; Total bytes of code: 18
-
+; Total bytes of code: 17
```
Another win-x64 example:
```csharp
static float Sum(PointF p)
{
return p.X + p.Y;
}
```
```diff
; Method Program:Sum(System.Drawing.PointF):float (FullOpts)
G_M48891_IG01: ;; offset=0x0000
- mov qword ptr [rsp+0x08], rcx
- ;; size=5 bbWeight=1 PerfScore 1.00
+ ;; size=0 bbWeight=1 PerfScore 0.00
-G_M48891_IG02: ;; offset=0x0005
- vmovss xmm0, dword ptr [rsp+0x08]
- vaddss xmm0, xmm0, dword ptr [rsp+0x0C]
- ;; size=12 bbWeight=1 PerfScore 8.00
+G_M48891_IG02: ;; offset=0x0000
+ vmovd xmm0, ecx
+ shr rcx, 32
+ vmovd xmm1, ecx
+ vaddss xmm0, xmm0, xmm1
+ ;; size=16 bbWeight=1 PerfScore 7.50
-G_M48891_IG03: ;; offset=0x0011
+G_M48891_IG03: ;; offset=0x0010
ret
;; size=1 bbWeight=1 PerfScore 1.00
-; Total bytes of code: 18
+; Total bytes of code: 17
```
An arm64 example:
```csharp
static bool Test(Memory<int> mem)
{
return mem.Length > 10;
}
```
```diff
; Method Program:Test(System.Memory`1[int]):ubyte (FullOpts)
G_M53448_IG01: ;; offset=0x0000
- stp fp, lr, [sp, #-0x20]!
+ stp fp, lr, [sp, #-0x10]!
mov fp, sp
- stp x0, x1, [fp, #0x10] // [V00 arg0], [V00 arg0+0x08]
- ;; size=12 bbWeight=1 PerfScore 2.50
+ ;; size=8 bbWeight=1 PerfScore 1.50
-G_M53448_IG02: ;; offset=0x000C
- ldr w0, [fp, #0x1C] // [V00 arg0+0x0c]
+G_M53448_IG02: ;; offset=0x0008
+ lsr x0, x1, dotnet#32
cmp w0, dotnet#10
cset x0, gt
- ;; size=12 bbWeight=1 PerfScore 3.00
+ ;; size=12 bbWeight=1 PerfScore 2.00
-G_M53448_IG03: ;; offset=0x0018
- ldp fp, lr, [sp], #0x20
+G_M53448_IG03: ;; offset=0x0014
+ ldp fp, lr, [sp], #0x10
ret lr
;; size=8 bbWeight=1 PerfScore 2.00
-; Total bytes of code: 32
+; Total bytes of code: 28
```
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
/azp run runtime-coreclr superpmi-replay, runtime-coreclr superpmi-diffs |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run runtime-coreclr superpmi-diffs |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Based on the new `FIELD_LIST` support for returns this PR adds support for the
JIT to combine smaller fields via bitwise operations when returned, instead of
spilling these to stack.
win-x64 examples:
```csharp
static int? Test()
{
return Environment.TickCount;
}
```
```diff
call System.Environment:get_TickCount():int
- mov dword ptr [rsp+0x24], eax
- mov byte ptr [rsp+0x20], 1
- mov rax, qword ptr [rsp+0x20]
- ;; size=19 bbWeight=1 PerfScore 4.00
+ mov eax, eax
+ shl rax, 32
+ or rax, 1
+ ;; size=15 bbWeight=1 PerfScore 2.00
```
(the `mov eax, eax` is unnecessary, but not that simple to get rid of)
```csharp
static (int x, float y) Test(int x, float y)
{
return (x, y);
}
```
```diff
- mov dword ptr [rsp], ecx
- vmovss dword ptr [rsp+0x04], xmm1
- mov rax, qword ptr [rsp]
+ vmovd eax, xmm1
+ shl rax, 32
+ mov ecx, ecx
+ or rax, rcx
;; size=13 bbWeight=1 PerfScore 3.00
```
An arm64 example:
```csharp
static Memory<int> ToMemory(int[] arr)
{
return arr.AsMemory();
}
```
```diff
G_M45070_IG01: ;; offset=0x0000
- stp fp, lr, [sp, #-0x20]!
+ stp fp, lr, [sp, #-0x10]!
mov fp, sp
- str xzr, [fp, #0x10] // [V03 tmp2]
- ;; size=12 bbWeight=1 PerfScore 2.50
-G_M45070_IG02: ;; offset=0x000C
+ ;; size=8 bbWeight=1 PerfScore 1.50
+G_M45070_IG02: ;; offset=0x0008
cbz x0, G_M45070_IG06
;; size=4 bbWeight=1 PerfScore 1.00
-G_M45070_IG03: ;; offset=0x0010
- str x0, [fp, #0x10] // [V07 tmp6]
- str wzr, [fp, #0x18] // [V08 tmp7]
- ldr x0, [fp, #0x10] // [V07 tmp6]
- ldr w0, [x0, #0x08]
- str w0, [fp, #0x1C] // [V09 tmp8]
- ;; size=20 bbWeight=0.80 PerfScore 6.40
-G_M45070_IG04: ;; offset=0x0024
- ldp x0, x1, [fp, #0x10] // [V03 tmp2], [V03 tmp2+0x08]
- ;; size=4 bbWeight=1 PerfScore 3.00
-G_M45070_IG05: ;; offset=0x0028
- ldp fp, lr, [sp], #0x20
+G_M45070_IG03: ;; offset=0x000C
+ ldr w1, [x0, #0x08]
+ ;; size=4 bbWeight=0.80 PerfScore 2.40
+G_M45070_IG04: ;; offset=0x0010
+ mov w1, w1
+ mov x2, xzr
+ orr x1, x2, x1, LSL #32
+ ;; size=12 bbWeight=1 PerfScore 2.00
+G_M45070_IG05: ;; offset=0x001C
+ ldp fp, lr, [sp], #0x10
ret lr
;; size=8 bbWeight=1 PerfScore 2.00
-G_M45070_IG06: ;; offset=0x0030
- str xzr, [fp, #0x10] // [V07 tmp6]
- str xzr, [fp, #0x18]
+G_M45070_IG06: ;; offset=0x0024
+ mov x0, xzr
+ mov w1, wzr
b G_M45070_IG04
- ;; size=12 bbWeight=0.20 PerfScore 0.60
+ ;; size=12 bbWeight=0.20 PerfScore 0.40
```
(sneak peek -- this codegen requires some supplementary changes, and there's
additional opportunities here)
This is the return counterpart to #112740. That PR has a bunch of regressions
that makes it look like we need to support returns/call arguments first, before
we try to support parameters.
There's a few follow-ups here:
- Support for float->float insertions (when a float value needs to be returned
as the 1st, 2nd, .... field of a SIMD register)
- Support for coalescing memory loads, particularly because the fields of the
`FIELD_LIST` come from a promoted struct that ended up DNER. In those cases we
should be able to recombine the fields back to a single large field, instead
of combining them with bitwise operations.
- Support for constant folding the bitwise insertions. This requires some more
constant folding support in lowering.
- The JIT has lots of (now outdated) restrictions based around multi-reg returns
that get in the way. Lifting these should improve things considerably.
|
Spot checking some regressions with #118778 included:
27865.dasm - Microsoft.EntityFrameworkCore.Metadata.Internal.InternalForeignKeyBuilder:Microsoft.EntityFrameworkCore.Metadata.Builders.IConventionForeignKeyBuilder.OnDelete(System.Nullable`1[int],bool):Microsoft.EntityFrameworkCore.Metadata.Builders.IConventionForeignKeyBuilder:this (FullOpts)@@ -20,7 +20,9 @@
;* V10 tmp7 [V10 ] ( 0, 0 ) int -> zero-ref "field V05.value (fldOffset=0x4)" P-INDEP
;* V11 tmp8 [V11 ] ( 0, 0 ) ubyte -> zero-ref "field V07.hasValue (fldOffset=0x0)" P-INDEP
;* V12 tmp9 [V12 ] ( 0, 0 ) int -> zero-ref "field V07.value (fldOffset=0x4)" P-INDEP
-; V13 rat0 [V13,T02] ( 3, 3 ) long -> rdx "V01.rdx"
+; V13 tmp10 [V13,T04] ( 2, 2 ) ubyte -> rax "V01.[000..001)"
+; V14 tmp11 [V14,T05] ( 2, 2 ) int -> rdx "V01.[004..008)"
+; V15 rat0 [V15,T02] ( 4, 4 ) long -> rdx "V01.rdx"
;
; Lcl frame size = 0
@@ -28,19 +30,24 @@ G_M3648_IG01: ; bbWeight=1, gcrefRegs=0000 {}, byrefRegs=0000 {}, byref,
;; size=0 bbWeight=1 PerfScore 0.00
G_M3648_IG02: ; bbWeight=1, gcrefRegs=0002 {rcx}, byrefRegs=0000 {}, byref
; gcrRegs +[rcx]
- mov eax, 1
- mov r10d, 2
+ movzx rax, dl
+ shr rdx, 32
+ mov r10d, 1
+ mov r9d, 2
test r8b, r8b
- mov r8d, r10d
- cmovne r8d, eax
+ mov r8d, r9d
+ cmovne r8d, r10d
+ mov edx, edx
+ shl rdx, 32
+ or rdx, rax
mov rax, qword ptr [rcx]
mov rax, qword ptr [rax+0x60]
- ;; size=28 bbWeight=1 PerfScore 5.25
+ ;; size=45 bbWeight=1 PerfScore 7.00
G_M3648_IG03: ; bbWeight=1, epilog, nogc, extend
tail.jmp [rax+0x38]<unknown method>
;; size=4 bbWeight=1 PerfScore 4.00
-; Total bytes of code 32, prolog size 0, PerfScore 9.25, instruction count 8, allocated bytes for code 32 (MethodHash=90bdf1bf) for method Microsoft.EntityFrameworkCore.Metadata.Internal.InternalForeignKeyBuilder:Microsoft.EntityFrameworkCore.Metadata.Builders.IConventionForeignKeyBuilder.OnDelete(System.Nullable`1[int],bool):Microsoft.EntityFrameworkCore.Metadata.Builders.IConventionForeignKeyBuilder:this (FullOpts)
+; Total bytes of code 49, prolog size 0, PerfScore 11.00, instruction count 13, allocated bytes for code 49 (MethodHash=90bdf1bf) for method Microsoft.EntityFrameworkCore.Metadata.Internal.InternalForeignKeyBuilder:Microsoft.EntityFrameworkCore.Metadata.Builders.IConventionForeignKeyBuilder.OnDelete(System.Nullable`1[int],bool):Microsoft.EntityFrameworkCore.Metadata.Builders.IConventionForeignKeyBuilder:this (FullOpts)
; ============================================================We end up promoting a Previously we did not promote it and end up copy propagating it such that the final call in this function takes This should eventually be better: the idea is that once old promotion is removed there is no point in promoting this local as a |
|
/azp run runtime-coreclr superpmi-diffs |
|
Azure Pipelines successfully started running 1 pipeline(s). |
446496.dasm - System.Numerics.Tensors.Tests.Helpers:DetermineTolerance[System.Half](System.Nullable`1[double],System.Nullable`1[float],System.Nullable`1[System.Half]):System.Nullable`1[System.Half] (Tier1)@@ -11,15 +11,16 @@
;
;* V00 arg0 [V00 ] ( 0, 0 ) byref -> zero-ref ld-addr-op single-def
;* V01 arg1 [V01 ] ( 0, 0 ) struct ( 8) zero-ref ld-addr-op single-def <System.Nullable`1[float]>
-; V02 arg2 [V02,T01] ( 3, 2 ) struct ( 8) [rsp+0x18] do-not-enreg[SF] ld-addr-op single-def <System.Nullable`1[System.Half]>
+; V02 arg2 [V02,T00] ( 4, 2 ) struct ( 8) [rsp+0x18] do-not-enreg[SF] ld-addr-op single-def <System.Nullable`1[System.Half]>
;* V03 loc0 [V03 ] ( 0, 0 ) struct ( 8) zero-ref ld-addr-op <System.Nullable`1[System.Half]>
;# V04 OutArgs [V04 ] ( 1, 1 ) struct ( 0) [rsp+0x00] do-not-enreg[XS] addr-exposed "OutgoingArgSpace" <Empty>
;* V05 tmp1 [V05 ] ( 0, 0 ) ubyte -> zero-ref "field V00.hasValue (fldOffset=0x0)" P-INDEP
;* V06 tmp2 [V06 ] ( 0, 0 ) double -> zero-ref "field V00.value (fldOffset=0x8)" P-INDEP
;* V07 tmp3 [V07 ] ( 0, 0 ) ubyte -> zero-ref single-def "field V03.hasValue (fldOffset=0x0)" P-INDEP
;* V08 tmp4 [V08 ] ( 0, 0 ) ushort -> zero-ref single-def "field V03.value (fldOffset=0x2)" P-INDEP
-;* V09 tmp5 [V09 ] ( 0, 0 ) struct (16) zero-ref "Promoted implicit byref" <System.Nullable`1[double]>
-; V10 rat0 [V10,T00] ( 5, 5 ) ubyte -> r8 "V02.r8"
+; V09 tmp5 [V09,T02] ( 3, 2 ) ubyte -> r8 single-def "V02.[000..001)"
+;* V10 tmp6 [V10 ] ( 0, 0 ) struct (16) zero-ref "Promoted implicit byref" <System.Nullable`1[double]>
+; V11 rat0 [V11,T01] ( 3, 3 ) long -> r8 "V02.r8"
;
; Lcl frame size = 0
@@ -36,13 +37,14 @@ G_M4475_IG03: ; bbWeight=1, epilog, nogc, extend
ret
;; size=1 bbWeight=1 PerfScore 1.00
G_M4475_IG04: ; bbWeight=0, gcVars=0000000000000000 {}, gcrefRegs=0000 {}, byrefRegs=0000 {}, gcvars, byref
+ mov byte ptr [rsp+0x18], r8b
mov eax, dword ptr [rsp+0x18]
- ;; size=4 bbWeight=0 PerfScore 0.00
+ ;; size=9 bbWeight=0 PerfScore 0.00
G_M4475_IG05: ; bbWeight=0, epilog, nogc, extend
ret
;; size=1 bbWeight=0 PerfScore 0.00
-; Total bytes of code 22, prolog size 0, PerfScore 3.75, instruction count 8, allocated bytes for code 22 (MethodHash=3ebaee84) for method System.Numerics.Tensors.Tests.Helpers:DetermineTolerance[System.Half](System.Nullable`1[double],System.Nullable`1[float],System.Nullable`1[System.Half]):System.Nullable`1[System.Half] (Tier1)
+; Total bytes of code 27, prolog size 0, PerfScore 3.75, instruction count 9, allocated bytes for code 27 (MethodHash=3ebaee84) for method System.Numerics.Tensors.Tests.Helpers:DetermineTolerance[System.Half](System.Nullable`1[double],System.Nullable`1[float],System.Nullable`1[System.Half]):System.Nullable`1[System.Half] (Tier1)
; ============================================================
Unwind Info:_hasValue, we can realize that it might be a good idea to promote _value as well, to avoid a partially promoted local. ABI considerations may play into this decision. |
_hasValue field of an incoming parameter nullable, but we do not promote the _value. We then later return it (as a LCL_FLD<nullable>), which means we now have to write the _hasValue field back before that return.
System.Collections.Generic.ArraySortHelper`1[Microsoft.CodeAnalysis.CSharp.Syntax.NullableContextState]:InternalBinarySearch(Microsoft.CodeAnalysis.CSharp.Syntax.NullableContextState[],int,int,Microsoft.CodeAnalysis.CSharp.Syntax.NullableContextState,System.Collections.Generic.IComparer`1[Microsoft.CodeAnalysis.CSharp.Syntax.NullableContextState]):int (Tier1)We now promote fields of a number of parameter structs, which leads to a bunch of bitwise extractions. It saves spilling in the prolog, but higher register pressure just leads to more callee saves. This might improve with more intelligent ABI handling; currently those field promotions happen because we see assignments between old promoted locals and the parameter. If everything was left up to physical promotion, and we then had ABI based promotion, then we may have chosen to promote this in a way that does not require bitwise operations. Overall I am not super worried about diffs like this, since the point of this PR is to get rid of spills in the general case by introducing bitwise operations to extract fields. I think in general that's the choice we want to make. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for bitwise field extractions from parameter registers, eliminating the need to spill parameters to stack when accessing struct fields. The implementation extends the JIT's ability to extract fields directly from parameter registers using bit operations like shifts and masks.
Key changes:
- Enhanced parameter register mapping logic to support bitwise field extractions
- Modified field access detection to allow non-exact register mappings
- Added support for extracting fields at arbitrary offsets within parameter registers
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/jit/promotion.cpp | Updated parameter register mapping detection to support bitwise extractions instead of requiring exact field-to-register alignment |
| src/coreclr/jit/lower.cpp | Replaced field access logic with comprehensive bitwise extraction implementation, including shift operations and type conversions |
|
cc @dotnet/jit-contrib PTAL @AndyAyersMS Diffs, diffs with old promotion disabled. Overall this is a size regression but perfscore improvement. That's expected since we are replacing spills (that are quite compact) with bitwise extraction sequences that are less so. I analyzed some of the regressions above. Many of them I will aim to improve as part of .NET 11; I think a lot of improvements will come from removing old promotion such that physical promotion can be smarter about promoting register passed parameters and arguments. For example, a struct value only used as a parameter/argument should be promoted as primitives that map to the registers that it is passed in. Today we often promote these as fields that require bitwise operations to extract/assemble because we see assignments to old promoted locals. |
|
/ba-g Issue is known, but build analysis is not unblocking |
|
Hmm, realized I never ran stress on this, so will do that first. |
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Recent work now allows us to finally add support for the backend to extract fields out of parameters without spilling them to stack. Previously this was only supported when the fields mapped cleanly to registers.
A win-x64 example:
Another win-x64 example:
An arm64 example:
Float -> float extractions that do not map cleanly is still not supported, but should be doable (via vector register extractions). Float -> int extractions are not supported, but I'm not sure we see these.
This is often not a code size improvement, but typically a perfscore improvement. Also this seems to have some bad interactions with call arguments since they do not yet support something similar, but hopefully that can be improved separately.
This should fix a number of issues that I need to go find.