Skip to content

Optimize ToScalar() and GetElement() to use arm64 intrinsic #36156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kunalspathak merged 16 commits into from
May 14, 2020
Merged

Optimize ToScalar() and GetElement() to use arm64 intrinsic #36156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
a9bb336
ARM64 intrisic for ToScalar()
kunalspathak May 2, 2020
b44a3ea
ARM64 intrinsic for GetElement()
kunalspathak May 5, 2020
e11d508
Make ToScalar() no-op for double/float
kunalspathak May 6, 2020
88d93a2
Fixed GetElement to just operate on constants
kunalspathak May 6, 2020
de7ca32
Fix bug in rationalize for Vector64<long>
kunalspathak May 8, 2020
8060e62
fix NotSupported issue for GetElement and ToScalar
kunalspathak May 6, 2020
e0ccf27
Reuse the baseType/retType in impSpecialIntrinsic and impBaseIntrinsic
kunalspathak May 9, 2020
a5397ec
Update comment
kunalspathak May 9, 2020
be741be
fix breaks
kunalspathak May 12, 2020
c7105e3
add comments
kunalspathak May 12, 2020
5c4a1fb
ran jit-format
kunalspathak May 12, 2020
bdfef3c
Refactored to move common logic inside isSupportedBaseType
kunalspathak May 12, 2020
110f368
review comments
kunalspathak May 12, 2020
30173a3
reuse simdSize
kunalspathak May 14, 2020
63a3488
formatting
kunalspathak May 14, 2020
a70d713
one missing formatting
kunalspathak May 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/coreclr/src/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3751,7 +3751,10 @@ class Compiler
GenTree* impSpecialIntrinsic(NamedIntrinsic intrinsic,
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_METHOD_HANDLE method,
CORINFO_SIG_INFO* sig);
CORINFO_SIG_INFO* sig,
var_types baseType,
var_types retType,
unsigned simdSize);

GenTree* getArgForHWIntrinsic(var_types argType, CORINFO_CLASS_HANDLE argClass, bool expectAddr = false);
GenTree* impNonConstFallback(NamedIntrinsic intrinsic, var_types simdType, var_types baseType);
Expand All @@ -3761,7 +3764,10 @@ class Compiler
GenTree* impBaseIntrinsic(NamedIntrinsic intrinsic,
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_METHOD_HANDLE method,
CORINFO_SIG_INFO* sig);
CORINFO_SIG_INFO* sig,
var_types baseType,
var_types retType,
unsigned simdSize);
GenTree* impSSEIntrinsic(NamedIntrinsic intrinsic, CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO* sig);
GenTree* impSSE2Intrinsic(NamedIntrinsic intrinsic, CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO* sig);
GenTree* impAvxOrAvx2Intrinsic(NamedIntrinsic intrinsic, CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO* sig);
Expand Down
67 changes: 65 additions & 2 deletions src/coreclr/src/jit/hwintrinsic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,40 @@ static bool impIsTableDrivenHWIntrinsic(NamedIntrinsic intrinsicId, HWIntrinsicC
!HWIntrinsicInfo::HasSpecialImport(intrinsicId);
}

//------------------------------------------------------------------------
// isSupportedBaseType
//
// Arguments:
// intrinsicId - HW intrinsic id
// baseType - Base type of the intrinsic.
//
// Return Value:
// returns true if the baseType is supported for given intrinsic.
//
static bool isSupportedBaseType(NamedIntrinsic intrinsic, var_types baseType)
{
// We don't actually check the intrinsic outside of the false case as we expect
// the exposed managed signatures are either generic and support all types
// or they are explicit and support the type indicated.
if (varTypeIsArithmetic(baseType))
{
return true;
}

#ifdef TARGET_XARCH
assert((intrinsic >= NI_Vector128_As && intrinsic <= NI_Vector128_AsUInt64) ||
(intrinsic >= NI_Vector128_get_AllBitsSet && intrinsic <= NI_Vector128_ToVector256Unsafe) ||
(intrinsic >= NI_Vector256_As && intrinsic <= NI_Vector256_AsUInt64) ||
(intrinsic >= NI_Vector256_get_AllBitsSet && intrinsic <= NI_Vector256_ToScalar));
#else
assert((intrinsic >= NI_Vector64_AsByte && intrinsic <= NI_Vector64_AsUInt32) ||
(intrinsic >= NI_Vector64_get_AllBitsSet && intrinsic <= NI_Vector64_ToScalar) ||
(intrinsic >= NI_Vector128_As && intrinsic <= NI_Vector128_AsUInt64) ||
(intrinsic >= NI_Vector128_get_AllBitsSet && intrinsic <= NI_Vector128_ToScalar));
#endif
return false;
}

