Skip to content

Commit cd42382

Browse files
committed
Refactor handling of two immediates in hwintrinsic
1 parent c754a25 commit cd42382

File tree

8 files changed

+307
-172
lines changed

8 files changed

+307
-172
lines changed

src/coreclr/jit/codegen.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1009,7 +1009,7 @@ class CodeGen final : public CodeGenInterface
10091009
class HWIntrinsicImmOpHelper final
10101010
{
10111011
public:
1012-
HWIntrinsicImmOpHelper(CodeGen* codeGen, GenTree* immOp, GenTreeHWIntrinsic* intrin);
1012+
HWIntrinsicImmOpHelper(CodeGen* codeGen, GenTree* immOp, GenTreeHWIntrinsic* intrin, int immNum = 1);
10131013

10141014
void EmitBegin();
10151015
void EmitCaseEnd();

src/coreclr/jit/compiler.h

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4600,6 +4600,34 @@ class Compiler
46004600
NamedIntrinsic intrinsic, GenTree* immOp, bool mustExpand, int immLowerBound, int immUpperBound);
46014601
GenTree* addRangeCheckForHWIntrinsic(GenTree* immOp, int immLowerBound, int immUpperBound);
46024602

4603+
void getHWIntrinsicImmOps(NamedIntrinsic intrinsic,
4604+
CORINFO_SIG_INFO* sig,
4605+
GenTree** immOp1Ptr,
4606+
GenTree** immOp2Ptr);
4607+
4608+
bool CheckHWIntrinsicImmRange(NamedIntrinsic intrinsic,
4609+
CorInfoType simdBaseJitType,
4610+
GenTree* immOp,
4611+
bool mustExpand,
4612+
int immLowerBound,
4613+
int immUpperBound,
4614+
bool hasFullRangeImm,
4615+
bool *useFallback);
4616+
4617+
#if defined(TARGET_ARM64)
4618+
4619+
void getHWIntrinsicImmTypes(NamedIntrinsic intrinsic,
4620+
CORINFO_SIG_INFO* sig,
4621+
unsigned immNumber,
4622+
var_types simdBaseType,
4623+
CorInfoType simdBaseJitType,
4624+
CORINFO_CLASS_HANDLE op2ClsHnd,
4625+
CORINFO_CLASS_HANDLE op3ClsHnd,
4626+
unsigned* immSimdSize,
4627+
var_types* immSimdBaseType);
4628+
4629+
#endif // TARGET_ARM64
4630+
46034631
#endif // FEATURE_HW_INTRINSICS
46044632
GenTree* impArrayAccessIntrinsic(CORINFO_CLASS_HANDLE clsHnd,
46054633
CORINFO_SIG_INFO* sig,

src/coreclr/jit/hwintrinsic.cpp

Lines changed: 109 additions & 162 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,6 +1032,81 @@ struct HWIntrinsicSignatureReader final
10321032
}
10331033
};
10341034

1035+
bool Compiler::CheckHWIntrinsicImmRange(NamedIntrinsic intrinsic,
1036+
CorInfoType simdBaseJitType,
1037+
GenTree* immOp,
1038+
bool mustExpand,
1039+
int immLowerBound,
1040+
int immUpperBound,
1041+
bool hasFullRangeImm,
1042+
bool* useFallback)
1043+
{
1044+
*useFallback = false;
1045+
1046+
if (!hasFullRangeImm && immOp->IsCnsIntOrI())
1047+
{
1048+
const int ival = (int)immOp->AsIntCon()->IconValue();
1049+
bool immOutOfRange;
1050+
#ifdef TARGET_XARCH
1051+
if (HWIntrinsicInfo::isAVX2GatherIntrinsic(intrinsic))
1052+
{
1053+
immOutOfRange = (ival != 1) && (ival != 2) && (ival != 4) && (ival != 8);
1054+
}
1055+
else
1056+
#endif
1057+
{
1058+
immOutOfRange = (ival < immLowerBound) || (ival > immUpperBound);
1059+
}
1060+
1061+
if (immOutOfRange)
1062+
{
1063+
assert(!mustExpand);
1064+
// The imm-HWintrinsics that do not accept all imm8 values may throw
1065+
// ArgumentOutOfRangeException when the imm argument is not in the valid range
1066+
return false;
1067+
}
1068+
}
1069+
else if (!immOp->IsCnsIntOrI())
1070+
{
1071+
if (HWIntrinsicInfo::NoJmpTableImm(intrinsic))
1072+
{
1073+
*useFallback = true;
1074+
return false;
1075+
}
1076+
#if defined(TARGET_XARCH)
1077+
else if (HWIntrinsicInfo::MaybeNoJmpTableImm(intrinsic))
1078+
{
1079+
#if defined(TARGET_X86)
1080+
var_types simdBaseType = JitType2PreciseVarType(simdBaseJitType);
1081+
1082+
if (varTypeIsLong(simdBaseType))
1083+
{
1084+
if (!mustExpand)
1085+
{
1086+
return false;
1087+
}
1088+
}
1089+
else
1090+
#endif // TARGET_XARCH
1091+
{
1092+
*useFallback = true;
1093+
return false;
1094+
}
1095+
}
1096+
#endif // TARGET_XARCH
1097+
else if (!mustExpand)
1098+
{
1099+
// When the imm-argument is not a constant and we are not being forced to expand, we need to
1100+
// return nullptr so a GT_CALL to the intrinsic method is emitted instead. The
1101+
// intrinsic method is recursive and will be forced to expand, at which point
1102+
// we emit some less efficient fallback code.
1103+
return false;
1104+
}
1105+
}
1106+
1107+
return true;
1108+
}
1109+
10351110
//------------------------------------------------------------------------
10361111
// impHWIntrinsic: Import a hardware intrinsic as a GT_HWINTRINSIC node if possible
10371112
//
@@ -1162,190 +1237,62 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,
11621237
}
11631238

