From 140e519ef8fe494ac794f4d050dd87c23dc85f22 Mon Sep 17 00:00:00 2001 From: Carol Eidt Date: Tue, 5 Mar 2019 17:32:13 -0800 Subject: [PATCH] Correctly type SIMD stack values When `impSIMDPopStack` pops a struct value, it needs to retype the `OBJ` if it exists and doesn't match. Fix #22850 --- src/jit/simd.cpp | 32 ++++++++++------ .../JitBlue/GitHub_22850/GitHub_22850.cs | 43 ++++++++++++++++++++++ .../JitBlue/GitHub_22850/GitHub_22850.csproj | 34 +++++++++++++++++ 3 files changed, 97 insertions(+), 12 deletions(-) create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_22850/GitHub_22850.cs create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_22850/GitHub_22850.csproj diff --git a/src/jit/simd.cpp b/src/jit/simd.cpp index 3d265ee..4f3f8eb 100644 --- a/src/jit/simd.cpp +++ b/src/jit/simd.cpp @@ -1022,17 +1022,17 @@ const SIMDIntrinsicInfo* Compiler::getSIMDIntrinsicInfo(CORINFO_CLASS_HANDLE* in // Normalizes TYP_STRUCT value in case of GT_CALL, GT_RET_EXPR and arg nodes. // // Arguments: -// type - the type of value that the caller expects to be popped off the stack. -// expectAddr - if true indicates we are expecting type stack entry to be a TYP_BYREF. -// structType - the class handle to use when normalizing if it is not the same as the stack entry class handle; -// this can happen for certain scenarios, such as folding away a static cast, where we want the -// value popped to have the type that would have been returned. +// type - the type of value that the caller expects to be popped off the stack. +// expectAddr - if true indicates we are expecting type stack entry to be a TYP_BYREF. +// structHandle - the class handle to use when normalizing if it is not the same as the stack entry class handle; +// this can happen for certain scenarios, such as folding away a static cast, where we want the +// value popped to have the type that would have been returned. // // Notes: // If the popped value is a struct, and the expected type is a simd type, it will be set // to that type, otherwise it will assert if the type being popped is not the expected type. -GenTree* Compiler::impSIMDPopStack(var_types type, bool expectAddr, CORINFO_CLASS_HANDLE structType) +GenTree* Compiler::impSIMDPopStack(var_types type, bool expectAddr, CORINFO_CLASS_HANDLE structHandle) { StackEntry se = impPopStack(); typeInfo ti = se.seTypeInfo; @@ -1058,10 +1058,18 @@ GenTree* Compiler::impSIMDPopStack(var_types type, bool expectAddr, CORINFO_CLAS // If we have a ldobj of a SIMD local we need to transform it. if (tree->OperGet() == GT_OBJ) { - GenTree* addr = tree->gtOp.gtOp1; - if ((addr->OperGet() == GT_ADDR) && isSIMDTypeLocal(addr->gtOp.gtOp1)) + if (tree->AsObj()->gtClass != structHandle) { - tree = addr->gtOp.gtOp1; + // In this case we need to retain the GT_OBJ to retype the value. + tree->AsObj()->gtClass = structHandle; + } + else + { + GenTree* addr = tree->gtOp.gtOp1; + if ((addr->OperGet() == GT_ADDR) && isSIMDTypeLocal(addr->gtOp.gtOp1)) + { + tree = addr->gtOp.gtOp1; + } } } @@ -1077,12 +1085,12 @@ GenTree* Compiler::impSIMDPopStack(var_types type, bool expectAddr, CORINFO_CLAS { assert(ti.IsType(TI_STRUCT)); - if (structType == nullptr) + if (structHandle == nullptr) { - structType = ti.GetClassHandleForValueClass(); + structHandle = ti.GetClassHandleForValueClass(); } - tree = impNormStructVal(tree, structType, (unsigned)CHECK_SPILL_ALL); + tree = impNormStructVal(tree, structHandle, (unsigned)CHECK_SPILL_ALL); } // Now set the type of the tree to the specialized SIMD struct type, if applicable. diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_22850/GitHub_22850.cs b/tests/src/JIT/Regression/JitBlue/GitHub_22850/GitHub_22850.cs new file mode 100644 index 0000000..664d1c1 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_22850/GitHub_22850.cs @@ -0,0 +1,43 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. +// + +using System; +using System.Runtime.CompilerServices; +using System.Runtime.Intrinsics; +using System.Runtime.Intrinsics.X86; + +public static class GitHub_22850 +{ + static int Main(string[] args) + { + return test128((byte)90) ? 100 : -1; + } + + static unsafe bool test128(int i) + { + Vector128 v = Vector128.Create(i); + return MyEquals(ref v, Vector128.Create(i)); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static bool MyEquals(ref Vector128 left, Vector128 right) + { + if (Sse2.IsSupported) + { + Vector128 result = MyCompareEqual(left.AsByte(), right.AsByte()); + return Sse2.MoveMask(result) == 0b1111_1111_1111_1111; // We have one bit per element + } + + return true; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static Vector128 MyCompareEqual(this Vector128 left, Vector128 right) + { + return Sse2.CompareEqual(left, right); + } +} + + diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_22850/GitHub_22850.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_22850/GitHub_22850.csproj new file mode 100644 index 0000000..5b28be0 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_22850/GitHub_22850.csproj @@ -0,0 +1,34 @@ + + + + + Debug + AnyCPU + 2.0 + {2649FAFE-07BF-4F93-8120-BA9A69285ABB} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + None + False + True + + + + False + + + + + + + + + + + -- 2.7.4