diff options
author | SingleAccretion <62474226+SingleAccretion@users.noreply.github.com> | 2022-06-07 03:48:33 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-06-07 03:48:33 +0300 |
commit | 09b2da88966a1bd3c2d23443223aed0a99903194 (patch) | |
tree | 6b33e1bd729b23876a40d7ce114ab7b1e01d1188 | |
parent | caff7b03a6dab33344430ee0e902deecb7bd9a9b (diff) |
Do not retype SIMD nodes (#70265)
* Do not retype SIMD nodes
It is not necessary: the only case where it is required, SIMD8 to
LONG bitcasts on Windows x64, is already handled by lowering.
It is dangerous: in case we CSE the retyped tree, its other uses
will be (effectively) retyped as well.
* Add a test
-rw-r--r-- | src/coreclr/jit/codegenarm64.cpp | 4 | ||||
-rw-r--r-- | src/coreclr/jit/codegenxarch.cpp | 4 | ||||
-rw-r--r-- | src/coreclr/jit/gentree.cpp | 9 | ||||
-rw-r--r-- | src/coreclr/jit/gentree.h | 28 | ||||
-rw-r--r-- | src/coreclr/jit/instr.cpp | 3 | ||||
-rw-r--r-- | src/coreclr/jit/morph.cpp | 19 | ||||
-rw-r--r-- | src/coreclr/jit/rationalize.cpp | 58 | ||||
-rw-r--r-- | src/coreclr/jit/valuenum.cpp | 23 | ||||
-rw-r--r-- | src/tests/JIT/Regression/JitBlue/Runtime_70124/Runtime_70124.cs | 24 | ||||
-rw-r--r-- | src/tests/JIT/Regression/JitBlue/Runtime_70124/Runtime_70124.csproj | 9 |
10 files changed, 53 insertions, 128 deletions
diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 9ad6220af8c..2b9d4be0abc 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -2324,12 +2324,8 @@ void CodeGen::genSetRegToConst(regNumber targetReg, var_types targetType, GenTre switch (tree->TypeGet()) { #if defined(FEATURE_SIMD) - case TYP_LONG: - case TYP_DOUBLE: case TYP_SIMD8: { - // TODO-1stClassStructs: do not retype SIMD nodes - if (vecCon->IsAllBitsSet()) { emit->emitIns_R_I(INS_mvni, attr, targetReg, 0, INS_OPTS_2S); diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 97c884f0e41..6302e12440b 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -537,12 +537,8 @@ void CodeGen::genSetRegToConst(regNumber targetReg, var_types targetType, GenTre switch (tree->TypeGet()) { #if defined(FEATURE_SIMD) - case TYP_LONG: - case TYP_DOUBLE: case TYP_SIMD8: { - // TODO-1stClassStructs: do not retype SIMD nodes - simd8_t constValue = vecCon->gtSimd8Val; CORINFO_FIELD_HANDLE hnd = emit->emitSimd8Const(constValue); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 91e4e0a72f4..53ac097ac13 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -2856,10 +2856,7 @@ AGAIN: } case TYP_SIMD8: - case TYP_DOUBLE: - case TYP_LONG: { - // TODO-1stClassStructs: do not retype SIMD nodes add = genTreeHashAdd(ulo32(add), vecCon->gtSimd8Val.u32[1]); add = genTreeHashAdd(ulo32(add), vecCon->gtSimd8Val.u32[0]); break; @@ -2873,7 +2870,6 @@ AGAIN: } add = genTreeHashAdd(ulo32(add), vecCon->GetSimdBaseType()); - add = genTreeHashAdd(ulo32(add), vecCon->GetSimdSize()); break; } @@ -6949,7 +6945,7 @@ GenTree* Compiler::gtNewSconNode(int CPX, CORINFO_MODULE_HANDLE scpHandle) GenTreeVecCon* Compiler::gtNewVconNode(var_types type, CorInfoType simdBaseJitType) { - GenTreeVecCon* vecCon = new (this, GT_CNS_VEC) GenTreeVecCon(type, simdBaseJitType, genTypeSize(type)); + GenTreeVecCon* vecCon = new (this, GT_CNS_VEC) GenTreeVecCon(type, simdBaseJitType); return vecCon; } @@ -11152,11 +11148,8 @@ void Compiler::gtDispConst(GenTree* tree) switch (vecCon->TypeGet()) { #if defined(FEATURE_SIMD) - case TYP_LONG: - case TYP_DOUBLE: case TYP_SIMD8: { - // TODO-1stClassStructs: do not retype SIMD nodes simd8_t simdVal = vecCon->gtSimd8Val; printf("<0x%08x, 0x%08x>", simdVal.u32[0], simdVal.u32[1]); break; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 4b09656cb28..df6452fb4e7 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -3270,7 +3270,6 @@ private: // size should be `gtType` and the handle should be looked up at callsites where required unsigned char gtSimdBaseJitType; // SIMD vector base JIT type - unsigned char gtSimdSize; // SIMD vector size in bytes public: CorInfoType GetSimdBaseJitType() const @@ -3286,17 +3285,6 @@ public: var_types GetSimdBaseType() const; - unsigned char GetSimdSize() const - { - return gtSimdSize; - } - - void SetSimdSize(unsigned simdSize) - { - gtSimdSize = (unsigned char)simdSize; - assert(gtSimdSize == simdSize); - } - #if defined(FEATURE_HW_INTRINSICS) static bool IsHWIntrinsicCreateConstant(GenTreeHWIntrinsic* node, simd32_t& simd32Val); @@ -3308,11 +3296,8 @@ public: switch (gtType) { #if defined(FEATURE_SIMD) - case TYP_LONG: - case TYP_DOUBLE: case TYP_SIMD8: { - // TODO-1stClassStructs: do not retype SIMD nodes return (gtSimd8Val.u64[0] == 0xFFFFFFFFFFFFFFFF); } @@ -3353,11 +3338,8 @@ public: switch (gtType) { #if defined(FEATURE_SIMD) - case TYP_LONG: - case TYP_DOUBLE: case TYP_SIMD8: { - // TODO-1stClassStructs: do not retype SIMD nodes return (left->gtSimd8Val.u64[0] == right->gtSimd8Val.u64[0]); } @@ -3395,11 +3377,8 @@ public: switch (gtType) { #if defined(FEATURE_SIMD) - case TYP_LONG: - case TYP_DOUBLE: case TYP_SIMD8: { - // TODO-1stClassStructs: do not retype SIMD nodes return (gtSimd8Val.u64[0] == 0x0000000000000000); } @@ -3428,14 +3407,11 @@ public: } } - GenTreeVecCon(var_types type, CorInfoType simdBaseJitType, unsigned simdSize) - : GenTree(GT_CNS_VEC, type) - , gtSimdBaseJitType((unsigned char)simdBaseJitType) - , gtSimdSize((unsigned char)simdSize) + GenTreeVecCon(var_types type, CorInfoType simdBaseJitType) + : GenTree(GT_CNS_VEC, type), gtSimdBaseJitType((unsigned char)simdBaseJitType) { assert(varTypeIsSIMD(type)); assert(gtSimdBaseJitType == simdBaseJitType); - assert(gtSimdSize == simdSize); } #if DEBUGGABLE_GENTREE diff --git a/src/coreclr/jit/instr.cpp b/src/coreclr/jit/instr.cpp index 4a3ab3e41ba..ae2831a3bdc 100644 --- a/src/coreclr/jit/instr.cpp +++ b/src/coreclr/jit/instr.cpp @@ -843,11 +843,8 @@ CodeGen::OperandDesc CodeGen::genOperandDesc(GenTree* op) switch (op->TypeGet()) { #if defined(FEATURE_SIMD) - case TYP_LONG: - case TYP_DOUBLE: case TYP_SIMD8: { - // TODO-1stClassStructs: do not retype SIMD nodes simd8_t constValue = op->AsVecCon()->gtSimd8Val; return OperandDesc(emit->emitSimd8Const(constValue)); } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 609569d145c..e35c7829b00 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -3404,11 +3404,24 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) argObj->gtType = structBaseType; } } - else + else if (argObj->OperIs(GT_LCL_FLD, GT_IND)) { - // Not a GT_LCL_VAR, so we can just change the type on the node + // We can just change the type on the node argObj->gtType = structBaseType; } + else + { +#ifdef FEATURE_SIMD + // We leave the SIMD8 <-> LONG (Windows x64) case to lowering. For SIMD8 <-> DOUBLE (Unix x64), + // we do not need to do anything as both types already use floating-point registers. + assert((argObj->TypeIs(TYP_SIMD8) && + ((structBaseType == TYP_LONG) || (structBaseType == TYP_DOUBLE))) || + argObj->TypeIs(structBaseType)); +#else // !FEATURE_SIMD + unreached(); +#endif // !FEATURE_SIMD + } + assert(varTypeIsEnregisterable(argObj->TypeGet()) || (makeOutArgCopy && varTypeIsEnregisterable(structBaseType))); } @@ -9922,7 +9935,7 @@ GenTree* Compiler::getSIMDStructFromField(GenTree* tree, { ret = obj; GenTreeVecCon* vecCon = obj->AsVecCon(); - *simdSizeOut = vecCon->GetSimdSize(); + *simdSizeOut = genTypeSize(vecCon); *simdBaseJitTypeOut = vecCon->GetSimdBaseJitType(); } } diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index 404759497a3..e6b7e1a4d92 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -670,16 +670,6 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge unsigned simdSize = simdNode->GetSimdSize(); var_types simdType = comp->getSIMDTypeForSize(simdSize); - // TODO-1stClassStructs: This should be handled more generally for enregistered or promoted - // structs that are passed or returned in a different register type than their enregistered - // type(s). - if (simdNode->gtType == TYP_I_IMPL && simdNode->GetSimdSize() == TARGET_POINTER_SIZE) - { - // This happens when it is consumed by a GT_RET_EXPR. - // It can only be a Vector2f or Vector2i. - assert(genTypeSize(simdNode->GetSimdBaseType()) == 4); - simdNode->gtType = TYP_SIMD8; - } // Certain SIMD trees require rationalizing. if (simdNode->AsSIMD()->GetSIMDIntrinsicId() == SIMDIntrinsicInitArray) { @@ -704,54 +694,6 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge break; #endif // FEATURE_SIMD -#ifdef FEATURE_HW_INTRINSICS - case GT_HWINTRINSIC: - { - GenTreeHWIntrinsic* hwIntrinsicNode = node->AsHWIntrinsic(); - - if (!hwIntrinsicNode->isSIMD()) - { - break; - } - - // TODO-1stClassStructs: This should be handled more generally for enregistered or promoted - // structs that are passed or returned in a different register type than their enregistered - // type(s). - if ((hwIntrinsicNode->gtType == TYP_I_IMPL) && (hwIntrinsicNode->GetSimdSize() == TARGET_POINTER_SIZE)) - { -#ifdef TARGET_ARM64 - // Special case for GetElement/ToScalar because they take Vector64<T> and return T - // and T can be long or ulong. - if (!((hwIntrinsicNode->GetHWIntrinsicId() == NI_Vector64_GetElement) || - (hwIntrinsicNode->GetHWIntrinsicId() == NI_Vector64_ToScalar))) -#endif - { - // This happens when it is consumed by a GT_RET_EXPR. - // It can only be a Vector2f or Vector2i. - assert(genTypeSize(hwIntrinsicNode->GetSimdBaseType()) == 4); - hwIntrinsicNode->gtType = TYP_SIMD8; - } - } - break; - } -#endif // FEATURE_HW_INTRINSICS - -#if defined(FEATURE_SIMD) - case GT_CNS_VEC: - { - GenTreeVecCon* vecCon = node->AsVecCon(); - - // TODO-1stClassStructs: do not retype SIMD nodes - - if ((vecCon->TypeIs(TYP_I_IMPL)) && (vecCon->GetSimdSize() == TARGET_POINTER_SIZE)) - { - assert(genTypeSize(vecCon->GetSimdBaseType()) == 4); - vecCon->gtType = TYP_SIMD8; - } - break; - } -#endif // FEATURE_SIMD - default: // Check that we don't have nodes not allowed in HIR here. assert((node->DebugOperKind() & DBK_NOTHIR) == 0); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 47a311ef2e1..67452c0fca2 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -7959,16 +7959,6 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree) tree->gtVNPair.SetBoth( vnStore->VNForHandle(ssize_t(tree->AsIntConCommon()->IconValue()), tree->GetIconHandleFlag())); } -#ifdef FEATURE_SIMD - else if (tree->IsCnsVec()) - { - // TODO-1stClassStructs: do not retype SIMD nodes - assert(varTypeIsLong(typ)); - - simd8_t simd8Val = tree->AsVecCon()->gtSimd8Val; - tree->gtVNPair.SetBoth(vnStore->VNForSimd8Con(simd8Val)); - } -#endif // FEATURE_SIMD else if ((typ == TYP_LONG) || (typ == TYP_ULONG)) { tree->gtVNPair.SetBoth(vnStore->VNForLongCon(INT64(tree->AsIntConCommon()->LngValue()))); @@ -8061,18 +8051,7 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree) case TYP_DOUBLE: { -#ifdef FEATURE_SIMD - if (tree->IsCnsVec()) - { - // TODO-1stClassStructs: do not retype SIMD nodes - simd8_t simd8Val = tree->AsVecCon()->gtSimd8Val; - tree->gtVNPair.SetBoth(vnStore->VNForSimd8Con(simd8Val)); - } - else -#endif // FEATURE_SIMD - { - tree->gtVNPair.SetBoth(vnStore->VNForDoubleCon(tree->AsDblCon()->gtDconVal)); - } + tree->gtVNPair.SetBoth(vnStore->VNForDoubleCon(tree->AsDblCon()->gtDconVal)); break; } diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_70124/Runtime_70124.cs b/src/tests/JIT/Regression/JitBlue/Runtime_70124/Runtime_70124.cs new file mode 100644 index 00000000000..aae6c864afd --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_70124/Runtime_70124.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Numerics; +using System.Runtime.CompilerServices; + +public class Runtime_70124 +{ + public static int Main() + { + return Problem(Vector2.One, Vector2.One) != new Vector2(3, 3) ? 101 : 100; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static Vector2 Problem(Vector2 a, Vector2 b) + { + CallForVtor2(a + b); + + return (a + b) + a; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void CallForVtor2(Vector2 vtor) { } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_70124/Runtime_70124.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_70124/Runtime_70124.csproj new file mode 100644 index 00000000000..f492aeac9d0 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_70124/Runtime_70124.csproj @@ -0,0 +1,9 @@ +<Project Sdk="Microsoft.NET.Sdk"> + <PropertyGroup> + <OutputType>Exe</OutputType> + <Optimize>True</Optimize> + </PropertyGroup> + <ItemGroup> + <Compile Include="$(MSBuildProjectName).cs" /> + </ItemGroup> +</Project>
\ No newline at end of file |