11641239
var_types simdBaseType = TYP_UNKNOWN;
1165-
GenTree* immOp = nullptr;
11661240

11671241
if (simdBaseJitType != CORINFO_TYPE_UNDEF)
11681242
{
11691243
simdBaseType = JitType2PreciseVarType(simdBaseJitType);
11701244
}
11711245

1246+
const unsigned simdSize = HWIntrinsicInfo::lookupSimdSize(this, intrinsic, sig);
1247+
11721248
HWIntrinsicSignatureReader sigReader;
11731249
sigReader.Read(info.compCompHnd, sig);
11741250

1175-
#ifdef TARGET_ARM64
1176-
if ((intrinsic == NI_AdvSimd_Insert) || (intrinsic == NI_AdvSimd_InsertScalar) ||
1177-
((intrinsic >= NI_AdvSimd_LoadAndInsertScalar) && (intrinsic <= NI_AdvSimd_LoadAndInsertScalarVector64x4)) ||
1178-
((intrinsic >= NI_AdvSimd_Arm64_LoadAndInsertScalar) &&
1179-
(intrinsic <= NI_AdvSimd_Arm64_LoadAndInsertScalarVector128x4)))
1180-
{
1181-
assert(sig->numArgs == 3);
1182-
immOp = impStackTop(1).val;
1183-
assert(HWIntrinsicInfo::isImmOp(intrinsic, immOp));
1184-
}
1185-
else if (intrinsic == NI_AdvSimd_Arm64_InsertSelectedScalar)
1186-
{
1187-
// InsertSelectedScalar intrinsic has two immediate operands.
1188-
// Since all the remaining intrinsics on both platforms have only one immediate
1189-
// operand, in order to not complicate the shared logic even further we ensure here that
1190-
// 1) The second immediate operand immOp2 is constant and
1191-
// 2) its value belongs to [0, sizeof(op3) / sizeof(op3.BaseType)).
1192-
// If either is false, we should fallback to the managed implementation Insert(dst, dstIdx, Extract(src,
1193-
// srcIdx)).
1194-
// The check for the first immediate operand immOp will use the same logic as other intrinsics that have an
1195-
// immediate operand.
1196-
1197-
GenTree* immOp2 = nullptr;
1198-
1199-
assert(sig->numArgs == 4);
1200-
1201-
immOp = impStackTop(2).val;
1202-
immOp2 = impStackTop().val;
1203-
1204-
assert(HWIntrinsicInfo::isImmOp(intrinsic, immOp));
1205-
assert(HWIntrinsicInfo::isImmOp(intrinsic, immOp2));
1206-
1207-
if (!immOp2->IsCnsIntOrI())
1208-
{
1209-
assert(HWIntrinsicInfo::NoJmpTableImm(intrinsic));
1210-
return impNonConstFallback(intrinsic, retType, simdBaseJitType);
1211-
}
1212-
1213-
unsigned int otherSimdSize = 0;
1214-
CorInfoType otherBaseJitType = getBaseJitTypeAndSizeOfSIMDType(sigReader.op3ClsHnd, &otherSimdSize);
1215-
var_types otherBaseType = JitType2PreciseVarType(otherBaseJitType);
1216-
1217-
assert(otherBaseJitType == simdBaseJitType);
1218-
1219-
int immLowerBound2 = 0;
1220-
int immUpperBound2 = 0;
1251+
GenTree* immOp1 = nullptr;
1252+
GenTree* immOp2 = nullptr;
1253+
int immLowerBound = 0;
1254+
int immUpperBound = 0;
1255+
bool hasFullRangeImm = false;
1256+
bool useFallback = false;
12211257

1222-
HWIntrinsicInfo::lookupImmBounds(intrinsic, otherSimdSize, otherBaseType, &immLowerBound2, &immUpperBound2);
1258+
getHWIntrinsicImmOps(intrinsic, sig, &immOp1, &immOp2);
12231259

1224-
const int immVal2 = (int)immOp2->AsIntCon()->IconValue();
1225-
1226-
if ((immVal2 < immLowerBound2) || (immVal2 > immUpperBound2))
1260+
// Validate the second immediate
1261+
#ifdef TARGET_ARM64
1262+
if (immOp2 != nullptr)
1263+
{
1264+
unsigned immSimdSize = simdSize;
1265+
var_types immSimdBaseType = simdBaseType;
1266+
getHWIntrinsicImmTypes(intrinsic, sig, 2, simdBaseType, simdBaseJitType, sigReader.op2ClsHnd,
1267+
sigReader.op3ClsHnd, &immSimdSize, &immSimdBaseType);
1268+
HWIntrinsicInfo::lookupImmBounds(intrinsic, immSimdSize, immSimdBaseType, 2, &immLowerBound, &immUpperBound);
1269+
1270+
if (!CheckHWIntrinsicImmRange(intrinsic, simdBaseJitType, immOp2, mustExpand, immLowerBound, immUpperBound,
1271+
false, &useFallback))
12271272
{
1228-
assert(!mustExpand);
1229-
return nullptr;
1273+
return useFallback ? impNonConstFallback(intrinsic, retType, simdBaseJitType) : nullptr;
12301274
}
12311275
}
1232-
else
12331276
#endif
1234-
if ((sig->numArgs > 0) && HWIntrinsicInfo::isImmOp(intrinsic, impStackTop().val))
1235-
{
1236-
// NOTE: The following code assumes that for all intrinsics
1237-
// taking an immediate operand, that operand will be last.
1238-
immOp = impStackTop().val;
1239-
}
12401277

1241-
const unsigned simdSize = HWIntrinsicInfo::lookupSimdSize(this, intrinsic, sig);
1242-
1243-
int immLowerBound = 0;
1244-
int immUpperBound = 0;
1245-
bool hasFullRangeImm = false;
1246-
1247-
if (immOp != nullptr)
1278+
// Validate the first immediate
1279+
if (immOp1 != nullptr)
12481280
{
1249-
#ifdef TARGET_XARCH
1281+
#ifdef TARGET_ARM64
1282+
unsigned immSimdSize = simdSize;
1283+
var_types immSimdBaseType = simdBaseType;
1284+
getHWIntrinsicImmTypes(intrinsic, sig, 1, simdBaseType, simdBaseJitType, sigReader.op2ClsHnd,
1285+
sigReader.op3ClsHnd, &immSimdSize, &immSimdBaseType);
1286+
HWIntrinsicInfo::lookupImmBounds(intrinsic, immSimdSize, immSimdBaseType, 1, &immLowerBound, &immUpperBound);
1287+
#else
12501288
immUpperBound = HWIntrinsicInfo::lookupImmUpperBound(intrinsic);
12511289
hasFullRangeImm = HWIntrinsicInfo::HasFullRangeImm(intrinsic);
1252-
#elif defined(TARGET_ARM64)
1253-
if (category == HW_Category_SIMDByIndexedElement)
1254-
{
1255-
CorInfoType indexedElementBaseJitType;
1256-
var_types indexedElementBaseType;
1257-
unsigned int indexedElementSimdSize = 0;
1258-
1259-
if (numArgs == 3)
1260-
{
1261-
indexedElementBaseJitType =
1262-
getBaseJitTypeAndSizeOfSIMDType(sigReader.op2ClsHnd, &indexedElementSimdSize);
1263-
indexedElementBaseType = JitType2PreciseVarType(indexedElementBaseJitType);
1264-
}
1265-
else
1266-
{
1267-
assert(numArgs == 4);
1268-
indexedElementBaseJitType =
1269-
getBaseJitTypeAndSizeOfSIMDType(sigReader.op3ClsHnd, &indexedElementSimdSize);
1270-
indexedElementBaseType = JitType2PreciseVarType(indexedElementBaseJitType);
1271-
1272-
if (intrinsic == NI_Dp_DotProductBySelectedQuadruplet)
1273-
{
1274-
assert(((simdBaseType == TYP_INT) && (indexedElementBaseType == TYP_BYTE)) ||
1275-
((simdBaseType == TYP_UINT) && (indexedElementBaseType == TYP_UBYTE)));
1276-
// The second source operand of sdot, udot instructions is an indexed 32-bit element.
1277-
indexedElementBaseJitType = simdBaseJitType;
1278-
indexedElementBaseType = simdBaseType;
1279-
}
1280-
}
1281-
1282-
assert(indexedElementBaseType == simdBaseType);
1283-
HWIntrinsicInfo::lookupImmBounds(intrinsic, indexedElementSimdSize, simdBaseType, &immLowerBound,
1284-
&immUpperBound);
1285-
}
1286-
else
1287-
{
1288-
HWIntrinsicInfo::lookupImmBounds(intrinsic, simdSize, simdBaseType, &immLowerBound, &immUpperBound);
1289-
}
1290-
#endif
1291-
1292-
if (!hasFullRangeImm && immOp->IsCnsIntOrI())
1293-
{
1294-
const int ival = (int)immOp->AsIntCon()->IconValue();
1295-
bool immOutOfRange;
1296-
#ifdef TARGET_XARCH
1297-
if (HWIntrinsicInfo::isAVX2GatherIntrinsic(intrinsic))
1298-
{
1299-
immOutOfRange = (ival != 1) && (ival != 2) && (ival != 4) && (ival != 8);
1300-
}
1301-
else
13021290
#endif
1303-
{
1304-
immOutOfRange = (ival < immLowerBound) || (ival > immUpperBound);
1305-
}
13061291

1307-
if (immOutOfRange)
1308-
{
1309-
assert(!mustExpand);
1310-
// The imm-HWintrinsics that do not accept all imm8 values may throw
1311-
// ArgumentOutOfRangeException when the imm argument is not in the valid range
1312-
return nullptr;
1313-
}
1314-
}
1315-
else if (!immOp->IsCnsIntOrI())
1292+
if (!CheckHWIntrinsicImmRange(intrinsic, simdBaseJitType, immOp1, mustExpand, immLowerBound, immUpperBound,
1293+
hasFullRangeImm, &useFallback))
13161294
{
1317-
if (HWIntrinsicInfo::NoJmpTableImm(intrinsic))
1318-
{
1319-
return impNonConstFallback(intrinsic, retType, simdBaseJitType);
1320-
}
1321-
#if defined(TARGET_XARCH)
1322-
else if (HWIntrinsicInfo::MaybeNoJmpTableImm(intrinsic))
1323-
{
1324-
#if defined(TARGET_X86)
1325-
var_types simdBaseType = JitType2PreciseVarType(simdBaseJitType);
1326-
1327-
if (varTypeIsLong(simdBaseType))
1328-
{
1329-
if (!mustExpand)
1330-
{
1331-
return nullptr;
1332-
}
1333-
}
1334-
else
1335-
#endif // TARGET_XARCH
1336-
{
1337-
return impNonConstFallback(intrinsic, retType, simdBaseJitType);
1338-
}
1339-
}
1340-
#endif // TARGET_XARCH
1341-
else if (!mustExpand)
1342-
{
1343-
// When the imm-argument is not a constant and we are not being forced to expand, we need to
1344-
// return nullptr so a GT_CALL to the intrinsic method is emitted instead. The
1345-
// intrinsic method is recursive and will be forced to expand, at which point
1346-
// we emit some less efficient fallback code.
1347-
return nullptr;
1348-
}
1295+
return useFallback ? impNonConstFallback(intrinsic, retType, simdBaseJitType) : nullptr;
13491296
}
13501297
}
13511298

src/coreclr/jit/hwintrinsic.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ struct HWIntrinsicInfo
515515
static int lookupImmUpperBound(NamedIntrinsic intrinsic);
516516
#elif defined(TARGET_ARM64)
517517
static void lookupImmBounds(
518-
NamedIntrinsic intrinsic, int simdSize, var_types baseType, int* lowerBound, int* upperBound);
518+
NamedIntrinsic intrinsic, int simdSize, var_types baseType, int immNumber, int* lowerBound, int* upperBound);
519519
#else
520520
#error Unsupported platform
521521
#endif

0 commit comments

Comments
 (0)