JIT x64: `GT_XCHG`/`GT_XADD` target-prefer the delay-free address operand, leaving a redundant `mov` ahead of every `xchg`

[AndyAyersMS]

On xarch, xchg r/m, r and lock xadd r/m, r return the previous memory value in the register operand that supplied the data. CodeGen::genLockedInstructions reflects that with a canSkip copy:

GetEmitter()->emitIns_Mov(INS_mov, size, node->GetRegNum(), data->GetRegNum(), /* canSkip */ true);

so the copy disappears whenever LSRA assigns the def the same register as the data use.

But in the shared GT_XADD/GT_XCHG case of LinearScan::BuildNode (src/coreclr/jit/lsraxarch.cpp), the target preference is set to the address use, which was just marked delay-free:

RefPosition* addrUse = BuildUse(addr);
setDelayFree(addrUse);
tgtPrefUse = addrUse;
assert(!data->isContained());
BuildUse(data, varTypeIsByte(tree) ? RBM_BYTE_REGS.GetIntRegSet() : RBM_NONE);

A delay-free use stays live across the def, so it interferes with the def and that preference can never be satisfied — it is effectively dead. The data use, which is the register the instruction writes its result into, gets no preference at all. The result is an extra mov reg, dataReg ahead of the atomic whenever the data operand is a computed last-use value whose register LSRA did not happen to reuse.

Affected source shape: any Interlocked.Exchange / Interlocked.Add whose result is consumed and whose data operand is a computed, last-use value.

Minimal repro

using System;
using System.Runtime.CompilerServices;
using System.Threading;

public static class B01B
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    public static void LongXchg(ref long loc, long[] d, long v, long w)
    {
        d[0] = Interlocked.Exchange(ref loc, v * w);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    public static void ToArray(ref int loc, int[] d, int i, int v, int w)
    {
        d[i] = Interlocked.Exchange(ref loc, v * w);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    public static void ToOut(ref int loc, out int o, int v, int w)
    {
        o = Interlocked.Exchange(ref loc, v + w);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    public static void TwoXchg(ref int a, ref int b, int[] d, int v)
    {
        d[0] = Interlocked.Exchange(ref a, v);
        d[1] = Interlocked.Exchange(ref b, v);
    }

    public static void Main()
    {
        int loc = 1, o;
        int loc2 = 7;
        long lloc = 3;
        int[] d = new int[4];
        long[] ld = new long[4];

        ToArray(ref loc, d, 0, 2, 3);
        ToOut(ref loc, out o, 4, 5);
        LongXchg(ref lloc, ld, 2, 3);
        TwoXchg(ref loc, ref loc2, d, 11);

        Console.WriteLine(loc + " " + loc2 + " " + o + " " + d[0] + " " + d[1] + " " + ld[0]);
    }
}

Run with DOTNET_TieredCompilation=0 DOTNET_ReadyToRun=0 DOTNET_JitDisasmDiffable=1 and DOTNET_JitDisasm=LongXchg on x64 Release.

Current codegen

B01B:LongXchg(byref,long[],long,long), x64 Release, FullOpts — 35 bytes:

G_M000_IG02:
       imul     r8, r9
       mov      rax, r8
       xchg     qword ptr [rcx], rax
       cmp      dword ptr [rdx+0x08], 0
       jbe      SHORT G_M000_IG04
       mov      qword ptr [rdx+0x10], rax

mov rax, r8 is pure overhead: xchg would have returned the old value in r8 just as well.

Expected codegen

Same method with the def preferenced onto the data use — 32 bytes:

G_M000_IG02:
       imul     r8, r9
       xchg     qword ptr [rcx], r8
       cmp      dword ptr [rdx+0x08], 0
       jbe      SHORT G_M000_IG04
       mov      qword ptr [rdx+0x10], r8

Impact

Repro deltas (from the ; Total bytes of code footers, base → diff): LongXchg 35 → 32 (−3), TwoXchg 46 → 44 (−2), ToArray 39 → 38 (−1), ToOut 11 → 10 (−1). Mixed, LoadedData, ConstData and XaddToArray are byte-identical — notably GT_XADD showed no benefit in this repro, because the Interlocked.Add(...) - (v * w) reconstruction keeps the addend live across the exchange.

Corpus measurement — SuperPMI asmdiffs over 12 windows.x64 collections (aspire.nativeaot, aspnet2.run, benchmarks.run, benchmarks.run_pgo, benchmarks.run_pgo_optrepeat, coreclr_tests.run, libraries.crossgen2, libraries.pmi, libraries_tests.run, libraries_tests_no_tiered_compilation.run, realworld.run, smoke_tests.nativeaot) at commit 4856f0c16f89d8cc625a62ee7bab7ed4afe14f9d, Release JIT, exit 0 with no asserts or replay failures:

Contexts with a textual diff	3,062
Paired base bytes → diff bytes	4,147,098 → 4,136,449
Net code size	−10,649 bytes (−0.257 % of those methods' bytes)
Improved / regressed / size-neutral	3,059 / 1 / 2
Largest improvement	libraries.crossgen2 20282.dasm 4370 → 4326 (−44)
Only regression	libraries_tests_no_tiered_compilation.run 111192.dasm 250 → 251 (+1)

Per-collection deltas range from −186 (aspire.nativeaot) to −3,866 (libraries_tests.run). Against the full replayed corpus (1.16 GB of base code) this is −0.0009 %.

SuperPMI's own per-collection byte totals sum to −10,396 instead of −10,649; the 253-byte gap is confined to libraries_tests.run and is consistent with the 9 contexts the diff JIT compiled but the base JIT did not (base-missing 38 vs diff-missing 29). Every other collection agrees exactly between the two views. Diff-missing ≤ base-missing in all 12 collections, so no new missing contexts are attributable to the change.

Measurement limitations, stated explicitly:

• No PerfScore figure exists. The Release JIT emits no PerfScore, so SuperPMI reported Total PerfScore of base: 0.0 / of diff: 0.0 for every collection, and jit-analyze against these .dasm files warns "the base metric is 0, the diff metric is 0" and reports 0 improved / 0 regressed for both CodeSize and PerfScore. All aggregates above were therefore computed directly from the ; Total bytes of code N footer of each generated .dasm pair. Whether any context regresses in PerfScore is unknown; a checked JIT build is needed to answer that.
• No throughput or timing data. No BenchmarkDotNet run, no tpdiff, no instruction counts, no wall-clock measurement of either the generated code or the JIT itself. Code size is a static metric; the honest claim is "one fewer mov r64, r64 per affected exchange and −10 KB of corpus code", not a measured speedup.
• Windows x64 only. No Linux x64, no x86 (which shares this code path), and no DOTNET_JitStressRegs run.

Notes

• Scope is src/coreclr/jit/lsraxarch.cpp only, in the shared GT_XADD/GT_XCHG case; arm/arm64 do not use this path. The GT_XORR/GT_XAND cmpxchg-loop path above it is untouched.
• Correctness: which architectural register carries the data in and the old value out is unobservable to managed code. The address use keeps its setDelayFree, so the def still cannot alias the address register, and the existing assert((node->GetRegNum() != addr->GetRegNum()) || (node->GetRegNum() == data->GetRegNum())) in genLockedInstructions continues to hold. The byte-register mask for varTypeIsByte must be preserved when the BuildUse is moved.
• Remaining risks: (1) PerfScore was never measured, so a PerfScore-negative context cannot be ruled out; (2) no JitStressRegs / non-Windows / x86 validation — a preference is opportunistic and may not hold under register pressure; (3) GT_XADD shows no measured repro benefit, so whether the lock xadd reconstruction pattern also wants this is a separate open question.
• Related but distinct: JIT: preference the operand of unary RMW nodes to the target reg on xarch #132102 (preference the operand of unary RMW neg/not/bswap nodes) applied the same idea to a different node set and is already in the base commit. PR JIT: Fix GC reporting for xarch interlocked reference results #133855 ("JIT: Fix GC reporting for xarch interlocked reference results") touches emitxarch.cpp GC liveness for the xchg/cmpxchg result register, is also already in base, and confirms the result register is the data register.

Prototype patch

Experimental patch

diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp
index 2695fe7a850..db93a93c6b8 100644
--- a/src/coreclr/jit/lsraxarch.cpp
+++ b/src/coreclr/jit/lsraxarch.cpp
@@ -528,10 +528,12 @@ int LinearScan::BuildNode(GenTree* tree)
             assert(!addr->isContained());
             RefPosition* addrUse = BuildUse(addr);
             setDelayFree(addrUse);
-            tgtPrefUse = addrUse;
             assert(!data->isContained());
-            BuildUse(data, varTypeIsByte(tree) ? RBM_BYTE_REGS.GetIntRegSet() : RBM_NONE);
-            srcCount = 2;
+            // `xchg`/`lock xadd` return the previous value in the register that supplied `data`, so
+            // prefer the def onto the data use. The address use is delay-free and therefore
+            // interferes with the def, so it can never satisfy a target preference.
+            tgtPrefUse = BuildUse(data, varTypeIsByte(tree) ? RBM_BYTE_REGS.GetIntRegSet() : RBM_NONE);
+            srcCount   = 2;
             assert(dstCount == 1);
             BuildDef(tree);
         }

Note

This issue was generated with GitHub Copilot.

[dotnet-policy-service]

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