Skip to content

JIT: x64 epilog restores of callee-saved XMM registers always use RSP-relative addressing, paying a SIB byte (and sometimes a disp32) even when RBP is available #134906

Description

@AndyAyersMS

On win-x64, CodeGen::genRestoreCalleeSavedFltRegs (src/coreclr/jit/codegenxarch.cpp) picks REG_FPBASE only on the compLocallocUsed path; every other method unconditionally restores the callee-saved XMM registers with REG_SPBASE:

if (m_compiler->compLocallocUsed)
{
    regBase = REG_FPBASE;
    offset  = lclFrameSize - genSPtoFPdelta() - firstFPRegPadding - XMM_REGSIZE_BYTES;
}
else
{
    regBase = REG_SPBASE;
    offset  = lclFrameSize - firstFPRegPadding - XMM_REGSIZE_BYTES;
}

[rsp+disp] always requires a SIB byte, while [rbp+disp8] requires neither a SIB byte nor a disp32. When the method already has a frame pointer, the exact same stack slots are reachable off RBP, and the RBP form is 1-3 bytes shorter per restore. The addresses are identical: genFnEpilog calls genRestoreCalleeSavedFltRegs() before any stack teardown, so RSP is still at its post-prolog value and rbp == rsp + genSPtoFPdelta().

Minimal repro

using System;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

public static class Program
{
    [StructLayout(LayoutKind.Sequential, Size = 512)]
    public struct Scratch
    {
        public double Value;
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    public static void Fill(ref Scratch s) => s.Value = 1.25;

    [MethodImpl(MethodImplOptions.NoInlining)]
    public static void Observe()
    {
    }

    // EH frame + large local frame + four callee-saved XMM values => frame pointer is used
    // and the RSP-relative restores need a disp32.
    [MethodImpl(MethodImplOptions.NoInlining)]
    public static double C05_Work2(double a, double b, double c, double d, double e)
    {
        Scratch scratch = default;
        try
        {
            Fill(ref scratch);
            double p = a * 1.5;
            double q = b * 2.5;
            double r = c * 3.5;
            double t = d * 4.5;
            double u = e * 5.5;
            Observe();
            return p + q + r + t + u + scratch.Value;
        }
        finally
        {
            Observe();
        }
    }

    // Negative control: no frame pointer, so nothing should change here.
    [MethodImpl(MethodImplOptions.NoInlining)]
    public static double C05_NoFramePointer(double a, double b, double c, double d, double e)
    {
        Scratch scratch = default;
        Fill(ref scratch);
        double p = a * 1.5;
        double q = b * 2.5;
        double r = c * 3.5;
        double t = d * 4.5;
        double u = e * 5.5;
        Observe();
        return p + q + r + t + u + scratch.Value;
    }

    public static int Main()
    {
        double work = C05_Work2(1, 2, 3, 4, 5);
        double noFrame = C05_NoFramePointer(1, 2, 3, 4, 5);
        Console.WriteLine(work.ToString("R"));
        Console.WriteLine(noFrame.ToString("R"));
        return ((work == 63.75) && (noFrame == 63.75)) ? 100 : 101;
    }
}

Run with DOTNET_TieredCompilation=0 DOTNET_ReadyToRun=0 DOTNET_JitDisasm=C05_* DOTNET_JitDisasmWithCodeBytes=1 on win-x64.

Current codegen

C05_Work2 prolog establishes rbp = rsp + 0x270 after sub rsp, 624; the epilog then restores off RSP (8 bytes each: opcode + ModRM + SIB + disp32):

       vmovaps  xmm6, xmmword ptr [rsp+0x260]
       vmovaps  xmm7, xmmword ptr [rsp+0x250]
       vmovaps  xmm8, xmmword ptr [rsp+0x240]
       vmovaps  xmm9, xmmword ptr [rsp+0x230]
       add      rsp, 624
       pop      rbp
       ret

Expected codegen

The same physical slots addressed off the already-live frame pointer (5 bytes each: no SIB, disp8):

       vmovaps  xmm6, xmmword ptr [rbp-0x10]
       vmovaps  xmm7, xmmword ptr [rbp-0x20]
       vmovaps  xmm8, xmmword ptr [rbp-0x30]
       vmovaps  xmm9, xmmword ptr [rbp-0x40]
       add      rsp, 624
       pop      rbp
       ret

rsp+0x270-0x10 == rsp+0x260, and so on for each register, so the loaded addresses are unchanged.

Impact

Measured with the experimental patch below, Release x64 JIT at commit 4856f0c1.

Target deltas from the repro harness (base bytes -> diff bytes):

Method Base Diff Delta
C05_Work2 (large frame + EH) 304 292 -12
C05_SmallFrame (disp8 already, SIB removed) 173 169 -4
C05_Throwing (throws through the protected region) 337 325 -12
C05_NoFramePointer (negative control) 286 286 0
C05_Localloc (negative control) 244 244 0
C05_ManyRegs (last displacement below -128) 347 347 0

SuperPMI asmdiffs, aspire.nativeaot.windows.x64.checked.mch, win-x64:

