From 9cc81637a1fffbda47ed7d42a8372b5c88100438 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Tue, 18 Apr 2023 12:42:00 -0700 Subject: [PATCH] [JIT] ARM32/ARM64 - Fixed zero/sign-extension for `GT_STORE_LCL_VAR` in codegen (#84716) * Try to zero-extend if it is possible * Added test * Fix arm32 as well * Remove branch check for arm32. Added more specific check for arm64. * Remove unwanted code * Trying to fix build * use general register check * Update Runtime_84693.cs --- src/coreclr/jit/codegenarm.cpp | 16 +++++++- src/coreclr/jit/codegenarm64.cpp | 12 +++++- src/coreclr/jit/emitarm64.cpp | 2 +- .../JitBlue/Runtime_84693/Runtime_84693.cs | 40 +++++++++++++++++++ .../Runtime_84693/Runtime_84693.csproj | 8 ++++ 5 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_84693/Runtime_84693.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_84693/Runtime_84693.csproj diff --git a/src/coreclr/jit/codegenarm.cpp b/src/coreclr/jit/codegenarm.cpp index 59f3210a0f58a..e8ebf46272fc4 100644 --- a/src/coreclr/jit/codegenarm.cpp +++ b/src/coreclr/jit/codegenarm.cpp @@ -1082,6 +1082,8 @@ void CodeGen::genCodeForStoreLclVar(GenTreeLclVar* tree) LclVarDsc* varDsc = compiler->lvaGetDesc(varNum); var_types targetType = varDsc->GetRegisterType(tree); + emitter* emit = GetEmitter(); + if (targetType == TYP_LONG) { genStoreLongLclVar(tree); @@ -1114,13 +1116,23 @@ void CodeGen::genCodeForStoreLclVar(GenTreeLclVar* tree) instruction ins = ins_StoreFromSrc(dataReg, targetType); emitAttr attr = emitTypeSize(targetType); - emitter* emit = GetEmitter(); emit->emitIns_S_R(ins, attr, dataReg, varNum, /* offset */ 0); } else // store into register (i.e move into register) { // Assign into targetReg when dataReg (from op1) is not the same register - inst_Mov(targetType, targetReg, dataReg, /* canSkip */ true); + // Only zero/sign extend if we are using general registers. + if (varTypeIsIntegral(targetType) && emit->isGeneralRegister(targetReg) && + emit->isGeneralRegister(dataReg)) + { + // We use 'emitActualTypeSize' as the instructions require 4BYTE. + inst_Mov_Extend(targetType, /* srcInReg */ true, targetReg, dataReg, /* canSkip */ true, + emitActualTypeSize(targetType)); + } + else + { + inst_Mov(targetType, targetReg, dataReg, /* canSkip */ true); + } } genUpdateLifeStore(tree, targetReg, varDsc); diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 6ecfb818a4daf..15e47ad6c346f 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -2940,7 +2940,17 @@ void CodeGen::genCodeForStoreLclVar(GenTreeLclVar* lclNode) else // store into register (i.e move into register) { // Assign into targetReg when dataReg (from op1) is not the same register - inst_Mov(targetType, targetReg, dataReg, /* canSkip */ true); + // Only zero/sign extend if we are using general registers. + if (varTypeIsIntegral(targetType) && emit->isGeneralRegister(targetReg) && emit->isGeneralRegister(dataReg)) + { + // We use 'emitActualTypeSize' as the instructions require 8BYTE or 4BYTE. + inst_Mov_Extend(targetType, /* srcInReg */ true, targetReg, dataReg, /* canSkip */ true, + emitActualTypeSize(targetType)); + } + else + { + inst_Mov(targetType, targetReg, dataReg, /* canSkip */ true); + } } genUpdateLifeStore(lclNode, targetReg, varDsc); } diff --git a/src/coreclr/jit/emitarm64.cpp b/src/coreclr/jit/emitarm64.cpp index 34ae7d9ccbb20..657f9645de04c 100644 --- a/src/coreclr/jit/emitarm64.cpp +++ b/src/coreclr/jit/emitarm64.cpp @@ -4185,7 +4185,7 @@ void emitter::emitIns_Mov( case INS_sxtw: { - assert(size == EA_8BYTE); + assert((size == EA_8BYTE) || (size == EA_4BYTE)); FALLTHROUGH; } diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_84693/Runtime_84693.cs b/src/tests/JIT/Regression/JitBlue/Runtime_84693/Runtime_84693.cs new file mode 100644 index 0000000000000..65e6202aabbe4 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_84693/Runtime_84693.cs @@ -0,0 +1,40 @@ +// 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.Collections; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using Xunit; + +public class Test +{ + // This is trying to verify that we zero-extend from the result of "(byte)(-s_2)". + public class Program + { + public static short s_2; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void Consume(int x) {} + + [MethodImpl(MethodImplOptions.NoInlining)] + public static int M8(byte arg0) + { + s_2 = 1; + arg0 = (byte)(-s_2); + var vr1 = arg0 & arg0; + Consume(vr1); + return vr1; + } + } + + [Fact] + public static int TestEntryPoint() { + var result = Test.Program.M8(1); + if (result != 255) + { + return 0; + } + return 100; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_84693/Runtime_84693.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_84693/Runtime_84693.csproj new file mode 100644 index 0000000000000..15edd99711a1a --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_84693/Runtime_84693.csproj @@ -0,0 +1,8 @@ + + + True + + + + + \ No newline at end of file