From fca9b85df7dc0c09292e2b56b8599daccd19abce Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 11 Mar 2021 12:00:46 -0800 Subject: [PATCH 1/9] Ensure Vector.op_Multiply is handled as an intrinsic in appropriate cases --- src/coreclr/jit/simdashwintrinsic.cpp | 204 +++++++++++++++--- src/coreclr/jit/simdashwintrinsiclistarm64.h | 2 +- src/coreclr/jit/simdashwintrinsiclistxarch.h | 4 +- .../src/System/Numerics/Vector_1.cs | 5 +- 4 files changed, 178 insertions(+), 37 deletions(-) diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp index d50def6863f45d..4f21e72e57e667 100644 --- a/src/coreclr/jit/simdashwintrinsic.cpp +++ b/src/coreclr/jit/simdashwintrinsic.cpp @@ -880,56 +880,149 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, case NI_VectorT128_op_Multiply: { - assert(baseType == TYP_INT); - NamedIntrinsic hwIntrinsic = NI_Illegal; + GenTree** broadcastOp = nullptr; - if (compOpportunisticallyDependsOn(InstructionSet_SSE41)) + if (varTypeIsFloating(op1->TypeGet())) { - hwIntrinsic = NI_SSE41_MultiplyLow; + broadcastOp = &op1; } - else + else if (varTypeIsFloating(op2->TypeGet())) { - // op1Dup = op1 - GenTree* op1Dup; - op1 = impCloneExpr(op1, &op1Dup, clsHnd, (unsigned)CHECK_SPILL_ALL, - nullptr DEBUGARG("Clone op1 for Vector.Multiply")); + broadcastOp = &op2; + } - // op2Dup = op2 - GenTree* op2Dup; - op2 = impCloneExpr(op2, &op2Dup, clsHnd, (unsigned)CHECK_SPILL_ALL, - nullptr DEBUGARG("Clone op2 for Vector.Multiply")); + if (broadcastOp != nullptr) + { + *broadcastOp = gtNewSimdCreateBroadcastNode(simdType, *broadcastOp, baseType, simdSize, /* isSimdAsHWIntrinsic */ true); + } - // op1 = Sse2.ShiftRightLogical128BitLane(op1, 4) - op1 = gtNewSimdAsHWIntrinsicNode(retType, op1, gtNewIconNode(4, TYP_INT), - NI_SSE2_ShiftRightLogical128BitLane, baseType, simdSize); + switch (baseType) + { + case TYP_SHORT: + case TYP_USHORT: + { + hwIntrinsic = NI_SSE2_MultiplyLow; + break; + } - // op2 = Sse2.ShiftRightLogical128BitLane(op1, 4) - op2 = gtNewSimdAsHWIntrinsicNode(retType, op2, gtNewIconNode(4, TYP_INT), - NI_SSE2_ShiftRightLogical128BitLane, baseType, simdSize); + case TYP_INT: + case TYP_UINT: + { + if (compOpportunisticallyDependsOn(InstructionSet_SSE41)) + { + hwIntrinsic = NI_SSE41_MultiplyLow; + } + else + { + // op1Dup = op1 + GenTree* op1Dup; + op1 = impCloneExpr(op1, &op1Dup, clsHnd, (unsigned)CHECK_SPILL_ALL, + nullptr DEBUGARG("Clone op1 for Vector.Multiply")); + + // op2Dup = op2 + GenTree* op2Dup; + op2 = impCloneExpr(op2, &op2Dup, clsHnd, (unsigned)CHECK_SPILL_ALL, + nullptr DEBUGARG("Clone op2 for Vector.Multiply")); + + // op1 = Sse2.ShiftRightLogical128BitLane(op1, 4) + op1 = gtNewSimdAsHWIntrinsicNode(retType, op1, gtNewIconNode(4, TYP_INT), + NI_SSE2_ShiftRightLogical128BitLane, baseType, simdSize); + + // op2 = Sse2.ShiftRightLogical128BitLane(op1, 4) + op2 = gtNewSimdAsHWIntrinsicNode(retType, op2, gtNewIconNode(4, TYP_INT), + NI_SSE2_ShiftRightLogical128BitLane, baseType, simdSize); + + // op2 = Sse2.Multiply(op2.AsUInt64(), op1.AsUInt64()).AsInt32() + op2 = gtNewSimdAsHWIntrinsicNode(retType, op2, op1, NI_SSE2_Multiply, TYP_ULONG, simdSize); - // op2 = Sse2.Multiply(op2.AsUInt64(), op1.AsUInt64()).AsInt32() - op2 = gtNewSimdAsHWIntrinsicNode(retType, op2, op1, NI_SSE2_Multiply, TYP_ULONG, simdSize); + // op2 = Sse2.Shuffle(op2, (0, 0, 2, 0)) + op2 = gtNewSimdAsHWIntrinsicNode(retType, op2, gtNewIconNode(SHUFFLE_XXZX, TYP_INT), + NI_SSE2_Shuffle, baseType, simdSize); - // op2 = Sse2.Shuffle(op2, (0, 0, 2, 0)) - op2 = gtNewSimdAsHWIntrinsicNode(retType, op2, gtNewIconNode(SHUFFLE_XXZX, TYP_INT), - NI_SSE2_Shuffle, baseType, simdSize); + // op1 = Sse2.Multiply(op1Dup.AsUInt64(), op2Dup.AsUInt64()).AsInt32() + op1 = + gtNewSimdAsHWIntrinsicNode(retType, op1Dup, op2Dup, NI_SSE2_Multiply, TYP_ULONG, simdSize); - // op1 = Sse2.Multiply(op1Dup.AsUInt64(), op2Dup.AsUInt64()).AsInt32() - op1 = - gtNewSimdAsHWIntrinsicNode(retType, op1Dup, op2Dup, NI_SSE2_Multiply, TYP_ULONG, simdSize); + // op1 = Sse2.Shuffle(op1, (0, 0, 2, 0)) + op1 = gtNewSimdAsHWIntrinsicNode(retType, op1, gtNewIconNode(SHUFFLE_XXZX, TYP_INT), + NI_SSE2_Shuffle, baseType, simdSize); + + // result = Sse2.UnpackLow(op1, op2) + hwIntrinsic = NI_SSE2_UnpackLow; + } + break; + } + + case TYP_FLOAT: + { + hwIntrinsic = NI_SSE_Multiply; + break; + } - // op1 = Sse2.Shuffle(op1, (0, 0, 2, 0)) - op1 = gtNewSimdAsHWIntrinsicNode(retType, op1, gtNewIconNode(SHUFFLE_XXZX, TYP_INT), - NI_SSE2_Shuffle, baseType, simdSize); + case TYP_DOUBLE: + { + hwIntrinsic = NI_SSE2_Multiply; + break; + } - // result = Sse2.UnpackLow(op1, op2) - hwIntrinsic = NI_SSE2_UnpackLow; + default: + { + unreached(); + } } + assert(hwIntrinsic != NI_Illegal); + return gtNewSimdAsHWIntrinsicNode(retType, op1, op2, hwIntrinsic, baseType, simdSize); + } + case NI_VectorT256_op_Multiply: + { + NamedIntrinsic hwIntrinsic = NI_Illegal; + GenTree** broadcastOp = nullptr; + + if (varTypeIsFloating(op1->TypeGet())) + { + broadcastOp = &op1; + } + else if (varTypeIsFloating(op2->TypeGet())) + { + broadcastOp = &op2; + } + + if (broadcastOp != nullptr) + { + *broadcastOp = gtNewSimdCreateBroadcastNode(simdType, *broadcastOp, baseType, simdSize, /* isSimdAsHWIntrinsic */ true); + } + + switch (baseType) + { + case TYP_SHORT: + case TYP_USHORT: + case TYP_INT: + case TYP_UINT: + { + hwIntrinsic = NI_AVX2_MultiplyLow; + break; + } + + case TYP_FLOAT: + case TYP_DOUBLE: + { + hwIntrinsic = NI_AVX_Multiply; + break; + } + + default: + { + unreached(); + } + } + + assert(hwIntrinsic != NI_Illegal); return gtNewSimdAsHWIntrinsicNode(retType, op1, op2, hwIntrinsic, baseType, simdSize); } + #elif defined(TARGET_ARM64) case NI_Vector2_CreateBroadcast: case NI_Vector3_CreateBroadcast: @@ -970,6 +1063,53 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, // result = ConditionalSelect(op1, op1Dup, op2Dup) return impSimdAsHWIntrinsicCndSel(clsHnd, retType, baseType, simdSize, op1, op1Dup, op2Dup); } + + case NI_VectorT128_op_Multiply: + { + NamedIntrinsic hwIntrinsic = NI_Illegal; + GenTree** broadcastOp = nullptr; + + if (varTypeIsFloating(op1->TypeGet())) + { + broadcastOp = &op1; + } + else if (varTypeIsFloating(op2->TypeGet())) + { + broadcastOp = &op2; + } + + if (broadcastOp != nullptr) + { + *broadcastOp = gtNewSimdCreateBroadcastNode(simdType, *broadcastOp, baseType, simdSize, /* isSimdAsHWIntrinsic */ true); + } + + switch (baseType) + { + case TYP_SHORT: + case TYP_USHORT: + case TYP_INT: + case TYP_UINT: + case TYP_FLOAT: + { + hwIntrinsic = NI_AdvSimd_Multiply; + break; + } + + case TYP_DOUBLE: + { + hwIntrinsic = NI_AdvSimd_Arm64_Multiply; + break; + } + + default: + { + unreached(); + } + } + + assert(hwIntrinsic != NI_Illegal); + return gtNewSimdAsHWIntrinsicNode(retType, op1, op2, hwIntrinsic, baseType, simdSize); + } #else #error Unsupported platform #endif // !TARGET_XARCH && !TARGET_ARM64 diff --git a/src/coreclr/jit/simdashwintrinsiclistarm64.h b/src/coreclr/jit/simdashwintrinsiclistarm64.h index 494377bc7172fe..fc75eca9f3fc30 100644 --- a/src/coreclr/jit/simdashwintrinsiclistarm64.h +++ b/src/coreclr/jit/simdashwintrinsiclistarm64.h @@ -128,7 +128,7 @@ SIMD_AS_HWINTRINSIC_ID(VectorT128, op_Equality, SIMD_AS_HWINTRINSIC_ID(VectorT128, op_ExclusiveOr, 2, {NI_AdvSimd_Xor, NI_AdvSimd_Xor, NI_AdvSimd_Xor, NI_AdvSimd_Xor, NI_AdvSimd_Xor, NI_AdvSimd_Xor, NI_AdvSimd_Xor, NI_AdvSimd_Xor, NI_AdvSimd_Xor, NI_AdvSimd_Xor}, SimdAsHWIntrinsicFlag::None) SIMD_AS_HWINTRINSIC_ID(VectorT128, op_Explicit, 1, {NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit}, SimdAsHWIntrinsicFlag::None) SIMD_AS_HWINTRINSIC_ID(VectorT128, op_Inequality, 2, {NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality}, SimdAsHWIntrinsicFlag::None) -SIMD_AS_HWINTRINSIC_ID(VectorT128, op_Multiply, 2, {NI_AdvSimd_Multiply, NI_AdvSimd_Multiply, NI_AdvSimd_Multiply, NI_AdvSimd_Multiply, NI_AdvSimd_Multiply, NI_AdvSimd_Multiply, NI_Illegal, NI_Illegal, NI_AdvSimd_Multiply, NI_AdvSimd_Arm64_Multiply}, SimdAsHWIntrinsicFlag::None) +SIMD_AS_HWINTRINSIC_ID(VectorT128, op_Multiply, 2, {NI_VectorT128_op_Multiply, NI_VectorT128_op_Multiply, NI_VectorT128_op_Multiply, NI_VectorT128_op_Multiply, NI_VectorT128_op_Multiply, NI_VectorT128_op_Multiply, NI_Illegal, NI_Illegal, NI_VectorT128_op_Multiply, NI_VectorT128_op_Multiply}, SimdAsHWIntrinsicFlag::None) SIMD_AS_HWINTRINSIC_ID(VectorT128, op_Subtraction, 2, {NI_AdvSimd_Subtract, NI_AdvSimd_Subtract, NI_AdvSimd_Subtract, NI_AdvSimd_Subtract, NI_AdvSimd_Subtract, NI_AdvSimd_Subtract, NI_AdvSimd_Subtract, NI_AdvSimd_Subtract, NI_AdvSimd_Subtract, NI_AdvSimd_Arm64_Subtract}, SimdAsHWIntrinsicFlag::None) SIMD_AS_HWINTRINSIC_ID(VectorT128, SquareRoot, 1, {NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_AdvSimd_Arm64_Sqrt, NI_AdvSimd_Arm64_Sqrt}, SimdAsHWIntrinsicFlag::None) diff --git a/src/coreclr/jit/simdashwintrinsiclistxarch.h b/src/coreclr/jit/simdashwintrinsiclistxarch.h index 65584c3abb09e0..99e5c29ff8a9c4 100644 --- a/src/coreclr/jit/simdashwintrinsiclistxarch.h +++ b/src/coreclr/jit/simdashwintrinsiclistxarch.h @@ -128,7 +128,7 @@ SIMD_AS_HWINTRINSIC_ID(VectorT128, op_Equality, SIMD_AS_HWINTRINSIC_ID(VectorT128, op_ExclusiveOr, 2, {NI_SSE2_Xor, NI_SSE2_Xor, NI_SSE2_Xor, NI_SSE2_Xor, NI_SSE2_Xor, NI_SSE2_Xor, NI_SSE2_Xor, NI_SSE2_Xor, NI_SSE_Xor, NI_SSE2_Xor}, SimdAsHWIntrinsicFlag::None) SIMD_AS_HWINTRINSIC_ID(VectorT128, op_Explicit, 1, {NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit, NI_VectorT128_op_Explicit}, SimdAsHWIntrinsicFlag::None) SIMD_AS_HWINTRINSIC_ID(VectorT128, op_Inequality, 2, {NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality, NI_Vector128_op_Inequality}, SimdAsHWIntrinsicFlag::None) -SIMD_AS_HWINTRINSIC_ID(VectorT128, op_Multiply, 2, {NI_Illegal, NI_Illegal, NI_SSE2_MultiplyLow, NI_Illegal, NI_VectorT128_op_Multiply, NI_Illegal, NI_Illegal, NI_Illegal, NI_SSE_Multiply, NI_SSE2_Multiply}, SimdAsHWIntrinsicFlag::None) +SIMD_AS_HWINTRINSIC_ID(VectorT128, op_Multiply, 2, {NI_Illegal, NI_Illegal, NI_VectorT128_op_Multiply, NI_VectorT128_op_Multiply, NI_VectorT128_op_Multiply, NI_VectorT128_op_Multiply, NI_Illegal, NI_Illegal, NI_VectorT128_op_Multiply, NI_VectorT128_op_Multiply}, SimdAsHWIntrinsicFlag::None) SIMD_AS_HWINTRINSIC_ID(VectorT128, op_Subtraction, 2, {NI_SSE2_Subtract, NI_SSE2_Subtract, NI_SSE2_Subtract, NI_SSE2_Subtract, NI_SSE2_Subtract, NI_SSE2_Subtract, NI_SSE2_Subtract, NI_SSE2_Subtract, NI_SSE_Subtract, NI_SSE2_Subtract}, SimdAsHWIntrinsicFlag::None) SIMD_AS_HWINTRINSIC_ID(VectorT128, SquareRoot, 1, {NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_SSE_Sqrt, NI_SSE2_Sqrt}, SimdAsHWIntrinsicFlag::None) @@ -165,7 +165,7 @@ SIMD_AS_HWINTRINSIC_ID(VectorT256, op_Equality, SIMD_AS_HWINTRINSIC_ID(VectorT256, op_ExclusiveOr, 2, {NI_AVX2_Xor, NI_AVX2_Xor, NI_AVX2_Xor, NI_AVX2_Xor, NI_AVX2_Xor, NI_AVX2_Xor, NI_AVX2_Xor, NI_AVX2_Xor, NI_AVX_Xor, NI_AVX_Xor}, SimdAsHWIntrinsicFlag::None) SIMD_AS_HWINTRINSIC_ID(VectorT256, op_Explicit, 1, {NI_VectorT256_op_Explicit, NI_VectorT256_op_Explicit, NI_VectorT256_op_Explicit, NI_VectorT256_op_Explicit, NI_VectorT256_op_Explicit, NI_VectorT256_op_Explicit, NI_VectorT256_op_Explicit, NI_VectorT256_op_Explicit, NI_VectorT256_op_Explicit, NI_VectorT256_op_Explicit}, SimdAsHWIntrinsicFlag::None) SIMD_AS_HWINTRINSIC_ID(VectorT256, op_Inequality, 2, {NI_Vector256_op_Inequality, NI_Vector256_op_Inequality, NI_Vector256_op_Inequality, NI_Vector256_op_Inequality, NI_Vector256_op_Inequality, NI_Vector256_op_Inequality, NI_Vector256_op_Inequality, NI_Vector256_op_Inequality, NI_Vector256_op_Inequality, NI_Vector256_op_Inequality}, SimdAsHWIntrinsicFlag::None) -SIMD_AS_HWINTRINSIC_ID(VectorT256, op_Multiply, 2, {NI_Illegal, NI_Illegal, NI_AVX2_MultiplyLow, NI_Illegal, NI_AVX2_MultiplyLow, NI_Illegal, NI_Illegal, NI_Illegal, NI_AVX_Multiply, NI_AVX_Multiply}, SimdAsHWIntrinsicFlag::None) +SIMD_AS_HWINTRINSIC_ID(VectorT256, op_Multiply, 2, {NI_Illegal, NI_Illegal, NI_VectorT256_op_Multiply, NI_VectorT256_op_Multiply, NI_VectorT256_op_Multiply, NI_VectorT256_op_Multiply, NI_Illegal, NI_Illegal, NI_VectorT256_op_Multiply, NI_VectorT256_op_Multiply}, SimdAsHWIntrinsicFlag::None) SIMD_AS_HWINTRINSIC_ID(VectorT256, op_Subtraction, 2, {NI_AVX2_Subtract, NI_AVX2_Subtract, NI_AVX2_Subtract, NI_AVX2_Subtract, NI_AVX2_Subtract, NI_AVX2_Subtract, NI_AVX2_Subtract, NI_AVX2_Subtract, NI_AVX_Subtract, NI_AVX_Subtract}, SimdAsHWIntrinsicFlag::None) SIMD_AS_HWINTRINSIC_ID(VectorT256, SquareRoot, 1, {NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_Illegal, NI_AVX_Sqrt, NI_AVX_Sqrt}, SimdAsHWIntrinsicFlag::None) diff --git a/src/libraries/System.Private.CoreLib/src/System/Numerics/Vector_1.cs b/src/libraries/System.Private.CoreLib/src/System/Numerics/Vector_1.cs index 6d8c7f32765418..d9056586a49afe 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Numerics/Vector_1.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Numerics/Vector_1.cs @@ -409,7 +409,7 @@ public readonly bool TryCopyTo(Span destination) /// The source vector. /// The scalar value. /// The scaled vector. - [MethodImpl(MethodImplOptions.AggressiveInlining)] + [Intrinsic] public static Vector operator *(Vector value, T factor) { Vector result = default; @@ -422,11 +422,11 @@ public readonly bool TryCopyTo(Span destination) return result; } - /// Multiplies a vector by the given scalar. /// The scalar value. /// The source vector. /// The scaled vector. + [Intrinsic] [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Vector operator *(T factor, Vector value) => value * factor; @@ -450,6 +450,7 @@ public readonly bool TryCopyTo(Span destination) /// Negates a given vector. /// The source vector. /// The negated vector. + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Vector operator -(Vector value) => Zero - value; /// Returns a new vector by performing a bitwise-and operation on each of the elements in the given vectors. From 24373c5c4ef7e5477bf3e01c3dca25fcc1c519cf Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 11 Mar 2021 17:45:25 -0800 Subject: [PATCH 2/9] Applying formatting patch --- src/coreclr/jit/simdashwintrinsic.cpp | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp index 4f21e72e57e667..671e6b80ac6df5 100644 --- a/src/coreclr/jit/simdashwintrinsic.cpp +++ b/src/coreclr/jit/simdashwintrinsic.cpp @@ -894,7 +894,8 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, if (broadcastOp != nullptr) { - *broadcastOp = gtNewSimdCreateBroadcastNode(simdType, *broadcastOp, baseType, simdSize, /* isSimdAsHWIntrinsic */ true); + *broadcastOp = gtNewSimdCreateBroadcastNode(simdType, *broadcastOp, baseType, simdSize, + /* isSimdAsHWIntrinsic */ true); } switch (baseType) @@ -926,23 +927,26 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, nullptr DEBUGARG("Clone op2 for Vector.Multiply")); // op1 = Sse2.ShiftRightLogical128BitLane(op1, 4) - op1 = gtNewSimdAsHWIntrinsicNode(retType, op1, gtNewIconNode(4, TYP_INT), - NI_SSE2_ShiftRightLogical128BitLane, baseType, simdSize); + op1 = + gtNewSimdAsHWIntrinsicNode(retType, op1, gtNewIconNode(4, TYP_INT), + NI_SSE2_ShiftRightLogical128BitLane, baseType, simdSize); // op2 = Sse2.ShiftRightLogical128BitLane(op1, 4) - op2 = gtNewSimdAsHWIntrinsicNode(retType, op2, gtNewIconNode(4, TYP_INT), - NI_SSE2_ShiftRightLogical128BitLane, baseType, simdSize); + op2 = + gtNewSimdAsHWIntrinsicNode(retType, op2, gtNewIconNode(4, TYP_INT), + NI_SSE2_ShiftRightLogical128BitLane, baseType, simdSize); // op2 = Sse2.Multiply(op2.AsUInt64(), op1.AsUInt64()).AsInt32() - op2 = gtNewSimdAsHWIntrinsicNode(retType, op2, op1, NI_SSE2_Multiply, TYP_ULONG, simdSize); + op2 = gtNewSimdAsHWIntrinsicNode(retType, op2, op1, NI_SSE2_Multiply, TYP_ULONG, + simdSize); // op2 = Sse2.Shuffle(op2, (0, 0, 2, 0)) op2 = gtNewSimdAsHWIntrinsicNode(retType, op2, gtNewIconNode(SHUFFLE_XXZX, TYP_INT), NI_SSE2_Shuffle, baseType, simdSize); // op1 = Sse2.Multiply(op1Dup.AsUInt64(), op2Dup.AsUInt64()).AsInt32() - op1 = - gtNewSimdAsHWIntrinsicNode(retType, op1Dup, op2Dup, NI_SSE2_Multiply, TYP_ULONG, simdSize); + op1 = gtNewSimdAsHWIntrinsicNode(retType, op1Dup, op2Dup, NI_SSE2_Multiply, TYP_ULONG, + simdSize); // op1 = Sse2.Shuffle(op1, (0, 0, 2, 0)) op1 = gtNewSimdAsHWIntrinsicNode(retType, op1, gtNewIconNode(SHUFFLE_XXZX, TYP_INT), @@ -992,7 +996,8 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, if (broadcastOp != nullptr) { - *broadcastOp = gtNewSimdCreateBroadcastNode(simdType, *broadcastOp, baseType, simdSize, /* isSimdAsHWIntrinsic */ true); + *broadcastOp = gtNewSimdCreateBroadcastNode(simdType, *broadcastOp, baseType, simdSize, + /* isSimdAsHWIntrinsic */ true); } switch (baseType) @@ -1080,7 +1085,8 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, if (broadcastOp != nullptr) { - *broadcastOp = gtNewSimdCreateBroadcastNode(simdType, *broadcastOp, baseType, simdSize, /* isSimdAsHWIntrinsic */ true); + *broadcastOp = gtNewSimdCreateBroadcastNode(simdType, *broadcastOp, baseType, simdSize, + /* isSimdAsHWIntrinsic */ true); } switch (baseType) From 9d7c8c59284958e2bdf090de497ef17f8e550851 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 11 Mar 2021 17:42:45 -0800 Subject: [PATCH 3/9] Ensure TYP_BYTE and TYP_UBYTE are handled for Vector.op_Multiply on ARM64 --- src/coreclr/jit/simdashwintrinsic.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp index 671e6b80ac6df5..4f168915188e19 100644 --- a/src/coreclr/jit/simdashwintrinsic.cpp +++ b/src/coreclr/jit/simdashwintrinsic.cpp @@ -1091,6 +1091,8 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, switch (baseType) { + case TYP_BYTE: + case TYP_UBYTE: case TYP_SHORT: case TYP_USHORT: case TYP_INT: From 2b5a43eb5053157fbbea0b4e00bfb52cfa62ba96 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Fri, 12 Mar 2021 08:11:01 -0800 Subject: [PATCH 4/9] Ensure broadcast nodes are inserted for all `operator *(Vector, T)` --- src/coreclr/jit/simdashwintrinsic.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp index 4f168915188e19..bd62d522e9e09c 100644 --- a/src/coreclr/jit/simdashwintrinsic.cpp +++ b/src/coreclr/jit/simdashwintrinsic.cpp @@ -883,11 +883,11 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, NamedIntrinsic hwIntrinsic = NI_Illegal; GenTree** broadcastOp = nullptr; - if (varTypeIsFloating(op1->TypeGet())) + if (varTypeIsArithmetic(op1->TypeGet())) { broadcastOp = &op1; } - else if (varTypeIsFloating(op2->TypeGet())) + else if (varTypeIsArithmetic(op2->TypeGet())) { broadcastOp = &op2; } @@ -985,11 +985,11 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, NamedIntrinsic hwIntrinsic = NI_Illegal; GenTree** broadcastOp = nullptr; - if (varTypeIsFloating(op1->TypeGet())) + if (varTypeIsArithmetic(op1->TypeGet())) { broadcastOp = &op1; } - else if (varTypeIsFloating(op2->TypeGet())) + else if (varTypeIsArithmetic(op2->TypeGet())) { broadcastOp = &op2; } @@ -1074,11 +1074,11 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, NamedIntrinsic hwIntrinsic = NI_Illegal; GenTree** broadcastOp = nullptr; - if (varTypeIsFloating(op1->TypeGet())) + if (varTypeIsArithmetic(op1->TypeGet())) { broadcastOp = &op1; } - else if (varTypeIsFloating(op2->TypeGet())) + else if (varTypeIsArithmetic(op2->TypeGet())) { broadcastOp = &op2; } From b9c77d7b1f8400f0b4cd63c0d0d2c66a9a99d58a Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Fri, 12 Mar 2021 13:51:49 -0800 Subject: [PATCH 5/9] Ensure ARM64 uses MultiplyByScalar when its available --- src/coreclr/jit/simdashwintrinsic.cpp | 42 +++++++++++++++++++-------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp index bd62d522e9e09c..e049642a61b290 100644 --- a/src/coreclr/jit/simdashwintrinsic.cpp +++ b/src/coreclr/jit/simdashwintrinsic.cpp @@ -1071,41 +1071,59 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, case NI_VectorT128_op_Multiply: { - NamedIntrinsic hwIntrinsic = NI_Illegal; - GenTree** broadcastOp = nullptr; + NamedIntrinsic hwIntrinsic = NI_Illegal; + NamedIntrinsic scalarIntrinsic = NI_Illegal; + GenTree** scalarOp = nullptr; if (varTypeIsArithmetic(op1->TypeGet())) { - broadcastOp = &op1; + scalarOp = &op1; } else if (varTypeIsArithmetic(op2->TypeGet())) { - broadcastOp = &op2; - } - - if (broadcastOp != nullptr) - { - *broadcastOp = gtNewSimdCreateBroadcastNode(simdType, *broadcastOp, baseType, simdSize, - /* isSimdAsHWIntrinsic */ true); + scalarOp = &op2; } switch (baseType) { case TYP_BYTE: case TYP_UBYTE: + { + if (scalarOp != nullptr) + { + *scalarOp = gtNewSimdCreateBroadcastNode(simdType, *scalarOp, baseType, simdSize, + /* isSimdAsHWIntrinsic */ true); + } + + hwIntrinsic = NI_AdvSimd_Multiply; + break; + } + case TYP_SHORT: case TYP_USHORT: case TYP_INT: case TYP_UINT: case TYP_FLOAT: { - hwIntrinsic = NI_AdvSimd_Multiply; + if (scalarOp != nullptr) + { + *scalarOp = gtNewSimdAsHWIntrinsicNode(TYP_SIMD8, *scalarOp, + NI_Vector64_CreateScalarUnsafe, baseType, 8); + } + + hwIntrinsic = NI_AdvSimd_MultiplyByScalar; break; } case TYP_DOUBLE: { - hwIntrinsic = NI_AdvSimd_Arm64_Multiply; + if (scalarOp != nullptr) + { + *scalarOp = gtNewSimdAsHWIntrinsicNode(TYP_SIMD8, *scalarOp, NI_Vector64_Create, + baseType, 8); + } + + hwIntrinsic = NI_AdvSimd_Arm64_MultiplyByScalar; break; } From 1b1715db348056c8ceb838b1c41909d5e5af6739 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Fri, 12 Mar 2021 15:39:38 -0800 Subject: [PATCH 6/9] Applying formatting patch --- src/coreclr/jit/simdashwintrinsic.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp index e049642a61b290..6764efefbdb564 100644 --- a/src/coreclr/jit/simdashwintrinsic.cpp +++ b/src/coreclr/jit/simdashwintrinsic.cpp @@ -1092,7 +1092,7 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, if (scalarOp != nullptr) { *scalarOp = gtNewSimdCreateBroadcastNode(simdType, *scalarOp, baseType, simdSize, - /* isSimdAsHWIntrinsic */ true); + /* isSimdAsHWIntrinsic */ true); } hwIntrinsic = NI_AdvSimd_Multiply; @@ -1119,8 +1119,8 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, { if (scalarOp != nullptr) { - *scalarOp = gtNewSimdAsHWIntrinsicNode(TYP_SIMD8, *scalarOp, NI_Vector64_Create, - baseType, 8); + *scalarOp = + gtNewSimdAsHWIntrinsicNode(TYP_SIMD8, *scalarOp, NI_Vector64_Create, baseType, 8); } hwIntrinsic = NI_AdvSimd_Arm64_MultiplyByScalar; From a79a1cb43872d8cafcdc4f8dca084afff00ff6c7 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Mon, 15 Mar 2021 09:11:56 -0700 Subject: [PATCH 7/9] Ensure the scalar for op_Multiply is op2 on ARM64 --- src/coreclr/jit/simdashwintrinsic.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp index 6764efefbdb564..d969a96fe76889 100644 --- a/src/coreclr/jit/simdashwintrinsic.cpp +++ b/src/coreclr/jit/simdashwintrinsic.cpp @@ -1077,7 +1077,10 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, if (varTypeIsArithmetic(op1->TypeGet())) { - scalarOp = &op1; + // MultiplyByScalar requires the scalar op to be op2 + std::swap(op1, op2); + + scalarOp = &op2; } else if (varTypeIsArithmetic(op2->TypeGet())) { From 56c210d677d271e6d982d38a3f52edcd1a441f3c Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Mon, 15 Mar 2021 13:19:29 -0700 Subject: [PATCH 8/9] Ensure we do a full multiply for `Vector * Vector` on ARM64 --- src/coreclr/jit/simdashwintrinsic.cpp | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp index d969a96fe76889..b5273e86483722 100644 --- a/src/coreclr/jit/simdashwintrinsic.cpp +++ b/src/coreclr/jit/simdashwintrinsic.cpp @@ -1110,11 +1110,14 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, { if (scalarOp != nullptr) { - *scalarOp = gtNewSimdAsHWIntrinsicNode(TYP_SIMD8, *scalarOp, - NI_Vector64_CreateScalarUnsafe, baseType, 8); + hwIntrinsic = NI_AdvSimd_MultiplyByScalar; + *scalarOp = gtNewSimdAsHWIntrinsicNode(TYP_SIMD8, *scalarOp, + NI_Vector64_CreateScalarUnsafe, baseType, 8); + } + else + { + hwIntrinsic = NI_AdvSimd_Multiply; } - - hwIntrinsic = NI_AdvSimd_MultiplyByScalar; break; } @@ -1122,11 +1125,15 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, { if (scalarOp != nullptr) { - *scalarOp = + hwIntrinsic = NI_AdvSimd_Arm64_MultiplyByScalar; + *scalarOp = gtNewSimdAsHWIntrinsicNode(TYP_SIMD8, *scalarOp, NI_Vector64_Create, baseType, 8); - } - hwIntrinsic = NI_AdvSimd_Arm64_MultiplyByScalar; + } + else + { + hwIntrinsic = NI_AdvSimd_Arm64_Multiply; + } break; } From 2d72fd0ab32d8174f43703aa787386af841c93e3 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 17 Mar 2021 12:38:10 -0700 Subject: [PATCH 9/9] Applying formatting patch --- src/coreclr/jit/simdashwintrinsic.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp index b5273e86483722..693343c06dd7e9 100644 --- a/src/coreclr/jit/simdashwintrinsic.cpp +++ b/src/coreclr/jit/simdashwintrinsic.cpp @@ -1112,7 +1112,7 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, { hwIntrinsic = NI_AdvSimd_MultiplyByScalar; *scalarOp = gtNewSimdAsHWIntrinsicNode(TYP_SIMD8, *scalarOp, - NI_Vector64_CreateScalarUnsafe, baseType, 8); + NI_Vector64_CreateScalarUnsafe, baseType, 8); } else { @@ -1126,9 +1126,8 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic intrinsic, if (scalarOp != nullptr) { hwIntrinsic = NI_AdvSimd_Arm64_MultiplyByScalar; - *scalarOp = + *scalarOp = gtNewSimdAsHWIntrinsicNode(TYP_SIMD8, *scalarOp, NI_Vector64_Create, baseType, 8); - } else {