Unify set-reg-to-imm functions and optimize with LEA at runtime on x64 #60228
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
This change * Unifies the genSetRegToImm and instGen_Set_Reg_To_Imm functions, removing the former. genSetRegToImm seems to be a variant that did not support relocations/handles, while the latter supported both. We now exclusively use the emitAttr to determine this. This was only a change in xarch since on ARM architectures the former function just forwarded to the latter. * Generates lea more often for handle GT_CNS_INT nodes on x64. Previously the logic generated only lea when a relocation was *required*, which is only the case for prejit. In particular, all VSD calls load an indirection cell that can use lea instead of the longer mov encoding during runtime JIT, which results in a nice code size decrease. * Changes ARM64 and ARM to handle byref-typed GT_CNS_INT nodes. This was fixed for xarch a few years back in 52a8890 but that change was not made to ARM architectures.
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsWhile working on #59602 I noticed that we did not use
|
|
No code size diffs on ARM64, but bullet 3 above means we see some GC info diffs: libraries.crossgen2.Linux.arm64.checked.mch:Detail diffsFor example: ; gcr arg pop 0
adrp x11, [HIGH RELOC #0xd1ffab1e] // static handle
add x11, x11, [LOW RELOC #0xd1ffab1e]
+ ; byrRegs +[x11]
lsl x0, x0, #1
add x0, x0, x11
; byrRegs +[x0]
ldrsh w0, [x0]
; byrRegs -[x0]
ldr w11, [fp,#36] // [V57 tmp46]
+ ; byrRegs -[x11]
add w1, w0, w11
str w1, [fp,#40] // [V56 tmp45]
mov w0, w1 G_M47710_IG02: ; gcrefRegs=80000 {x19}, byrefRegs=0000 {}, byref
adrp x20, [HIGH RELOC #0xd1ffab1e] // static handle
add x20, x20, [LOW RELOC #0xd1ffab1e]
+ ; byrRegs +[x20]
adrp x11, [HIGH RELOC #0xd1ffab1e] // function address
add x11, x11, [LOW RELOC #0xd1ffab1e]
ldr x0, [x11]
blr x0
- ; byrRegs +[x0 x20]
+ ; byrRegs +[x0]
; gcr arg pop 0
ldr x0, [x0,#0xd1ffab1e]
; gcrRegs +[x0]For x64 we see diffs in non-crossgen2 collections only, as expected. The summary is: aspnet.run.windows.x64.checked.mch:Detail diffscoreclr_tests.pmi.windows.x64.checked.mch:Detail diffslibraries.pmi.windows.x64.checked.mch:Detail diffslibraries_tests.pmi.windows.x64.checked.mch:Detail diffsSome example diffs: G_M11776_IG03: ; gcrefRegs=00000040 {rsi}, byrefRegs=00000000 {}, byref
mov rcx, gword ptr [rsi+32]
; gcrRegs +[rcx]
- mov r11, 0xD1FFAB1E
+ lea r11, [(reloc)]
call [hackishModuleName:hackishMethodName()]
; gcrRegs -[rcx] +[rax]
; gcr arg pop 0
@@ -81,11 +81,11 @@ G_M11776_IG03: ; gcrefRegs=00000040 {rsi}, byrefRegs=00000000 {}, byref
call CORINFO_HELP_ASSIGN_REF
; gcrRegs -[rax rdx]
; byrRegs -[rcx]
- ;; bbWeight=0.50 PerfScore 8.00
+ ;; bbWeight=0.50 PerfScore 8.12 G_M40268_IG02: ; gcrefRegs=00000002 {rcx}, byrefRegs=00000000 {}, byref
; gcrRegs +[rcx]
mov rcx, gword ptr [rcx+8]
- mov r11, 0xD1FFAB1E
- ;; bbWeight=1 PerfScore 2.25
+ lea r11, [(reloc)]
+ ;; bbWeight=1 PerfScore 2.50
G_M40268_IG03: ; , epilog, nogc, extend
tail.jmp [hackishModuleName:hackishMethodName()]
; gcr arg pop 0 |
While this is x64 only, EA_PTRSIZE seems more correct.
|
PTAL @dotnet/jit-contrib |
This was referenced Oct 19, 2021
|
windows/x64 improvement dotnet/perf-autofiling-issues#1907
|
|
another windows/x64 improvement - dotnet/perf-autofiling-issues#1934 |
This was referenced Oct 21, 2021
ghost
locked as resolved and limited conversation to collaborators
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.
While working on #59602 I noticed that we did not use
leafor indirection cells at runtime in x64.This change
removing the former. genSetRegToImm seems to be a variant that did not
support relocations/handles, while the latter supported both. We now
exclusively use the emitAttr to determine this. This was only a change
in xarch since on ARM architectures the former function just forwarded
to the latter.
Previously the logic generated only lea when a relocation was
required, which is only the case for prejit. In particular, all VSD
calls load an indirection cell that can use lea instead of the longer
mov encoding during runtime JIT, which results in a nice code size
decrease.
fixed for xarch a few years back in
52a8890 but that change was not made
to ARM architectures.