Skip to content

Commit

Permalink
JIT: Model GT_RETURN kills with contained operand (#111230)
Browse files Browse the repository at this point in the history
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:
```asm
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
```
  • Loading branch information
jakobbotsch authored Jan 12, 2025
1 parent a69d74a commit 010e61a
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions src/coreclr/jit/lsrabuild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4300,6 +4300,16 @@ int LinearScan::BuildReturn(GenTree* tree)
return 1;
}
}
else
{
// In other cases we require the incoming operand to be in the
// right register(s) when we build the use(s), and thus we do not
// need to model that as a kill. However, in this case we have a
// contained operand. Codegen will move it to the right return
// registers; thus they will be killed.
regMaskTP killedRegs = compiler->compRetTypeDesc.GetABIReturnRegs(compiler->info.compCallConv);
buildKillPositionsForNode(tree, currentLoc + 1, killedRegs);
}

// No kills or defs.
return 0;
Expand Down

0 comments on commit 010e61a

Please sign in to comment.