Skip to content

JIT: Introduce LclVarDsc::lvIsMultiRegDest #113294

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

Merged
merged 1 commit into from
Mar 10, 2025

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Mar 8, 2025

With recent work to expand returned promoted locals into FIELD_LIST the only "whole references" of promoted locals we should see is when stored from a multi-reg node. This is the only knowledge the backend should need for correctness purposes, so introduce a bit to track this property, and switch the backend to check this instead.

The existing lvIsMultiRegRet is essentially this + whether the local is returned. We should be able to remove this, but it is currently used for some outdated correctness checks in old promotion, so keep it around for now.

This makes the new bitwise combination of returns kick in much more often for multireg returns. For example on win-arm64:

static Memory<int> ToMemory(int[] arr)
{
    return arr.AsMemory();
}
 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
-            cbz     x0, G_M45070_IG04
-                                                ;; 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]
-            b       G_M45070_IG05
-                                                ;; size=24 bbWeight=0.50 PerfScore 4.50
-G_M45070_IG04:  ;; offset=0x0028
-            str     xzr, [fp, #0x10]    // [V07 tmp6]
-            str     xzr, [fp, #0x18]
-                                                ;; size=8 bbWeight=0.50 PerfScore 1.00
-G_M45070_IG05:  ;; offset=0x0030
-            ldp     x0, x1, [fp, #0x10] // [V03 tmp2], [V03 tmp2+0x08]
-                                                ;; size=4 bbWeight=1 PerfScore 3.00
-G_M45070_IG06:  ;; offset=0x0034
-            ldp     fp, lr, [sp], #0x20
+						;; 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=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
+						;; size=8 bbWeight=1 PerfScore 2.00
+G_M45070_IG06:  ;; offset=0x0024
+            mov     x0, xzr
+            mov     w1, wzr
+            b       G_M45070_IG04
+						;; size=12 bbWeight=0.20 PerfScore 0.40
 
-; Total bytes of code 60, prolog size 12, PerfScore 14.00, instruction count 15, allocated bytes for code 60 (MethodHash=b2994ff1) for method Program:ToMemory(int[]):System.Memory`1[int] (FullOpts)
+; Total bytes of code 48, prolog size 8, PerfScore 9.30, instruction count 12, allocated bytes for code 48 (MethodHash=b2994ff1) for method Program:ToMemory(int[]):System.Memory`1[int] (FullOpts)

(which still can be improved)

With recent work to expand returned promoted locals into `FIELD_LIST`
the only "whole references" of promoted locals we should see is when
stored from a multi-reg node. This is the only knowledge the backend
should need for correctness purposes, so introduce a bit to track this
property, and switch the backend to check this instead.

The existing `lvIsMultiRegRet` is essentially this + whether the local
is returned. We should be able to remove this, but it is currently used
for some heuristics in old promotion, so keep it around for now.
@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 Mar 8, 2025
Copy link
Contributor

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

@jakobbotsch jakobbotsch marked this pull request as ready for review March 9, 2025 10:29
@Copilot Copilot AI review requested due to automatic review settings March 9, 2025 10:29
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @EgorBo

Diffs

@jakobbotsch jakobbotsch merged commit 1c9d5e3 into dotnet:main Mar 10, 2025
114 checks passed
@jakobbotsch jakobbotsch deleted the return-multireg-locals branch March 10, 2025 11:57
@github-actions github-actions bot locked and limited conversation to collaborators Apr 10, 2025
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 this pull request may close these issues.

2 participants