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

JIT: Model GT_RETURN kills with contained operand #111230

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Jan 9, 2025

When GT_RETURN has a contained operand (due to returning a DNER local), it will load the ABI return registers directly from stack. This will kill those registers. However, nothing was modelling this kill.

This was noticed while trying to ensure that GC information is correctly marked in codegen when going into epilogs (#107283). However, this is potential bad codegen even on its own: before this change LSRA might not properly spill locals that are required to be live around a GT_RETURN node even if they are getting killed by the GT_RETURN. A concrete case was seen in <StartupCode$FSharp-Core>.$Tasks+Using@148-5[System.__Canon]:Invoke under JitStressRegs=2, where the VM requests that the JIT keep "this" alive throughout the function. Before this change we get the following codegen:

G_M51753_IG05:        ; bbWeight=0.50, gcVars=0000000000000000 {}, gcrefRegs=0002 {x1}, byrefRegs=0000 {}, gcvars, byref
                             ; gcrRegs -[x0] +[x1]
                             ; GC ptr vars -{V00}
            stp     xzr, xzr, [fp, #0x38]
            ldp     x0, x1, [fp, #0x38]	// [V04 loc2], [V04 loc2+0x08]
                             ; gcrRegs -[x1] +[x0]
						;; size=8 bbWeight=0.50 PerfScore 2.00

where "this" is in x1 going into the block, and gets overridden by the return without being available somewhere else. After this change, the codegen becomes

G_M51753_IG05:        ; bbWeight=0.50, gcVars=0000000000000000 {}, gcrefRegs=0002 {x1}, byrefRegs=0000 {}, gcvars, byref
                             ; gcrRegs -[x0] +[x1]
                             ; GC ptr vars -{V00}
            str     x1, [fp, #0x20]	// [V00 this]
                             ; GC ptr vars +{V00}
            stp     xzr, xzr, [fp, #0x38]
            ldp     x0, x1, [fp, #0x38]	// [V04 loc2], [V04 loc2+0x08]
                             ; gcrRegs -[x1] +[x0]
						;; size=12 bbWeight=0.50 PerfScore 2.50

When `GT_RETURN` has a contained operand (due to returning a DNER
local), it will load the ABI return registers directly from stack. This
will kill those registers. However, nothing was modelling this kill.

This was noticed while trying to ensure that GC information is correctly
marked in codegen when going into epilogs. However, this is potential
bad codegen even on its own: before this change LSRA might not properly
spill locals that are required to be live around a `GT_RETURN` node even
if they are getting killed by the `GT_RETURN`. A concrete case was seen
in `<StartupCode$FSharp-Core>.$Tasks+Using@148-5[System.__Canon]:Invoke`
under JitStressRegs=2, where the VM requests that the JIT keep "this"
alive throughout the function. Before this change we get the following
codegen:
```
G_M51753_IG05:        ; bbWeight=0.50, gcVars=0000000000000000 {}, gcrefRegs=0002 {x1}, byrefRegs=0000 {}, gcvars, byref
                             ; gcrRegs -[x0] +[x1]
                             ; GC ptr vars -{V00}
            stp     xzr, xzr, [fp, #0x38]
            ldp     x0, x1, [fp, #0x38]	// [V04 loc2], [V04 loc2+0x08]
                             ; gcrRegs -[x1] +[x0]
						;; size=8 bbWeight=0.50 PerfScore 2.00

```
where "this"  is in `x1` going into the block, and gets overridden by
the return without being available somewhere else. After this change,
the codegen becomes
```asm
G_M51753_IG05:        ; bbWeight=0.50, gcVars=0000000000000000 {}, gcrefRegs=0002 {x1}, byrefRegs=0000 {}, gcvars, byref
                             ; gcrRegs -[x0] +[x1]
                             ; GC ptr vars -{V00}
            str     x1, [fp, #0x20]	// [V00 this]
                             ; GC ptr vars +{V00}
            stp     xzr, xzr, [fp, #0x38]
            ldp     x0, x1, [fp, #0x38]	// [V04 loc2], [V04 loc2+0x08]
                             ; gcrRegs -[x1] +[x0]
						;; size=12 bbWeight=0.50 PerfScore 2.50
```
@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 Jan 9, 2025
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

1 participant