Skip to content

Commit

Permalink
Optimize ToScalar() and GetElement() to use arm64 intrinsic (#36156)
Browse files Browse the repository at this point in the history 
* ARM64 intrisic for ToScalar() and GetElement()

* Fixed GetElement to just operate on constants

* Fix bug in rationalize for Vector64<long>

* fix NotSupported issue for GetElement and ToScalar

* Reuse the baseType/retType in impSpecialIntrinsic and impBaseIntrinsic

* Update comment

* fix breaks

* add comments

* ran jit-format

* Refactored to move common logic inside isSupportedBaseType

* review comments

* reuse simdSize

* formatting

* one missing formatting
  • Loading branch information
@kunalspathak
kunalspathak authored May 14, 2020
1 parent 7debfe4 commit 8b24e64
Show file tree
Hide file tree
Showing 9 changed files with 150 additions and 97 deletions.
10 changes: 8 additions & 2 deletions src/coreclr/src/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3752,7 +3752,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 @@ -3762,7 +3765,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
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,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 @@ -614,9 +648,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 @@ -836,7 +899,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
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
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,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
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

0 comments on commit 8b24e64

Please sign in to comment.