From d624161312857e3897e605a026b21bcbb4226807 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 21 Aug 2024 12:29:02 -0400 Subject: [PATCH 01/11] Always convert out-of-range constants to user call --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/hwintrinsic.cpp | 68 +++++++++++++++++++--------- src/coreclr/jit/hwintrinsicarm64.cpp | 14 +++--- src/coreclr/jit/hwintrinsicxarch.cpp | 2 +- 4 files changed, 56 insertions(+), 30 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index b748f3b81138a3..5cff02c3162fde 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4718,7 +4718,7 @@ class Compiler GenTree* getArgForHWIntrinsic(var_types argType, CORINFO_CLASS_HANDLE argClass); GenTree* impNonConstFallback(NamedIntrinsic intrinsic, var_types simdType, CorInfoType simdBaseJitType); GenTree* addRangeCheckIfNeeded( - NamedIntrinsic intrinsic, GenTree* immOp, bool mustExpand, int immLowerBound, int immUpperBound); + NamedIntrinsic intrinsic, GenTree* immOp, int immLowerBound, int immUpperBound); GenTree* addRangeCheckForHWIntrinsic(GenTree* immOp, int immLowerBound, int immUpperBound); void getHWIntrinsicImmOps(NamedIntrinsic intrinsic, diff --git a/src/coreclr/jit/hwintrinsic.cpp b/src/coreclr/jit/hwintrinsic.cpp index 9d3db42ec4c097..026024677569c9 100644 --- a/src/coreclr/jit/hwintrinsic.cpp +++ b/src/coreclr/jit/hwintrinsic.cpp @@ -1332,7 +1332,6 @@ GenTree* Compiler::getArgForHWIntrinsic(var_types argType, CORINFO_CLASS_HANDLE // Arguments: // intrinsic -- intrinsic ID // immOp -- the immediate operand of the intrinsic -// mustExpand -- true if the compiler is compiling the fallback(GT_CALL) of this intrinsics // immLowerBound -- lower incl. bound for a value of the immediate operand (for a non-full-range imm-intrinsic) // immUpperBound -- upper incl. bound for a value of the immediate operand (for a non-full-range imm-intrinsic) // @@ -1340,21 +1339,19 @@ GenTree* Compiler::getArgForHWIntrinsic(var_types argType, CORINFO_CLASS_HANDLE // add a GT_BOUNDS_CHECK node for non-full-range imm-intrinsic, which would throw ArgumentOutOfRangeException // when the imm-argument is not in the valid range // -GenTree* Compiler::addRangeCheckIfNeeded( - NamedIntrinsic intrinsic, GenTree* immOp, bool mustExpand, int immLowerBound, int immUpperBound) +GenTree* Compiler::addRangeCheckIfNeeded(NamedIntrinsic intrinsic, GenTree* immOp, int immLowerBound, int immUpperBound) { assert(immOp != nullptr); // Full-range imm-intrinsics do not need the range-check // because the imm-parameter of the intrinsic method is a byte. // AVX2 Gather intrinsics no not need the range-check // because their imm-parameter have discrete valid values that are handle by managed code - if (mustExpand && HWIntrinsicInfo::isImmOp(intrinsic, immOp) + if (!immOp->IsCnsIntOrI() && HWIntrinsicInfo::isImmOp(intrinsic, immOp) #ifdef TARGET_XARCH && !HWIntrinsicInfo::isAVX2GatherIntrinsic(intrinsic) && !HWIntrinsicInfo::HasFullRangeImm(intrinsic) #endif ) { - assert(!immOp->IsCnsIntOrI()); assert(varTypeIsIntegral(immOp)); return addRangeCheckForHWIntrinsic(immOp, immLowerBound, immUpperBound); @@ -1596,7 +1593,6 @@ bool Compiler::CheckHWIntrinsicImmRange(NamedIntrinsic intrinsic, if (immOutOfRange) { - assert(!mustExpand); // The imm-HWintrinsics that do not accept all imm8 values may throw // ArgumentOutOfRangeException when the imm argument is not in the valid range, // unless the intrinsic can be transformed into one that does accept all imm8 values @@ -1859,15 +1855,30 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic, { return impNonConstFallback(intrinsic, retType, simdBaseJitType); } - else if (!opts.OptimizationEnabled()) + else if (immOp2->IsCnsIntOrI()) { - // Only enable late stage rewriting if optimizations are enabled - // as we won't otherwise encounter a constant at the later point - return nullptr; + // If we know the immediate is out-of-range, + // convert the intrinsic into a user call (or throw if we must expand) + return impUnsupportedNamedIntrinsic(CORINFO_HELP_THROW_ARGUMENTOUTOFRANGEEXCEPTION, method, sig, + mustExpand); } else { - setMethodHandle = true; + // The immediate is unknown, and we aren't using a fallback intrinsic. + // In this case, CheckHWIntrinsicImmRange should not return false for intrinsics that must expand. + assert(!mustExpand); + + if (opts.OptimizationEnabled()) + { + // Only enable late stage rewriting if optimizations are enabled + // as we won't otherwise encounter a constant at the later point + setMethodHandle = true; + } + else + { + // Just convert to a user call + return nullptr; + } } } } @@ -1896,15 +1907,30 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic, { return impNonConstFallback(intrinsic, retType, simdBaseJitType); } - else if (!opts.OptimizationEnabled()) + else if (immOp1->IsCnsIntOrI()) { - // Only enable late stage rewriting if optimizations are enabled - // as we won't otherwise encounter a constant at the later point - return nullptr; + // If we know the immediate is out-of-range, + // convert the intrinsic into a user call (or throw if we must expand) + return impUnsupportedNamedIntrinsic(CORINFO_HELP_THROW_ARGUMENTOUTOFRANGEEXCEPTION, method, sig, + mustExpand); } else { - setMethodHandle = true; + // The immediate is unknown, and we aren't using a fallback intrinsic. + // In this case, CheckHWIntrinsicImmRange should not return false for intrinsics that must expand. + assert(!mustExpand); + + if (opts.OptimizationEnabled()) + { + // Only enable late stage rewriting if optimizations are enabled + // as we won't otherwise encounter a constant at the later point + setMethodHandle = true; + } + else + { + // Just convert to a user call + return nullptr; + } } } } @@ -1960,7 +1986,7 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic, { case 4: op4 = getArgForHWIntrinsic(sigReader.GetOp4Type(), sigReader.op4ClsHnd); - op4 = addRangeCheckIfNeeded(intrinsic, op4, mustExpand, immLowerBound, immUpperBound); + op4 = addRangeCheckIfNeeded(intrinsic, op4, immLowerBound, immUpperBound); op3 = getArgForHWIntrinsic(sigReader.GetOp3Type(), sigReader.op3ClsHnd); op2 = getArgForHWIntrinsic(sigReader.GetOp2Type(), sigReader.op2ClsHnd); op1 = getArgForHWIntrinsic(sigReader.GetOp1Type(), sigReader.op1ClsHnd); @@ -1974,7 +2000,7 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic, case 2: op2 = getArgForHWIntrinsic(sigReader.GetOp2Type(), sigReader.op2ClsHnd); - op2 = addRangeCheckIfNeeded(intrinsic, op2, mustExpand, immLowerBound, immUpperBound); + op2 = addRangeCheckIfNeeded(intrinsic, op2, immLowerBound, immUpperBound); op1 = getArgForHWIntrinsic(sigReader.GetOp1Type(), sigReader.op1ClsHnd); break; @@ -2144,7 +2170,7 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic, #ifdef TARGET_ARM64 if (intrinsic == NI_AdvSimd_LoadAndInsertScalar) { - op2 = addRangeCheckIfNeeded(intrinsic, op2, mustExpand, immLowerBound, immUpperBound); + op2 = addRangeCheckIfNeeded(intrinsic, op2, immLowerBound, immUpperBound); if (op1->OperIs(GT_CAST)) { @@ -2158,12 +2184,12 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic, } else if ((intrinsic == NI_AdvSimd_Insert) || (intrinsic == NI_AdvSimd_InsertScalar)) { - op2 = addRangeCheckIfNeeded(intrinsic, op2, mustExpand, immLowerBound, immUpperBound); + op2 = addRangeCheckIfNeeded(intrinsic, op2, immLowerBound, immUpperBound); } else #endif { - op3 = addRangeCheckIfNeeded(intrinsic, op3, mustExpand, immLowerBound, immUpperBound); + op3 = addRangeCheckIfNeeded(intrinsic, op3, immLowerBound, immUpperBound); } retNode = isScalar ? gtNewScalarHWIntrinsicNode(nodeRetType, op1, op2, op3, intrinsic) diff --git a/src/coreclr/jit/hwintrinsicarm64.cpp b/src/coreclr/jit/hwintrinsicarm64.cpp index b159e767a3d774..afe933da87c3fc 100644 --- a/src/coreclr/jit/hwintrinsicarm64.cpp +++ b/src/coreclr/jit/hwintrinsicarm64.cpp @@ -2466,7 +2466,7 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic, assert(HWIntrinsicInfo::isImmOp(intrinsic, op3)); HWIntrinsicInfo::lookupImmBounds(intrinsic, simdSize, simdBaseType, 1, &immLowerBound, &immUpperBound); - op3 = addRangeCheckIfNeeded(intrinsic, op3, (!op3->IsCnsIntOrI()), immLowerBound, immUpperBound); + op3 = addRangeCheckIfNeeded(intrinsic, op3, immLowerBound, immUpperBound); argType = JITtype2varType(strip(info.compCompHnd->getArgType(sig, arg1, &argClass))); op1 = getArgForHWIntrinsic(argType, argClass); @@ -2939,11 +2939,11 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic, assert(HWIntrinsicInfo::isImmOp(intrinsic, op2)); HWIntrinsicInfo::lookupImmBounds(intrinsic, simdSize, simdBaseType, 1, &immLowerBound, &immUpperBound); - op2 = addRangeCheckIfNeeded(intrinsic, op2, (!op2->IsCnsIntOrI()), immLowerBound, immUpperBound); + op2 = addRangeCheckIfNeeded(intrinsic, op2, immLowerBound, immUpperBound); assert(HWIntrinsicInfo::isImmOp(intrinsic, op3)); HWIntrinsicInfo::lookupImmBounds(intrinsic, simdSize, simdBaseType, 2, &immLowerBound, &immUpperBound); - op3 = addRangeCheckIfNeeded(intrinsic, op3, (!op3->IsCnsIntOrI()), immLowerBound, immUpperBound); + op3 = addRangeCheckIfNeeded(intrinsic, op3, immLowerBound, immUpperBound); retNode = isScalar ? gtNewScalarHWIntrinsicNode(retType, op1, op2, op3, intrinsic) : gtNewSimdHWIntrinsicNode(retType, op1, op2, op3, intrinsic, simdBaseJitType, simdSize); @@ -3010,7 +3010,7 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic, op3 = getArgForHWIntrinsic(argType, argClass); assert(HWIntrinsicInfo::isImmOp(intrinsic, op3)); - op3 = addRangeCheckIfNeeded(intrinsic, op3, mustExpand, immLowerBound, immUpperBound); + op3 = addRangeCheckIfNeeded(intrinsic, op3, immLowerBound, immUpperBound); argType = JITtype2varType(strip(info.compCompHnd->getArgType(sig, arg2, &argClass))); op2 = getArgForHWIntrinsic(argType, argClass); @@ -3040,7 +3040,7 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic, op4 = getArgForHWIntrinsic(argType, argClass); assert(HWIntrinsicInfo::isImmOp(intrinsic, op4)); - op4 = addRangeCheckIfNeeded(intrinsic, op4, mustExpand, immLowerBound, immUpperBound); + op4 = addRangeCheckIfNeeded(intrinsic, op4, immLowerBound, immUpperBound); argType = JITtype2varType(strip(info.compCompHnd->getArgType(sig, arg3, &argClass))); op3 = getArgForHWIntrinsic(argType, argClass); @@ -3108,12 +3108,12 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic, argType = JITtype2varType(strip(info.compCompHnd->getArgType(sig, arg5, &argClass))); GenTree* op5 = getArgForHWIntrinsic(argType, argClass); assert(HWIntrinsicInfo::isImmOp(intrinsic, op5)); - op5 = addRangeCheckIfNeeded(intrinsic, op5, mustExpand, imm1LowerBound, imm1UpperBound); + op5 = addRangeCheckIfNeeded(intrinsic, op5, imm1LowerBound, imm1UpperBound); argType = JITtype2varType(strip(info.compCompHnd->getArgType(sig, arg4, &argClass))); op4 = getArgForHWIntrinsic(argType, argClass); assert(HWIntrinsicInfo::isImmOp(intrinsic, op4)); - op4 = addRangeCheckIfNeeded(intrinsic, op4, mustExpand, imm2LowerBound, imm2UpperBound); + op4 = addRangeCheckIfNeeded(intrinsic, op4, imm2LowerBound, imm2UpperBound); argType = JITtype2varType(strip(info.compCompHnd->getArgType(sig, arg3, &argClass))); op3 = getArgForHWIntrinsic(argType, argClass); diff --git a/src/coreclr/jit/hwintrinsicxarch.cpp b/src/coreclr/jit/hwintrinsicxarch.cpp index 1a016f60fbe990..afc27d12fb7675 100644 --- a/src/coreclr/jit/hwintrinsicxarch.cpp +++ b/src/coreclr/jit/hwintrinsicxarch.cpp @@ -4800,7 +4800,7 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic, int immUpperBound = HWIntrinsicInfo::lookupImmUpperBound(intrinsic); op3 = impPopStack().val; - op3 = addRangeCheckIfNeeded(intrinsic, op3, mustExpand, immLowerBound, immUpperBound); + op3 = addRangeCheckIfNeeded(intrinsic, op3, immLowerBound, immUpperBound); op2 = impSIMDPopStack(); op1 = impSIMDPopStack(); From 2a1642c728abeeb0f28ee07be8609a6854094ebb Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 21 Aug 2024 12:58:28 -0400 Subject: [PATCH 02/11] Add tests --- .../JitBlue/Runtime_106546/Runtime_106546.cs | 66 +++++++++++++++++++ .../Runtime_106546/Runtime_106546.csproj | 8 +++ 2 files changed, 74 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.csproj diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.cs b/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.cs new file mode 100644 index 00000000000000..df5fb90cb7d40f --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.cs @@ -0,0 +1,66 @@ +// 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.Numerics; +using System.Runtime.Intrinsics; +using System.Runtime.Intrinsics.Arm; +using Xunit; + +public class Runtime_106546 +{ + [Fact] + public static void TestDotProductThrows() + { + if (Dp.IsSupported) + { + Assert.Throws(() => new Runtime_106546().DotProduct()); + } + } + + [Fact] + public static void TestShiftRightThrows() + { + if (AdvSimd.IsSupported) + { + Assert.Throws(() => new Runtime_106546().ShiftRight()); + } + } + + // Generated by Fuzzlyn v2.2 on 2024-08-17 15:13:27 + // Run on Arm64 MacOS + // Seed: 17566447992392941035-vectort,vector64,vector128,armadvsimd,armadvsimdarm64,armaes,armarmbase,armarmbasearm64,armcrc32,armcrc32arm64,armdp,armrdm,armrdmarm64,armsha1,armsha256 + // Reduced from 52.5 KiB to 0.7 KiB in 00:00:21 + // Debug: Throws 'System.ArgumentOutOfRangeException' + // Release: Runs successfully + private void DotProduct() + { + var vr4 = Vector64.Create(0); + var vr5 = Vector64.Create(0); + var vr6 = Vector64.Create(0); + var vr3 = Dp.DotProductBySelectedQuadruplet(vr4, vr5, vr6, 7); + Console.WriteLine(vr3); + } + + private struct S1 + { + public byte byte_1; + } + + // Found by Antigen + private void ShiftRight() + { + Vector64 floatVec = Vector64.Create(1f); + Vector64 doubleVec = Vector64.CreateScalar((double)82); + Vector64 intVec = Vector64.Create(1, 1); + Vector128 longVec = Vector128.AllBitsSet; + S1 s1 = new S1(); + + for (int i = 0; i < 3; i++) + { + Vector64.Shuffle(floatVec += Vector64.AsSingle(doubleVec), AdvSimd.ShiftRightLogicalNarrowingLower(longVec, s1.byte_1) & (intVec - intVec)* 15*4); + } + + Console.WriteLine(floatVec); + } +} \ No newline at end of file diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.csproj new file mode 100644 index 00000000000000..15edd99711a1a4 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.csproj @@ -0,0 +1,8 @@ + + + True + + + + + \ No newline at end of file From 9b99239ff3e4bc3732f93bbd6d5def4f876795e4 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 21 Aug 2024 17:40:18 -0400 Subject: [PATCH 03/11] Tweak StoreSelectedScalar definition --- src/coreclr/jit/hwintrinsiclistarm64.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/hwintrinsiclistarm64.h b/src/coreclr/jit/hwintrinsiclistarm64.h index c3649cb64beb55..a7503787e66ee7 100644 --- a/src/coreclr/jit/hwintrinsiclistarm64.h +++ b/src/coreclr/jit/hwintrinsiclistarm64.h @@ -487,7 +487,7 @@ HARDWARE_INTRINSIC(AdvSimd, SignExtendWideningLower, HARDWARE_INTRINSIC(AdvSimd, SignExtendWideningUpper, 16, 1, {INS_sxtl2, INS_invalid, INS_sxtl2, INS_invalid, INS_sxtl2, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_BaseTypeFromFirstArg) HARDWARE_INTRINSIC(AdvSimd, SqrtScalar, 8, 1, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_fsqrt, INS_fsqrt}, HW_Category_SIMD, HW_Flag_SIMDScalar) HARDWARE_INTRINSIC(AdvSimd, Store, 8, 2, {INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_invalid, INS_invalid, INS_st1_2regs, INS_invalid}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen|HW_Flag_NeedsConsecutiveRegisters) -HARDWARE_INTRINSIC(AdvSimd, StoreSelectedScalar, 8, 3, {INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_HasImmediateOperand|HW_Flag_SIMDScalar|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_NeedsConsecutiveRegisters) +HARDWARE_INTRINSIC(AdvSimd, StoreSelectedScalar, -1, 3, {INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromSecondArg|HW_Flag_HasImmediateOperand|HW_Flag_SIMDScalar|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_NeedsConsecutiveRegisters) HARDWARE_INTRINSIC(AdvSimd, StoreVectorAndZip, 8, 2, {INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen|HW_Flag_NeedsConsecutiveRegisters) HARDWARE_INTRINSIC(AdvSimd, Subtract, -1, 2, {INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_fsub, INS_invalid}, HW_Category_SIMD, HW_Flag_NoFlag) HARDWARE_INTRINSIC(AdvSimd, SubtractHighNarrowingLower, 8, 2, {INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_NoFlag) From 42641785bcaf776cf13e4c147291977aab72dfff Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 21 Aug 2024 17:42:21 -0400 Subject: [PATCH 04/11] Remove unnecessary arg --- src/coreclr/jit/compiler.h | 1 - src/coreclr/jit/hwintrinsic.cpp | 10 ++++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 5cff02c3162fde..3343ab23273298 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3446,7 +3446,6 @@ class Compiler GenTreeHWIntrinsic* gtNewScalarHWIntrinsicNode( var_types type, GenTree* op1, GenTree* op2, GenTree* op3, NamedIntrinsic hwIntrinsicID); CorInfoType getBaseJitTypeFromArgIfNeeded(NamedIntrinsic intrinsic, - CORINFO_CLASS_HANDLE clsHnd, CORINFO_SIG_INFO* sig, CorInfoType simdBaseJitType); diff --git a/src/coreclr/jit/hwintrinsic.cpp b/src/coreclr/jit/hwintrinsic.cpp index 026024677569c9..759a6d5c60ba23 100644 --- a/src/coreclr/jit/hwintrinsic.cpp +++ b/src/coreclr/jit/hwintrinsic.cpp @@ -718,7 +718,6 @@ uint8_t TernaryLogicInfo::GetTernaryControlByte(const TernaryLogicInfo& info, ui // // Arguments: // intrinsic -- id of the intrinsic function. -// clsHnd -- class handle containing the intrinsic function. // method -- method handle of the intrinsic function. // sig -- signature of the intrinsic call. // simdBaseJitType -- Predetermined simdBaseJitType, could be CORINFO_TYPE_UNDEF @@ -726,10 +725,9 @@ uint8_t TernaryLogicInfo::GetTernaryControlByte(const TernaryLogicInfo& info, ui // Return Value: // The basetype of intrinsic of it can be fetched from 1st or 2nd argument, else return baseType unmodified. // -CorInfoType Compiler::getBaseJitTypeFromArgIfNeeded(NamedIntrinsic intrinsic, - CORINFO_CLASS_HANDLE clsHnd, - CORINFO_SIG_INFO* sig, - CorInfoType simdBaseJitType) +CorInfoType Compiler::getBaseJitTypeFromArgIfNeeded(NamedIntrinsic intrinsic, + CORINFO_SIG_INFO* sig, + CorInfoType simdBaseJitType) { if (HWIntrinsicInfo::BaseTypeFromSecondArg(intrinsic) || HWIntrinsicInfo::BaseTypeFromFirstArg(intrinsic)) { @@ -1760,7 +1758,7 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic, } } - simdBaseJitType = getBaseJitTypeFromArgIfNeeded(intrinsic, clsHnd, sig, simdBaseJitType); + simdBaseJitType = getBaseJitTypeFromArgIfNeeded(intrinsic, sig, simdBaseJitType); if (simdBaseJitType == CORINFO_TYPE_UNDEF) { From eb93133230813d2964e671b9c6ff996919bc7942 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 21 Aug 2024 18:21:31 -0400 Subject: [PATCH 05/11] Add AVX tests --- .../JitBlue/Runtime_106546/Runtime_106546.cs | 61 +++++++++++++++++++ .../Runtime_106546/Runtime_106546.csproj | 1 + 2 files changed, 62 insertions(+) diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.cs b/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.cs index df5fb90cb7d40f..a92d32daed0c61 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.cs @@ -5,6 +5,7 @@ using System.Numerics; using System.Runtime.Intrinsics; using System.Runtime.Intrinsics.Arm; +using System.Runtime.Intrinsics.X86; using Xunit; public class Runtime_106546 @@ -27,6 +28,18 @@ public static void TestShiftRightThrows() } } + [Fact] + public static void TestAvxGatherThrows() + { + if (Avx2.IsSupported) + { + Assert.Throws(() => new Runtime_106546().GatherVector128()); + Assert.Throws(() => new Runtime_106546().GatherVector256()); + Assert.Throws(() => new Runtime_106546().GatherMaskVector128()); + Assert.Throws(() => new Runtime_106546().GatherMaskVector256()); + } + } + // Generated by Fuzzlyn v2.2 on 2024-08-17 15:13:27 // Run on Arm64 MacOS // Seed: 17566447992392941035-vectort,vector64,vector128,armadvsimd,armadvsimdarm64,armaes,armarmbase,armarmbasearm64,armcrc32,armcrc32arm64,armdp,armrdm,armrdmarm64,armsha1,armsha256 @@ -63,4 +76,52 @@ private void ShiftRight() Console.WriteLine(floatVec); } + + private unsafe void GatherVector128() + { + Vector128 indices = Vector128.Create(0, 1, 2, 3); + int[] data = new int[]{1, 2, 3, 4}; + + fixed (int* ptr = data) + { + Console.WriteLine(Avx2.GatherVector128(ptr, indices, 0)); + } + } + + private unsafe void GatherVector256() + { + Vector256 indices = Vector256.Create(0L, 1L, 2L, 3L); + long[] data = new long[]{1L, 2L, 3L, 4L}; + + fixed (long* ptr = data) + { + Console.WriteLine(Avx2.GatherVector256(ptr, indices, 3)); + } + } + + private unsafe void GatherMaskVector128() + { + Vector128 source = Vector128.Zero; + Vector128 indices = Vector128.Create(0, 1, 2, 3); + Vector128 mask = Vector128.AllBitsSet; + int[] data = new int[]{1, 2, 3, 4}; + + fixed (int* ptr = data) + { + Console.WriteLine(Avx2.GatherMaskVector128(source, ptr, indices, mask, 5)); + } + } + + private unsafe void GatherMaskVector256() + { + Vector256 source = Vector256.Zero; + Vector256 indices = Vector256.Create(0L, 1L, 2L, 3L); + Vector256 mask = Vector256.AllBitsSet; + long[] data = new long[]{1L, 2L, 3L, 4L}; + + fixed (long* ptr = data) + { + Console.WriteLine(Avx2.GatherMaskVector256(source, ptr, indices, mask, 7)); + } + } } \ No newline at end of file diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.csproj index 15edd99711a1a4..6bb210527e0797 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.csproj +++ b/src/tests/JIT/Regression/JitBlue/Runtime_106546/Runtime_106546.csproj @@ -1,6 +1,7 @@ True + True From 4fecd8a543d999c28165a145cfa34d0d7fd93538 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Thu, 22 Aug 2024 19:42:31 -0400 Subject: [PATCH 06/11] Use first arg of StoreSelectedScalar for type info --- src/coreclr/jit/hwintrinsiclistarm64.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/hwintrinsiclistarm64.h b/src/coreclr/jit/hwintrinsiclistarm64.h index a7503787e66ee7..faf1c2a41a9c96 100644 --- a/src/coreclr/jit/hwintrinsiclistarm64.h +++ b/src/coreclr/jit/hwintrinsiclistarm64.h @@ -487,7 +487,7 @@ HARDWARE_INTRINSIC(AdvSimd, SignExtendWideningLower, HARDWARE_INTRINSIC(AdvSimd, SignExtendWideningUpper, 16, 1, {INS_sxtl2, INS_invalid, INS_sxtl2, INS_invalid, INS_sxtl2, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_BaseTypeFromFirstArg) HARDWARE_INTRINSIC(AdvSimd, SqrtScalar, 8, 1, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_fsqrt, INS_fsqrt}, HW_Category_SIMD, HW_Flag_SIMDScalar) HARDWARE_INTRINSIC(AdvSimd, Store, 8, 2, {INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_invalid, INS_invalid, INS_st1_2regs, INS_invalid}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen|HW_Flag_NeedsConsecutiveRegisters) -HARDWARE_INTRINSIC(AdvSimd, StoreSelectedScalar, -1, 3, {INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromSecondArg|HW_Flag_HasImmediateOperand|HW_Flag_SIMDScalar|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_NeedsConsecutiveRegisters) +HARDWARE_INTRINSIC(AdvSimd, StoreSelectedScalar, -1, 3, {INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_HasImmediateOperand|HW_Flag_SIMDScalar|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_NeedsConsecutiveRegisters) HARDWARE_INTRINSIC(AdvSimd, StoreVectorAndZip, 8, 2, {INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen|HW_Flag_NeedsConsecutiveRegisters) HARDWARE_INTRINSIC(AdvSimd, Subtract, -1, 2, {INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_fsub, INS_invalid}, HW_Category_SIMD, HW_Flag_NoFlag) HARDWARE_INTRINSIC(AdvSimd, SubtractHighNarrowingLower, 8, 2, {INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_NoFlag) From 13536da112b33630d26e1e972970f8c12d19eba9 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Thu, 22 Aug 2024 19:43:00 -0400 Subject: [PATCH 07/11] Update HWIntrinsicInfo::lookupSimdSize to handle pointer to primitive as base type arg --- src/coreclr/jit/hwintrinsic.cpp | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/hwintrinsic.cpp b/src/coreclr/jit/hwintrinsic.cpp index 759a6d5c60ba23..688e7d5b1ebf0b 100644 --- a/src/coreclr/jit/hwintrinsic.cpp +++ b/src/coreclr/jit/hwintrinsic.cpp @@ -718,7 +718,6 @@ uint8_t TernaryLogicInfo::GetTernaryControlByte(const TernaryLogicInfo& info, ui // // Arguments: // intrinsic -- id of the intrinsic function. -// method -- method handle of the intrinsic function. // sig -- signature of the intrinsic call. // simdBaseJitType -- Predetermined simdBaseJitType, could be CORINFO_TYPE_UNDEF // @@ -1223,14 +1222,24 @@ unsigned HWIntrinsicInfo::lookupSimdSize(Compiler* comp, NamedIntrinsic id, CORI CORINFO_CLASS_HANDLE typeHnd = nullptr; - if (HWIntrinsicInfo::BaseTypeFromFirstArg(id)) + if (HWIntrinsicInfo::BaseTypeFromFirstArg(id) || HWIntrinsicInfo::BaseTypeFromSecondArg(id)) { - typeHnd = comp->info.compCompHnd->getArgClass(sig, sig->args); - } - else if (HWIntrinsicInfo::BaseTypeFromSecondArg(id)) - { - CORINFO_ARG_LIST_HANDLE secondArg = comp->info.compCompHnd->getArgNext(sig->args); - typeHnd = comp->info.compCompHnd->getArgClass(sig, secondArg); + CORINFO_ARG_LIST_HANDLE arg = sig->args; + + if (HWIntrinsicInfo::BaseTypeFromSecondArg(id)) + { + arg = comp->info.compCompHnd->getArgNext(arg); + } + + CorInfoType jitType = strip(comp->info.compCompHnd->getArgType(sig, arg, &typeHnd)); + + if (jitType == CORINFO_TYPE_PTR) + { + typeHnd = comp->info.compCompHnd->getArgClass(sig, arg); + jitType = comp->info.compCompHnd->getChildType(typeHnd, &typeHnd); + assert(jitType != CORINFO_TYPE_UNDEF); + return genTypeSize(JitType2PreciseVarType(jitType)); + } } else { From b80c45db7a2a0fec73cb3f6168260336fdbf40d5 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Thu, 22 Aug 2024 22:31:10 -0400 Subject: [PATCH 08/11] Revert "Update HWIntrinsicInfo::lookupSimdSize to handle pointer to primitive as base type arg" This reverts commit 13536da112b33630d26e1e972970f8c12d19eba9. --- src/coreclr/jit/hwintrinsic.cpp | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/src/coreclr/jit/hwintrinsic.cpp b/src/coreclr/jit/hwintrinsic.cpp index 688e7d5b1ebf0b..759a6d5c60ba23 100644 --- a/src/coreclr/jit/hwintrinsic.cpp +++ b/src/coreclr/jit/hwintrinsic.cpp @@ -718,6 +718,7 @@ uint8_t TernaryLogicInfo::GetTernaryControlByte(const TernaryLogicInfo& info, ui // // Arguments: // intrinsic -- id of the intrinsic function. +// method -- method handle of the intrinsic function. // sig -- signature of the intrinsic call. // simdBaseJitType -- Predetermined simdBaseJitType, could be CORINFO_TYPE_UNDEF // @@ -1222,24 +1223,14 @@ unsigned HWIntrinsicInfo::lookupSimdSize(Compiler* comp, NamedIntrinsic id, CORI CORINFO_CLASS_HANDLE typeHnd = nullptr; - if (HWIntrinsicInfo::BaseTypeFromFirstArg(id) || HWIntrinsicInfo::BaseTypeFromSecondArg(id)) + if (HWIntrinsicInfo::BaseTypeFromFirstArg(id)) { - CORINFO_ARG_LIST_HANDLE arg = sig->args; - - if (HWIntrinsicInfo::BaseTypeFromSecondArg(id)) - { - arg = comp->info.compCompHnd->getArgNext(arg); - } - - CorInfoType jitType = strip(comp->info.compCompHnd->getArgType(sig, arg, &typeHnd)); - - if (jitType == CORINFO_TYPE_PTR) - { - typeHnd = comp->info.compCompHnd->getArgClass(sig, arg); - jitType = comp->info.compCompHnd->getChildType(typeHnd, &typeHnd); - assert(jitType != CORINFO_TYPE_UNDEF); - return genTypeSize(JitType2PreciseVarType(jitType)); - } + typeHnd = comp->info.compCompHnd->getArgClass(sig, sig->args); + } + else if (HWIntrinsicInfo::BaseTypeFromSecondArg(id)) + { + CORINFO_ARG_LIST_HANDLE secondArg = comp->info.compCompHnd->getArgNext(sig->args); + typeHnd = comp->info.compCompHnd->getArgClass(sig, secondArg); } else { From 8bba188c4cd19699cae77f5d5b2682397a9a090c Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Thu, 22 Aug 2024 23:26:06 -0400 Subject: [PATCH 09/11] Special-case StoreSelectedScalar during importation --- src/coreclr/jit/hwintrinsic.cpp | 24 +++++++++++++++++++++++- src/coreclr/jit/hwintrinsiclistarm64.h | 2 +- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/hwintrinsic.cpp b/src/coreclr/jit/hwintrinsic.cpp index 759a6d5c60ba23..649e81a7c28b12 100644 --- a/src/coreclr/jit/hwintrinsic.cpp +++ b/src/coreclr/jit/hwintrinsic.cpp @@ -735,7 +735,17 @@ CorInfoType Compiler::getBaseJitTypeFromArgIfNeeded(NamedIntrinsic intrinsic, if (HWIntrinsicInfo::BaseTypeFromSecondArg(intrinsic)) { - arg = info.compCompHnd->getArgNext(arg); +#ifdef TARGET_ARM64 + // Quirk: For the ValueTuple variants of StoreSelectedScalar, + // avoid trying to get its underlying type here. + // getBaseJitTypeAndSizeOfSIMDType will return CORINFO_TYPE_UNDEF + // because ValueTuple is not an intrinsic type. + // Instead, use the first arg, which is a pointer to the base type. + if (intrinsic != NI_AdvSimd_StoreSelectedScalar) +#endif // TARGET_ARM64 + { + arg = info.compCompHnd->getArgNext(arg); + } } CORINFO_CLASS_HANDLE argClass = info.compCompHnd->getArgClass(sig, arg); @@ -1239,6 +1249,18 @@ unsigned HWIntrinsicInfo::lookupSimdSize(Compiler* comp, NamedIntrinsic id, CORI } CorInfoType simdBaseJitType = comp->getBaseJitTypeAndSizeOfSIMDType(typeHnd, &simdSize); + +#ifdef TARGET_ARM64 + // Quirk: getBaseJitTypeAndSizeOfSIMDType returns CORINFO_TYPE_UNDEF for the ValueTuple + // variant of StoreSelectedScalar, because ValueTuple is not an intrinsic type. + // We know for sure that the ValueTuple holds Vector64 elements, so return its size here. + if ((simdBaseJitType == CORINFO_TYPE_UNDEF) && (id == NI_AdvSimd_StoreSelectedScalar)) + { + assert(HWIntrinsicInfo::BaseTypeFromSecondArg(id)); + return 8; + } +#endif // TARGET_ARM64 + assert((simdSize > 0) && (simdBaseJitType != CORINFO_TYPE_UNDEF)); return simdSize; } diff --git a/src/coreclr/jit/hwintrinsiclistarm64.h b/src/coreclr/jit/hwintrinsiclistarm64.h index faf1c2a41a9c96..a7503787e66ee7 100644 --- a/src/coreclr/jit/hwintrinsiclistarm64.h +++ b/src/coreclr/jit/hwintrinsiclistarm64.h @@ -487,7 +487,7 @@ HARDWARE_INTRINSIC(AdvSimd, SignExtendWideningLower, HARDWARE_INTRINSIC(AdvSimd, SignExtendWideningUpper, 16, 1, {INS_sxtl2, INS_invalid, INS_sxtl2, INS_invalid, INS_sxtl2, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_BaseTypeFromFirstArg) HARDWARE_INTRINSIC(AdvSimd, SqrtScalar, 8, 1, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_fsqrt, INS_fsqrt}, HW_Category_SIMD, HW_Flag_SIMDScalar) HARDWARE_INTRINSIC(AdvSimd, Store, 8, 2, {INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_invalid, INS_invalid, INS_st1_2regs, INS_invalid}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen|HW_Flag_NeedsConsecutiveRegisters) -HARDWARE_INTRINSIC(AdvSimd, StoreSelectedScalar, -1, 3, {INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_HasImmediateOperand|HW_Flag_SIMDScalar|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_NeedsConsecutiveRegisters) +HARDWARE_INTRINSIC(AdvSimd, StoreSelectedScalar, -1, 3, {INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromSecondArg|HW_Flag_HasImmediateOperand|HW_Flag_SIMDScalar|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_NeedsConsecutiveRegisters) HARDWARE_INTRINSIC(AdvSimd, StoreVectorAndZip, 8, 2, {INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen|HW_Flag_NeedsConsecutiveRegisters) HARDWARE_INTRINSIC(AdvSimd, Subtract, -1, 2, {INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_fsub, INS_invalid}, HW_Category_SIMD, HW_Flag_NoFlag) HARDWARE_INTRINSIC(AdvSimd, SubtractHighNarrowingLower, 8, 2, {INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_NoFlag) From 16cc11ee74721cdbf577e1a72a5e4d089ae19f1b Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 23 Aug 2024 13:52:20 -0400 Subject: [PATCH 10/11] Revert "Special-case StoreSelectedScalar during importation" This reverts commit 8bba188c4cd19699cae77f5d5b2682397a9a090c. --- src/coreclr/jit/hwintrinsic.cpp | 24 +----------------------- src/coreclr/jit/hwintrinsiclistarm64.h | 2 +- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/src/coreclr/jit/hwintrinsic.cpp b/src/coreclr/jit/hwintrinsic.cpp index 649e81a7c28b12..759a6d5c60ba23 100644 --- a/src/coreclr/jit/hwintrinsic.cpp +++ b/src/coreclr/jit/hwintrinsic.cpp @@ -735,17 +735,7 @@ CorInfoType Compiler::getBaseJitTypeFromArgIfNeeded(NamedIntrinsic intrinsic, if (HWIntrinsicInfo::BaseTypeFromSecondArg(intrinsic)) { -#ifdef TARGET_ARM64 - // Quirk: For the ValueTuple variants of StoreSelectedScalar, - // avoid trying to get its underlying type here. - // getBaseJitTypeAndSizeOfSIMDType will return CORINFO_TYPE_UNDEF - // because ValueTuple is not an intrinsic type. - // Instead, use the first arg, which is a pointer to the base type. - if (intrinsic != NI_AdvSimd_StoreSelectedScalar) -#endif // TARGET_ARM64 - { - arg = info.compCompHnd->getArgNext(arg); - } + arg = info.compCompHnd->getArgNext(arg); } CORINFO_CLASS_HANDLE argClass = info.compCompHnd->getArgClass(sig, arg); @@ -1249,18 +1239,6 @@ unsigned HWIntrinsicInfo::lookupSimdSize(Compiler* comp, NamedIntrinsic id, CORI } CorInfoType simdBaseJitType = comp->getBaseJitTypeAndSizeOfSIMDType(typeHnd, &simdSize); - -#ifdef TARGET_ARM64 - // Quirk: getBaseJitTypeAndSizeOfSIMDType returns CORINFO_TYPE_UNDEF for the ValueTuple - // variant of StoreSelectedScalar, because ValueTuple is not an intrinsic type. - // We know for sure that the ValueTuple holds Vector64 elements, so return its size here. - if ((simdBaseJitType == CORINFO_TYPE_UNDEF) && (id == NI_AdvSimd_StoreSelectedScalar)) - { - assert(HWIntrinsicInfo::BaseTypeFromSecondArg(id)); - return 8; - } -#endif // TARGET_ARM64 - assert((simdSize > 0) && (simdBaseJitType != CORINFO_TYPE_UNDEF)); return simdSize; } diff --git a/src/coreclr/jit/hwintrinsiclistarm64.h b/src/coreclr/jit/hwintrinsiclistarm64.h index a7503787e66ee7..faf1c2a41a9c96 100644 --- a/src/coreclr/jit/hwintrinsiclistarm64.h +++ b/src/coreclr/jit/hwintrinsiclistarm64.h @@ -487,7 +487,7 @@ HARDWARE_INTRINSIC(AdvSimd, SignExtendWideningLower, HARDWARE_INTRINSIC(AdvSimd, SignExtendWideningUpper, 16, 1, {INS_sxtl2, INS_invalid, INS_sxtl2, INS_invalid, INS_sxtl2, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_BaseTypeFromFirstArg) HARDWARE_INTRINSIC(AdvSimd, SqrtScalar, 8, 1, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_fsqrt, INS_fsqrt}, HW_Category_SIMD, HW_Flag_SIMDScalar) HARDWARE_INTRINSIC(AdvSimd, Store, 8, 2, {INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_invalid, INS_invalid, INS_st1_2regs, INS_invalid}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen|HW_Flag_NeedsConsecutiveRegisters) -HARDWARE_INTRINSIC(AdvSimd, StoreSelectedScalar, -1, 3, {INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromSecondArg|HW_Flag_HasImmediateOperand|HW_Flag_SIMDScalar|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_NeedsConsecutiveRegisters) +HARDWARE_INTRINSIC(AdvSimd, StoreSelectedScalar, -1, 3, {INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_HasImmediateOperand|HW_Flag_SIMDScalar|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_NeedsConsecutiveRegisters) HARDWARE_INTRINSIC(AdvSimd, StoreVectorAndZip, 8, 2, {INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen|HW_Flag_NeedsConsecutiveRegisters) HARDWARE_INTRINSIC(AdvSimd, Subtract, -1, 2, {INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_fsub, INS_invalid}, HW_Category_SIMD, HW_Flag_NoFlag) HARDWARE_INTRINSIC(AdvSimd, SubtractHighNarrowingLower, 8, 2, {INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_NoFlag) From cc27598f0fc293a6c4d18fe407a3db2acd8a9599 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 23 Aug 2024 14:46:19 -0400 Subject: [PATCH 11/11] Add handling for base type from ValueTuple --- src/coreclr/jit/hwintrinsic.cpp | 39 +++++++++++++++++++++++--- src/coreclr/jit/hwintrinsic.h | 10 +++++++ src/coreclr/jit/hwintrinsiclistarm64.h | 2 +- 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/hwintrinsic.cpp b/src/coreclr/jit/hwintrinsic.cpp index 759a6d5c60ba23..ca045d8bb7acfc 100644 --- a/src/coreclr/jit/hwintrinsic.cpp +++ b/src/coreclr/jit/hwintrinsic.cpp @@ -1758,7 +1758,8 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic, } } - simdBaseJitType = getBaseJitTypeFromArgIfNeeded(intrinsic, sig, simdBaseJitType); + simdBaseJitType = getBaseJitTypeFromArgIfNeeded(intrinsic, sig, simdBaseJitType); + unsigned simdSize = 0; if (simdBaseJitType == CORINFO_TYPE_UNDEF) { @@ -1777,7 +1778,7 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic, simdBaseJitType = getBaseJitTypeAndSizeOfSIMDType(clsHnd, &sizeBytes); -#if defined(TARGET_ARM64) +#ifdef TARGET_ARM64 if (simdBaseJitType == CORINFO_TYPE_UNDEF && HWIntrinsicInfo::HasScalarInputVariant(intrinsic)) { // Did not find a valid vector type. The intrinsic has alternate scalar version. Switch to that. @@ -1793,12 +1794,40 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic, assert(simdBaseJitType != CORINFO_TYPE_VALUECLASS); } else -#endif +#endif // TARGET_ARM64 { assert((category == HW_Category_Special) || (category == HW_Category_Helper) || (sizeBytes != 0)); } } } +#ifdef TARGET_ARM64 + else if ((simdBaseJitType == CORINFO_TYPE_VALUECLASS) && (HWIntrinsicInfo::BaseTypeFromValueTupleArg(intrinsic))) + { + // If HW_Flag_BaseTypeFromValueTupleArg is set, one of the base type position flags must be set. + // There is no point to using this flag if the SIMD size is known at compile-time. + assert(HWIntrinsicInfo::BaseTypeFromFirstArg(intrinsic) || HWIntrinsicInfo::BaseTypeFromSecondArg(intrinsic)); + assert(!HWIntrinsicInfo::tryLookupSimdSize(intrinsic, &simdSize)); + + CORINFO_ARG_LIST_HANDLE arg = sig->args; + + if (HWIntrinsicInfo::BaseTypeFromSecondArg(intrinsic)) + { + arg = info.compCompHnd->getArgNext(arg); + } + + CORINFO_CLASS_HANDLE argClass = info.compCompHnd->getArgClass(sig, arg); + INDEBUG(unsigned fieldCount = info.compCompHnd->getClassNumInstanceFields(argClass)); + assert(fieldCount > 1); + + CORINFO_CLASS_HANDLE classHnd; + CORINFO_FIELD_HANDLE fieldHandle = info.compCompHnd->getFieldInClass(argClass, 0); + CorInfoType fieldType = info.compCompHnd->getFieldType(fieldHandle, &classHnd); + assert(isIntrinsicType(classHnd)); + + simdBaseJitType = getBaseJitTypeAndSizeOfSIMDType(classHnd, &simdSize); + assert(simdSize > 0); + } +#endif // TARGET_ARM64 // Immediately return if the category is other than scalar/special and this is not a supported base type. if ((category != HW_Category_Special) && (category != HW_Category_Scalar) && !HWIntrinsicInfo::isScalarIsa(isa) && @@ -1821,7 +1850,9 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic, #endif // TARGET_XARCH } - const unsigned simdSize = HWIntrinsicInfo::lookupSimdSize(this, intrinsic, sig); + // We may have already determined simdSize for intrinsics that require special handling. + // If so, skip the lookup. + simdSize = (simdSize == 0) ? HWIntrinsicInfo::lookupSimdSize(this, intrinsic, sig) : simdSize; HWIntrinsicSignatureReader sigReader; sigReader.Read(info.compCompHnd, sig); diff --git a/src/coreclr/jit/hwintrinsic.h b/src/coreclr/jit/hwintrinsic.h index 06d3f433d48fc9..cbc88369cb03a6 100644 --- a/src/coreclr/jit/hwintrinsic.h +++ b/src/coreclr/jit/hwintrinsic.h @@ -225,6 +225,10 @@ enum HWIntrinsicFlag : unsigned int // (instead of merging). HW_Flag_ZeroingMaskedOperation = 0x800000, + // The intrinsic has an overload where the base type is extracted from a ValueTuple of SIMD types + // (HW_Flag_BaseTypeFrom{First, Second}Arg must also be set to denote the position of the ValueTuple) + HW_Flag_BaseTypeFromValueTupleArg = 0x1000000, + #else #error Unsupported platform #endif @@ -988,6 +992,12 @@ struct HWIntrinsicInfo return (flags & HW_Flag_ZeroingMaskedOperation) != 0; } + static bool BaseTypeFromValueTupleArg(NamedIntrinsic id) + { + const HWIntrinsicFlag flags = lookupFlags(id); + return (flags & HW_Flag_BaseTypeFromValueTupleArg) != 0; + } + static NamedIntrinsic GetScalarInputVariant(NamedIntrinsic id) { assert(HasScalarInputVariant(id)); diff --git a/src/coreclr/jit/hwintrinsiclistarm64.h b/src/coreclr/jit/hwintrinsiclistarm64.h index faf1c2a41a9c96..5f25e123086f38 100644 --- a/src/coreclr/jit/hwintrinsiclistarm64.h +++ b/src/coreclr/jit/hwintrinsiclistarm64.h @@ -487,7 +487,7 @@ HARDWARE_INTRINSIC(AdvSimd, SignExtendWideningLower, HARDWARE_INTRINSIC(AdvSimd, SignExtendWideningUpper, 16, 1, {INS_sxtl2, INS_invalid, INS_sxtl2, INS_invalid, INS_sxtl2, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_BaseTypeFromFirstArg) HARDWARE_INTRINSIC(AdvSimd, SqrtScalar, 8, 1, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_fsqrt, INS_fsqrt}, HW_Category_SIMD, HW_Flag_SIMDScalar) HARDWARE_INTRINSIC(AdvSimd, Store, 8, 2, {INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_st1_2regs, INS_invalid, INS_invalid, INS_st1_2regs, INS_invalid}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen|HW_Flag_NeedsConsecutiveRegisters) -HARDWARE_INTRINSIC(AdvSimd, StoreSelectedScalar, -1, 3, {INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_HasImmediateOperand|HW_Flag_SIMDScalar|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_NeedsConsecutiveRegisters) +HARDWARE_INTRINSIC(AdvSimd, StoreSelectedScalar, -1, 3, {INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1, INS_st1}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromSecondArg|HW_Flag_BaseTypeFromValueTupleArg|HW_Flag_HasImmediateOperand|HW_Flag_SIMDScalar|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_NeedsConsecutiveRegisters) HARDWARE_INTRINSIC(AdvSimd, StoreVectorAndZip, 8, 2, {INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2, INS_st2}, HW_Category_MemoryStore, HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen|HW_Flag_NeedsConsecutiveRegisters) HARDWARE_INTRINSIC(AdvSimd, Subtract, -1, 2, {INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_sub, INS_fsub, INS_invalid}, HW_Category_SIMD, HW_Flag_NoFlag) HARDWARE_INTRINSIC(AdvSimd, SubtractHighNarrowingLower, 8, 2, {INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_subhn, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_NoFlag)