JIT: Support more scaled addressing modes on arm64 #97921
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We currently support scaled addressing modes when the index also needs an extension through contained `BFIZ` nodes. However, we did not support scaled addressing modes if the index was 64 bits. This adds that support as a natural extension to the `GT_LEA`.
ghost
assigned 
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsWe currently support scaled addressing modes when the index also needs an extension through contained static int Foo(int[] p, nint index) => p[index];Before: G_M45892_IG02: ;; offset=0x0008
ldr w2, [x0, #0x08]
cmp x1, x2
bhs G_M45892_IG04
add x0, x0, #16
lsl x1, x1, #2
ldr w0, [x0, x1]After: G_M45892_IG02: ;; offset=0x0008
ldr w2, [x0, #0x08]
cmp x1, x2
bhs G_M45892_IG04
add x0, x0, #16
ldr w0, [x0, x1, LSL #2]Prerequisite for #97865
|
jakobbotsch
commented
Feb 3, 2024
jakobbotsch
commented
Feb 3, 2024
Diff results for #97921Assembly diffsAssembly diffs for linux/arm64 ran on windows/x64Diffs are based on 2,505,882 contexts (1,007,092 MinOpts, 1,498,790 FullOpts). MISSED contexts: base: 1,433 (0.06%), diff: 1,436 (0.06%) Overall (-250,480 bytes)
MinOpts (-22,888 bytes)
FullOpts (-227,592 bytes)
Assembly diffs for osx/arm64 ran on windows/x64Diffs are based on 2,270,095 contexts (932,669 MinOpts, 1,337,426 FullOpts). MISSED contexts: base: 772 (0.03%), diff: 775 (0.03%) Overall (-177,740 bytes)
MinOpts (-21,812 bytes)
FullOpts (-155,928 bytes)
Assembly diffs for windows/arm64 ran on windows/x64Diffs are based on 2,339,805 contexts (938,449 MinOpts, 1,401,356 FullOpts). MISSED contexts: base: 1,309 (0.06%), diff: 1,312 (0.06%) Overall (-181,504 bytes)
MinOpts (-21,684 bytes)
FullOpts (-159,820 bytes)
Details here Throughput diffsThroughput diffs for linux/arm64 ran on windows/x64Overall (-0.09% to -0.01%)
MinOpts (-0.03% to +0.01%)
FullOpts (-0.12% to -0.01%)
Throughput diffs for linux/x64 ran on windows/x64MinOpts (+0.00% to +0.01%)
Throughput diffs for osx/arm64 ran on windows/x64Overall (-0.07% to -0.01%)
MinOpts (-0.03% to +0.01%)
FullOpts (-0.10% to -0.01%)
Throughput diffs for windows/arm64 ran on windows/x64Overall (-0.07% to -0.01%)
MinOpts (-0.03% to +0.01%)
FullOpts (-0.07% to -0.01%)
Throughput diffs for windows/x64 ran on windows/x64MinOpts (+0.00% to +0.01%)
Details here Throughput diffs for linux/arm ran on windows/x86MinOpts (-0.02% to -0.00%)
Throughput diffs for windows/x86 ran on windows/x86Overall (+0.00% to +0.01%)
MinOpts (+0.00% to +0.02%)
FullOpts (+0.00% to +0.01%)
Details here |
Diff results for #97921Throughput diffsThroughput diffs for linux/arm64 ran on linux/x64Overall (-0.07% to +0.01%)
MinOpts (+0.01% to +0.08%)
FullOpts (-0.10% to +0.01%)
Throughput diffs for linux/x64 ran on linux/x64MinOpts (+0.00% to +0.01%)
Details here |
EgorBo
approved these changes
Feb 5, 2024
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, Fuzzlyn |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
The jitstress/libraries-jitstress failures look like #97892 although it's hard to know if anything is hidden in there... |
This was referenced Feb 5, 2024
This was referenced Feb 8, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We currently support scaled addressing modes when the index also needs an extension through contained
BFIZnodes. However, we did not support scaled addressing modes if the index was 64 bits. This adds that support as a natural extension to theGT_LEA.Before:
After:
Some regressions expected from fewer CSEs since we do not CSE address modes.
Prerequisite for #97865