  • 54,025 successful compiles, 0 missing, 0 failing
  • 35 contexts with diffs: 35 size improvements, 0 size regressions, 0 same-size
  • Base 12,091,248 bytes -> diff 12,091,080 bytes, net -168 bytes
  • PerfScore unchanged (relative geomean 1.0)

All 35 diffed assembly pairs were inspected; the only changed instruction lines are callee-saved movaps/vmovaps restores switching from [rsp+...] to [rbp-...].

Measurement limitations:

  • The authoritative corpus number above comes from a single collection, so -168 bytes (-0.0014% of diffed code) understates or overstates breadth by an unknown amount. A broader 12-collection run during development reported an aggregate of about -16.8 KB with no regressing collection, but that run's artifacts were not retained and it should be reproduced before relying on the figure.
  • No throughput (tpdiff) measurement and no microbenchmark were taken. The change removes bytes from the epilog and does not add instructions, and PerfScore was flat, but the runtime effect is expected to be in the noise.
  • Only win-x64 was measured. SysV x64 has no callee-saved XMM registers, so the path is inert there.

Notes

  • Scope is TARGET_AMD64; Windows x64 is where compCalleeFPRegsSavedMask is non-empty in practice.
  • Correctness rests on RSP being stable at this point in the epilog. genFnEpilog calls genRestoreCalleeSavedFltRegs() before any stack teardown, and the compLocallocUsed case is already handled by the pre-existing mandatory-RBP branch, so the new path never runs under localloc.
  • Save instructions, frame layout, register allocation, and instruction order are unchanged; only the epilog addressing form changes. Checked-JIT JitUnwindDump/JitEHDump output for the repro methods is identical between base and patched builds: the same UWOP_SAVE_XMM128 offsets, the same UWOP_ALLOC_LARGE/UWOP_PUSH_NONVOL rbp records, the same funclet unwind record, and the same EH table. Funclet epilogs are generated by genFuncletEpilog and do not call this helper.
  • The guard in the prototype is deliberately all-or-none over the whole restore sequence, so methods whose last displacement falls below -128 (such as C05_ManyRegs) are left entirely unchanged. A per-register policy could capture those partial wins but was not prototyped.
  • The prototype writes a negative FP-relative displacement through the existing unsigned offset variable, matching what the localloc path already does; the subsequent (offset % 16) == 0 assert and the loop's offset -= XMM_REGSIZE_BYTES are unaffected because 2^32 is a multiple of 16. A reviewer may still prefer to retype the variable as int for clarity.

Prototype patch

Experimental patch
diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp
index 36c4bcfd21f..2bc353b04ff 100644
--- a/src/coreclr/jit/codegenxarch.cpp
+++ b/src/coreclr/jit/codegenxarch.cpp
@@ -11482,6 +11482,25 @@ void CodeGen::genRestoreCalleeSavedFltRegs()
     {
         regBase = REG_SPBASE;
         offset  = lclFrameSize - firstFPRegPadding - XMM_REGSIZE_BYTES;
+
+#ifdef TARGET_AMD64
+        // SP is stable here (no localloc, and the epilog restores these registers before SP is
+        // adjusted), so when a frame pointer is established the very same slots can be addressed
+        // off RBP instead. RBP needs no SIB byte, so an RBP form that encodes as a disp8 is never
+        // longer than the RSP form of the same address, and is typically 1-3 bytes shorter.
+        if (isFramePointerUsed())
+        {
+            int spToFpDelta = genSPtoFPdelta();
+            int fpFirstOffs = static_cast<int>(offset) - spToFpDelta;
+            int fpLastOffs  = fpFirstOffs - static_cast<int>((genCountBits(regMask) - 1) * XMM_REGSIZE_BYTES);
+
+            if ((fpLastOffs >= -128) && (fpFirstOffs <= 127))
+            {
+                regBase = REG_FPBASE;
+                offset  = static_cast<unsigned>(fpFirstOffs);
+            }
+        }
+#endif // TARGET_AMD64
     }
 
 #ifdef TARGET_AMD64

Note

This issue was generated with GitHub Copilot.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMIperformanceuntriagedNew issue has not been triaged by the area owner

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions