From d0e31bb7d6fc99d56e825774c21ec4614a555a3a Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 25 Jun 2026 11:38:12 -0700 Subject: [PATCH 1/2] [arm64] Avoid sign-extending TYP_INT register moves Revise ins_Move_Extend to emit mov instead of a sxtw for TYP_INT moves. Add an optional dstType arg to assert the move is valid and non-widening. Fixes #129052. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/codegen.h | 7 +- src/coreclr/jit/instr.cpp | 25 +++--- .../IntMoveNoSignExtend.cs | 78 +++++++++++++++++++ .../IntMoveNoSignExtend.csproj | 17 ++++ 4 files changed, 113 insertions(+), 14 deletions(-) create mode 100644 src/tests/JIT/opt/InstructionCombining/IntMoveNoSignExtend.cs create mode 100644 src/tests/JIT/opt/InstructionCombining/IntMoveNoSignExtend.csproj diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index f8f9eddf0c6322..a377184d08a4a7 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -1416,8 +1416,9 @@ class CodeGen final : public CodeGenInterface regNumber dstReg, regNumber srcReg, bool canSkip, - emitAttr size = EA_UNKNOWN, - insFlags flags = INS_FLAGS_DONT_CARE); + emitAttr size = EA_UNKNOWN, + insFlags flags = INS_FLAGS_DONT_CARE, + var_types dstType = TYP_UNKNOWN); void inst_RV_RV(instruction ins, regNumber reg1, @@ -1637,7 +1638,7 @@ class CodeGen final : public CodeGenInterface bool arm_Valid_Imm_For_Add_SP(target_ssize_t imm); #endif - instruction ins_Move_Extend(var_types srcType, bool srcInReg); + instruction ins_Move_Extend(var_types srcType, bool srcInReg, var_types dstType = TYP_UNKNOWN); instruction ins_Copy(var_types dstType); instruction ins_Copy(regNumber srcReg, var_types dstType); diff --git a/src/coreclr/jit/instr.cpp b/src/coreclr/jit/instr.cpp index 500e94832339cc..8307895790e99b 100644 --- a/src/coreclr/jit/instr.cpp +++ b/src/coreclr/jit/instr.cpp @@ -790,9 +790,10 @@ void CodeGen::inst_Mov_Extend(var_types srcType, regNumber srcReg, bool canSkip, emitAttr size, - insFlags flags /* = INS_FLAGS_DONT_CARE */) + insFlags flags /* = INS_FLAGS_DONT_CARE */, + var_types dstType /* = TYP_UNKNOWN */) { - instruction ins = ins_Move_Extend(srcType, srcInReg); + instruction ins = ins_Move_Extend(srcType, srcInReg, dstType); if (size == EA_UNKNOWN) { @@ -1920,9 +1921,17 @@ bool CodeGenInterface::validImmForBL(ssize_t addr) * Parameters * srcType - source type * srcInReg - whether source is in a register + * dstType - type the value will be consumed as; defaults to srcType. Must share srcType's + * actual type, as genuine widening (e.g. int -> long) must use a cast instead. */ -instruction CodeGen::ins_Move_Extend(var_types srcType, bool srcInReg) +instruction CodeGen::ins_Move_Extend(var_types srcType, bool srcInReg, var_types dstType /* = TYP_UNKNOWN */) { + if (dstType == TYP_UNKNOWN) + { + dstType = srcType; + } + assert(genActualType(srcType) == genActualType(dstType)); + if (varTypeUsesIntReg(srcType)) { instruction ins = INS_invalid; @@ -2005,14 +2014,8 @@ instruction CodeGen::ins_Move_Extend(var_types srcType, bool srcInReg) } else { - if (srcType == TYP_INT) - { - ins = INS_sxtw; - } - else - { - ins = INS_mov; - } + // A TYP_INT value only needs its low 32 bits; mov Wd, Wn zero extends the rest. + ins = INS_mov; } } } diff --git a/src/tests/JIT/opt/InstructionCombining/IntMoveNoSignExtend.cs b/src/tests/JIT/opt/InstructionCombining/IntMoveNoSignExtend.cs new file mode 100644 index 00000000000000..23181f5bf0c8c3 --- /dev/null +++ b/src/tests/JIT/opt/InstructionCombining/IntMoveNoSignExtend.cs @@ -0,0 +1,78 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using Xunit; + +namespace TestIntMoveNoSignExtend +{ + public class Program + { + static int s_value = 7; + + [MethodImpl(MethodImplOptions.NoInlining)] + static int GetInt() => s_value; + + [MethodImpl(MethodImplOptions.NoInlining)] + static void Sink(int x) => s_value ^= x; + + // A reg-to-reg move of a TYP_INT value (here the call result kept in a callee-saved register + // across the following call) must not be sign extended to 64 bits: the upper bits are never + // observed for an int, so a plain 'mov Wd, Wn' suffices instead of 'sxtw Wd, Wn'. + [MethodImpl(MethodImplOptions.NoInlining)] + static int IntMove() + { + //ARM64-NOT: sxtw {{w[0-9]+}}, {{w[0-9]+}} + int a = GetInt(); + Sink(0); + return a + GetInt(); + } + + // A genuine int -> long widening must still sign extend with 'sxtw Xd, Wn'. + [MethodImpl(MethodImplOptions.NoInlining)] + static long Widen(int x) + { + //ARM64-FULL-LINE: sxtw {{x[0-9]+}}, {{w[0-9]+}} + return x; + } + + [Fact] + public static int TestEntryPoint() + { + int result = 100; + + s_value = 7; + if (IntMove() != 14) + { + result = -1; + } + + // Negative values must round-trip correctly through the move. + s_value = -123456; + if (IntMove() != -246912) + { + result = -1; + } + + s_value = int.MinValue; + // int.MinValue + int.MinValue overflows to 0 in unchecked int arithmetic. + if (IntMove() != 0) + { + result = -1; + } + + // Genuine widening must preserve the sign. + if (Widen(-7) != -7L) + { + result = -1; + } + if (Widen(int.MinValue) != int.MinValue) + { + result = -1; + } + + return result; + } + } +} diff --git a/src/tests/JIT/opt/InstructionCombining/IntMoveNoSignExtend.csproj b/src/tests/JIT/opt/InstructionCombining/IntMoveNoSignExtend.csproj new file mode 100644 index 00000000000000..9a71cd1712d753 --- /dev/null +++ b/src/tests/JIT/opt/InstructionCombining/IntMoveNoSignExtend.csproj @@ -0,0 +1,17 @@ + + + + true + + + None + True + + + + true + + + + + From 54e41ee55a15208a3d9dfe3a2caa4c5ab096b556 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 25 Jun 2026 12:11:47 -0700 Subject: [PATCH 2/2] Address review feedback Broaden the int/long mov comment to mention the EA_4BYTE zero extension, and simplify the test's sxtw disasm check to a bare ARM64-NOT. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/instr.cpp | 3 ++- src/tests/JIT/opt/InstructionCombining/IntMoveNoSignExtend.cs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/instr.cpp b/src/coreclr/jit/instr.cpp index 8307895790e99b..b697347644aa41 100644 --- a/src/coreclr/jit/instr.cpp +++ b/src/coreclr/jit/instr.cpp @@ -2014,7 +2014,8 @@ instruction CodeGen::ins_Move_Extend(var_types srcType, bool srcInReg, var_types } else { - // A TYP_INT value only needs its low 32 bits; mov Wd, Wn zero extends the rest. + // A signed int/long value already fills its register width, so a plain mov + // suffices; for int the EA_4BYTE mov also zero extends the unused upper bits. ins = INS_mov; } } diff --git a/src/tests/JIT/opt/InstructionCombining/IntMoveNoSignExtend.cs b/src/tests/JIT/opt/InstructionCombining/IntMoveNoSignExtend.cs index 23181f5bf0c8c3..05d2b71de536e2 100644 --- a/src/tests/JIT/opt/InstructionCombining/IntMoveNoSignExtend.cs +++ b/src/tests/JIT/opt/InstructionCombining/IntMoveNoSignExtend.cs @@ -19,11 +19,11 @@ public class Program // A reg-to-reg move of a TYP_INT value (here the call result kept in a callee-saved register // across the following call) must not be sign extended to 64 bits: the upper bits are never - // observed for an int, so a plain 'mov Wd, Wn' suffices instead of 'sxtw Wd, Wn'. + // observed for an int, so a plain 'mov' suffices instead of an 'sxtw'. [MethodImpl(MethodImplOptions.NoInlining)] static int IntMove() { - //ARM64-NOT: sxtw {{w[0-9]+}}, {{w[0-9]+}} + //ARM64-NOT: sxtw int a = GetInt(); Sink(0); return a + GetInt();