//------------------------------------------------------------------------
// impHWIntrinsic: Import a hardware intrinsic as a GT_HWINTRINSIC node if possible
//
Expand Down Expand Up @@ -658,9 +692,38 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,
baseType = getBaseTypeAndSizeOfSIMDType(sig->retTypeSigClass, &sizeBytes);
retType = getSIMDTypeForSize(sizeBytes);
assert(sizeBytes != 0);

// We want to return early here for cases where retType was TYP_STRUCT as per method signature and
// rather than deferring the decision after getting the baseType of arg.
if (!isSupportedBaseType(intrinsic, baseType))
{
return nullptr;
}
}

baseType = getBaseTypeFromArgIfNeeded(intrinsic, clsHnd, sig, baseType);

if (baseType == TYP_UNKNOWN)
{
if (category != HW_Category_Scalar)
{
unsigned int sizeBytes;
baseType = getBaseTypeAndSizeOfSIMDType(clsHnd, &sizeBytes);
assert((category == HW_Category_Special) || (sizeBytes != 0));
}
else
{
baseType = retType;
}
}

// 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) &&
!isSupportedBaseType(intrinsic, baseType))
{
return nullptr;
}

baseType = getBaseTypeFromArgIfNeeded(intrinsic, clsHnd, sig, baseType);
unsigned simdSize = HWIntrinsicInfo::lookupSimdSize(this, intrinsic, sig);

GenTree* immOp = nullptr;
Expand Down Expand Up @@ -880,7 +943,7 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,
return retNode;
}

return impSpecialIntrinsic(intrinsic, clsHnd, method, sig);
return impSpecialIntrinsic(intrinsic, clsHnd, method, sig, baseType, retType, simdSize);
}

#endif // FEATURE_HW_INTRINSICS
52 changes: 10 additions & 42 deletions src/coreclr/src/jit/hwintrinsicarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ int HWIntrinsicInfo::lookupImmUpperBound(NamedIntrinsic intrinsic, int simdSize,
case NI_AdvSimd_ExtractVector64:
case NI_AdvSimd_Insert:
case NI_AdvSimd_Arm64_DuplicateSelectedScalarToVector128:
case NI_Vector64_GetElement:
case NI_Vector128_GetElement:
immUpperBound = Compiler::getSIMDVectorLength(simdSize, baseType);
break;

Expand Down Expand Up @@ -260,60 +262,26 @@ GenTree* Compiler::impNonConstFallback(NamedIntrinsic intrinsic, var_types simdT
// 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
// sig -- signature of the intrinsic call.
// baseType -- generic argument of the intrinsic.
// retType -- return type of the intrinsic.
//
// Return Value:
// The GT_HWINTRINSIC node, or nullptr if not a supported intrinsic
//
GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
CORINFO_CLASS_HANDLE clsHnd,
CORINFO_METHOD_HANDLE method,
CORINFO_SIG_INFO* sig)
CORINFO_SIG_INFO* sig,
var_types baseType,
var_types retType,
unsigned simdSize)
{
HWIntrinsicCategory category = HWIntrinsicInfo::lookupCategory(intrinsic);
int numArgs = sig->numArgs;
var_types retType = JITtype2varType(sig->retType);
var_types baseType = TYP_UNKNOWN;

if ((retType == TYP_STRUCT) && featureSIMD)
{
unsigned int sizeBytes;
baseType = getBaseTypeAndSizeOfSIMDType(sig->retTypeSigClass, &sizeBytes);
retType = getSIMDTypeForSize(sizeBytes);
assert(sizeBytes != 0);

if (!varTypeIsArithmetic(baseType))
{
assert((intrinsic == NI_Vector64_AsByte) || (intrinsic == NI_Vector128_As) ||
(intrinsic == NI_Vector64_get_Zero) || (intrinsic == NI_Vector64_get_AllBitsSet) ||
(intrinsic == NI_Vector128_get_Zero) || (intrinsic == NI_Vector128_get_AllBitsSet));
return nullptr;
}
}

baseType = getBaseTypeFromArgIfNeeded(intrinsic, clsHnd, sig, baseType);

if (baseType == TYP_UNKNOWN)
{
if (category != HW_Category_Scalar)
{
unsigned int sizeBytes;
baseType = getBaseTypeAndSizeOfSIMDType(clsHnd, &sizeBytes);
assert(sizeBytes != 0);
}
else
{
baseType = retType;
}
}

if (!varTypeIsArithmetic(baseType))
{
return nullptr;
}

unsigned simdSize = HWIntrinsicInfo::lookupSimdSize(this, intrinsic, sig);
assert(numArgs >= 0);
assert(varTypeIsArithmetic(baseType));

GenTree* retNode = nullptr;
GenTree* op1 = nullptr;
Expand Down
23 changes: 23 additions & 0 deletions src/coreclr/src/jit/hwintrinsiccodegenarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,29 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
}
break;

