From 32b6e137d8e92cd4c3543ebc14eb7983c4079015 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 9 Jul 2024 11:03:07 -0700 Subject: [PATCH 1/9] Ensure that we create a valid GT_IND node --- src/coreclr/jit/lowerarmarch.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index ca37c3a951c59..224525fbd90dc 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -1264,9 +1264,10 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node) if (isContainableMemory || !op2->OperIsConst()) { - unsigned simdSize = node->GetSimdSize(); - var_types simdBaseType = node->GetSimdBaseType(); - var_types simdType = Compiler::getSIMDTypeForSize(simdSize); + unsigned simdSize = node->GetSimdSize(); + CorInfoType simdBaseJitType = node->GetSimdBaseJitType(); + var_types simdBaseType = node->GetSimdBaseType(); + var_types simdType = Compiler::getSIMDTypeForSize(simdSize); // We're either already loading from memory or we need to since // we don't know what actual index is going to be retrieved. @@ -1355,7 +1356,7 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node) } // Finally we can indirect the memory address to get the actual value - GenTreeIndir* indir = comp->gtNewIndir(simdBaseType, addr); + GenTreeIndir* indir = comp->gtNewIndir(JITtype2varType(simdBaseJitType), addr); BlockRange().InsertBefore(node, indir); LIR::Use use; From 61d8289e41de2724d60eeac8aa0e3232f80c7637 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 9 Jul 2024 11:19:22 -0700 Subject: [PATCH 2/9] Remove a bad assert in the associative morphs --- src/coreclr/jit/morph.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 0205f020825ff..04a4920caa759 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -10463,7 +10463,6 @@ GenTree* Compiler::fgOptimizeHWIntrinsicAssociative(GenTreeHWIntrinsic* tree) { return nullptr; } - assert(intrinOp1->GetHWIntrinsicId() == intrinsicId); if (needsMatchingBaseType && (intrinOp1->GetSimdBaseType() != simdBaseType)) { From 91e6f8fdee9a34bcedbfb5dd1d7ba356672f138c Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 9 Jul 2024 11:33:58 -0700 Subject: [PATCH 3/9] Ensure that we check for GT_NULLCHECK when handling containment --- src/coreclr/jit/lowerarmarch.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index 224525fbd90dc..1ebfc98d17318 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -2340,7 +2340,8 @@ void Lowering::ContainCheckIndir(GenTreeIndir* indirNode) MakeSrcContained(indirNode, addr); } } - else if (addr->OperIs(GT_LCL_ADDR) && IsContainableLclAddr(addr->AsLclFld(), indirNode->Size())) + else if (addr->OperIs(GT_LCL_ADDR) && !indirNode->OperIs(GT_NULLCHECK) && + IsContainableLclAddr(addr->AsLclFld(), indirNode->Size())) { // These nodes go into an addr mode: // - GT_LCL_ADDR is a stack addr mode. From 7976e1efdec84a77d6d5f45a44ee7c4f1e0c524f Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 9 Jul 2024 11:39:23 -0700 Subject: [PATCH 4/9] Fix a bad assert for x64 --- src/coreclr/jit/gentree.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 4093f3b796bdf..d2a78b592a5fc 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -30300,7 +30300,7 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree) case NI_Vector256_ToVector512: { - assert(retType == TYP_SIMD32); + assert(retType == TYP_SIMD64); assert(cnsNode->gtType == TYP_SIMD32); cnsNode->AsVecCon()->gtSimd64Val.v256[1] = {}; From 6217f8a35dc663b8b37635f429b5b9a971e10a47 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 9 Jul 2024 11:45:46 -0700 Subject: [PATCH 5/9] Ensure we properly check for ConvertMaskToVector --- src/coreclr/jit/gentree.h | 44 +++++++++++++++++----------------- src/coreclr/jit/lowerxarch.cpp | 2 +- src/coreclr/jit/morph.cpp | 7 +----- 3 files changed, 24 insertions(+), 29 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index ec043090da346..1ced7fee1c7fb 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1640,6 +1640,28 @@ struct GenTree bool OperIsHWIntrinsic(NamedIntrinsic intrinsicId) const; + bool OperIsConvertMaskToVector() const + { +#if defined(TARGET_XARCH) + return OperIsHWIntrinsic(NI_EVEX_ConvertMaskToVector); +#elif defined(TARGET_ARM64) + return OperIsHWIntrinsic(NI_Sve_ConvertMaskToVector); +#else + return false; +#endif + } + + bool OperIsConvertVectorToMask() const + { +#if defined(TARGET_XARCH) + return OperIsHWIntrinsic(NI_EVEX_ConvertVectorToMask); +#elif defined(TARGET_ARM64) + return OperIsHWIntrinsic(NI_Sve_ConvertVectorToMask); +#else + return false; +#endif + } + // This is here for cleaner GT_LONG #ifdefs. static bool OperIsLong(genTreeOps gtOper) { @@ -6507,28 +6529,6 @@ struct GenTreeHWIntrinsic : public GenTreeJitIntrinsic bool OperIsBitwiseHWIntrinsic() const; bool OperIsEmbRoundingEnabled() const; - bool OperIsConvertMaskToVector() const - { -#if defined(TARGET_XARCH) - return GetHWIntrinsicId() == NI_EVEX_ConvertMaskToVector; -#elif defined(TARGET_ARM64) - return GetHWIntrinsicId() == NI_Sve_ConvertMaskToVector; -#else - return false; -#endif // TARGET_ARM64 && FEATURE_MASKED_HW_INTRINSICS - } - - bool OperIsConvertVectorToMask() const - { -#if defined(TARGET_XARCH) - return GetHWIntrinsicId() == NI_EVEX_ConvertVectorToMask; -#elif defined(TARGET_ARM64) - return GetHWIntrinsicId() == NI_Sve_ConvertVectorToMask; -#else - return false; -#endif - } - bool OperRequiresAsgFlag() const; bool OperRequiresCallFlag() const; bool OperRequiresGlobRefFlag() const; diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index 97800d5d10ddb..73fbcb7af66f0 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -3063,7 +3063,7 @@ GenTree* Lowering::LowerHWIntrinsicCndSel(GenTreeHWIntrinsic* node) // Next, determine if the target architecture supports BlendVariable NamedIntrinsic blendVariableId = NI_Illegal; - bool isOp1CvtMaskToVector = op1->AsHWIntrinsic()->OperIsConvertMaskToVector(); + bool isOp1CvtMaskToVector = op1->OperIsConvertMaskToVector(); if ((simdSize == 64) || isOp1CvtMaskToVector) { diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 04a4920caa759..b6c50e5fc307b 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9992,7 +9992,7 @@ GenTree* Compiler::fgOptimizeHWIntrinsic(GenTreeHWIntrinsic* node) // We need both operands to be ConvertMaskToVector in // order to optimize this to a direct mask operation - if (!op1->OperIsHWIntrinsic()) + if (!op1->OperIsConvertMaskToVector()) { break; } @@ -10018,11 +10018,6 @@ GenTree* Compiler::fgOptimizeHWIntrinsic(GenTreeHWIntrinsic* node) GenTreeHWIntrinsic* cvtOp1 = op1->AsHWIntrinsic(); GenTreeHWIntrinsic* cvtOp2 = op2->AsHWIntrinsic(); - if (!cvtOp1->OperIsConvertMaskToVector()) - { - break; - } - if (!cvtOp2->OperIsConvertMaskToVector()) { break; From af07a4a68909e4e6fbf3cfd962e7b136096fe2d4 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 9 Jul 2024 11:56:23 -0700 Subject: [PATCH 6/9] Fix the memory size used for some vbroadcast instructions in disasm and asserts --- src/coreclr/jit/emit.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h index 88152601a7564..83bcd1e5dfe69 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -4212,6 +4212,10 @@ emitAttr emitter::emitGetBaseMemOpSize(instrDesc* id) const return EA_16BYTE; } + case INS_vbroadcastf32x8: + case INS_vbroadcasti32x8: + case INS_vbroadcasti64x4: + case INS_vbroadcastf64x4: case INS_vextractf32x8: case INS_vextracti32x8: case INS_vextractf64x4: From ff45c4e03b0f386f384d3f5a6efad0bea76eaeb1 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 9 Jul 2024 12:04:24 -0700 Subject: [PATCH 7/9] Ensure rewriting WithElement takes into account unsupported ISAs --- src/coreclr/jit/rationalize.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index 0d2e03d424879..aa4bd68dcd0dd 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -372,6 +372,26 @@ void Rationalizer::RewriteHWIntrinsicAsUserCall(GenTree** use, ArrayStackcompOpportunisticallyDependsOn(InstructionSet_SSE41_X64)) + { + break; + } + } + else if (!varTypeIsShort(simdBaseType)) + { + if (!comp->compOpportunisticallyDependsOn(InstructionSet_SSE41)) + { + break; + } + } + } +#endif // TARGET_XARCH + result = comp->gtNewSimdWithElementNode(retType, op1, op2, op3, simdBaseJitType, simdSize); break; } From 33ac032c02305e18bd3ce0f16ec06f426f684776 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 9 Jul 2024 12:46:53 -0700 Subject: [PATCH 8/9] Ensure we check FEATURE_HW_INTRINSICS --- src/coreclr/jit/gentree.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 1ced7fee1c7fb..d3fcb784a0b9d 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1642,24 +1642,29 @@ struct GenTree bool OperIsConvertMaskToVector() const { +#if defined(FEATURE_HW_INTRINSICS) #if defined(TARGET_XARCH) return OperIsHWIntrinsic(NI_EVEX_ConvertMaskToVector); #elif defined(TARGET_ARM64) return OperIsHWIntrinsic(NI_Sve_ConvertMaskToVector); +#endif // !TARGET_XARCH && !TARGET_ARM64 #else return false; -#endif +#endif // FEATURE_HW_INTRINSICS + } bool OperIsConvertVectorToMask() const { +#if defined(FEATURE_HW_INTRINSICS) #if defined(TARGET_XARCH) return OperIsHWIntrinsic(NI_EVEX_ConvertVectorToMask); #elif defined(TARGET_ARM64) return OperIsHWIntrinsic(NI_Sve_ConvertVectorToMask); +#endif // !TARGET_XARCH && !TARGET_ARM64 #else return false; -#endif +#endif // FEATURE_HW_INTRINSICS } // This is here for cleaner GT_LONG #ifdefs. From 04eb6cab7050be32a0f9254c674004524aa9552c Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Tue, 9 Jul 2024 13:10:53 -0700 Subject: [PATCH 9/9] Apply formatting patch --- src/coreclr/jit/gentree.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index d3fcb784a0b9d..c8cb232d8e40a 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -1651,7 +1651,6 @@ struct GenTree #else return false; #endif // FEATURE_HW_INTRINSICS - } bool OperIsConvertVectorToMask() const