case NI_Vector64_GetElement:
case NI_Vector128_GetElement:
case NI_Vector64_ToScalar:
case NI_Vector128_ToScalar:
{
ssize_t indexValue = 0;
if ((intrin.id == NI_Vector64_GetElement) || (intrin.id == NI_Vector128_GetElement))
{
assert(intrin.op2->IsCnsIntOrI());
indexValue = intrin.op2->AsIntCon()->gtIconVal;
}

// no-op if vector is float/double, targetReg == op1Reg and fetching for 0th index.
if ((varTypeIsFloating(intrin.baseType) && (targetReg == op1Reg) && (indexValue == 0)))
{
break;
}

GetEmitter()->emitIns_R_R_I(ins, emitTypeSize(intrin.baseType), targetReg, op1Reg, indexValue,
INS_OPTS_NONE);
}
break;

default:
unreached();
}
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/src/jit/hwintrinsiclistarm64.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ HARDWARE_INTRINSIC(Vector64, CreateScalarUnsafe,
HARDWARE_INTRINSIC(Vector64, get_AllBitsSet, 8, 0, {INS_mvni, INS_mvni, INS_mvni, INS_mvni, INS_mvni, INS_mvni, INS_mvni, INS_mvni, INS_mvni, INS_mvni}, HW_Category_Helper, HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(Vector64, get_Count, 8, 0, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_Helper, HW_Flag_SpecialImport|HW_Flag_NoCodeGen)
HARDWARE_INTRINSIC(Vector64, get_Zero, 8, 0, {INS_movi, INS_movi, INS_movi, INS_movi, INS_movi, INS_movi, INS_movi, INS_movi, INS_movi, INS_movi}, HW_Category_Helper, HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(Vector64, GetElement, 8, 2, {INS_smov, INS_umov, INS_smov, INS_umov, INS_smov, INS_umov, INS_umov, INS_umov, INS_dup, INS_dup}, HW_Category_IMM, HW_Flag_NoJmpTableIMM|HW_Flag_SupportsContainment|HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(Vector64, ToScalar, 8, 1, {INS_smov, INS_umov, INS_smov, INS_umov, INS_smov, INS_umov, INS_umov, INS_umov, INS_dup, INS_dup}, HW_Category_SIMDScalar, HW_Flag_SpecialCodeGen|HW_Flag_BaseTypeFromFirstArg)

// ***************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************
// ISA Function name SIMD size NumArg Instructions Category Flags
Expand All @@ -50,6 +52,8 @@ HARDWARE_INTRINSIC(Vector128, CreateScalarUnsafe, 1
HARDWARE_INTRINSIC(Vector128, get_AllBitsSet, 16, 0, {INS_mvni, INS_mvni, INS_mvni, INS_mvni, INS_mvni, INS_mvni, INS_mvni, INS_mvni, INS_mvni, INS_mvni}, HW_Category_Helper, HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(Vector128, get_Count, 16, 0, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_Helper, HW_Flag_SpecialImport|HW_Flag_NoCodeGen)
HARDWARE_INTRINSIC(Vector128, get_Zero, 16, 0, {INS_movi, INS_movi, INS_movi, INS_movi, INS_movi, INS_movi, INS_movi, INS_movi, INS_movi, INS_movi}, HW_Category_Helper, HW_Flag_SpecialImport|HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(Vector128, GetElement, 16, 2, {INS_smov, INS_umov, INS_smov, INS_umov, INS_smov, INS_umov, INS_umov, INS_umov, INS_dup, INS_dup}, HW_Category_IMM, HW_Flag_NoJmpTableIMM|HW_Flag_SupportsContainment|HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialCodeGen)
HARDWARE_INTRINSIC(Vector128, ToScalar, 16, 1, {INS_smov, INS_umov, INS_smov, INS_umov, INS_smov, INS_umov, INS_umov, INS_umov, INS_dup, INS_dup}, HW_Category_SIMDScalar, HW_Flag_SpecialCodeGen|HW_Flag_BaseTypeFromFirstArg)

// ***************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************************
// ISA Function name SIMD size NumArg Instructions Category Flags
Expand Down
Loading