Skip to content

Commit 2edd5fc

Browse files
amanasifkhalidamanasifkhalid
authored
JIT: Simplify Compiler::getHWIntrinsicImmTypes (#117784)
Numerous SVE2 PRs have had to expand the checks in this function to catch new intrinsics. These checks aren't necessary for computing the intrinsic's immediates' sizes/types. They are essentially debug checks that are (or ought to be) checked downstream. Removing them simplifies later intrinsic additions, and cleans up the function quite a bit.
1 parent 7132299 commit 2edd5fc

File tree

4 files changed

+36
-115
lines changed

4 files changed

+36
-115
lines changed

‎src/coreclr/jit/compiler.h‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4741,11 +4741,6 @@ class Compiler
47414741
void getHWIntrinsicImmTypes(NamedIntrinsic intrinsic,
47424742
CORINFO_SIG_INFO* sig,
47434743
unsigned immNumber,
4744-
var_types simdBaseType,
4745-
CorInfoType simdBaseJitType,
4746-
CORINFO_CLASS_HANDLE op1ClsHnd,
4747-
CORINFO_CLASS_HANDLE op2ClsHnd,
4748-
CORINFO_CLASS_HANDLE op3ClsHnd,
47494744
unsigned* immSimdSize,
47504745
var_types* immSimdBaseType);
47514746

‎src/coreclr/jit/hwintrinsic.cpp‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2054,9 +2054,6 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,
20542054
// If so, skip the lookup.
20552055
simdSize = (simdSize == 0) ? HWIntrinsicInfo::lookupSimdSize(this, intrinsic, sig) : simdSize;
20562056

2057-
HWIntrinsicSignatureReader sigReader;
2058-
sigReader.Read(info.compCompHnd, sig);
2059-
20602057
GenTree* immOp1 = nullptr;
20612058
GenTree* immOp2 = nullptr;
20622059
int immLowerBound = 0;
@@ -2073,8 +2070,7 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,
20732070
{
20742071
unsigned immSimdSize = simdSize;
20752072
var_types immSimdBaseType = simdBaseType;
2076-
getHWIntrinsicImmTypes(intrinsic, sig, 2, simdBaseType, simdBaseJitType, sigReader.op1ClsHnd,
2077-
sigReader.op2ClsHnd, sigReader.op3ClsHnd, &immSimdSize, &immSimdBaseType);
2073+
getHWIntrinsicImmTypes(intrinsic, sig, 2, &immSimdSize, &immSimdBaseType);
20782074
HWIntrinsicInfo::lookupImmBounds(intrinsic, immSimdSize, immSimdBaseType, 2, &immLowerBound, &immUpperBound);
20792075

20802076
if (!CheckHWIntrinsicImmRange(intrinsic, simdBaseJitType, immOp2, mustExpand, immLowerBound, immUpperBound,
@@ -2121,8 +2117,7 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,
21212117
#ifdef TARGET_ARM64
21222118
unsigned immSimdSize = simdSize;
21232119
var_types immSimdBaseType = simdBaseType;
2124-
getHWIntrinsicImmTypes(intrinsic, sig, 1, simdBaseType, simdBaseJitType, sigReader.op1ClsHnd,
2125-
sigReader.op2ClsHnd, sigReader.op3ClsHnd, &immSimdSize, &immSimdBaseType);
2120+
getHWIntrinsicImmTypes(intrinsic, sig, 1, &immSimdSize, &immSimdBaseType);
21262121
HWIntrinsicInfo::lookupImmBounds(intrinsic, immSimdSize, immSimdBaseType, 1, &immLowerBound, &immUpperBound);
21272122
#else
21282123
immUpperBound = HWIntrinsicInfo::lookupImmUpperBound(intrinsic);
@@ -2206,10 +2201,12 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,
22062201
}
22072202
}
22082203

2209-
GenTree* op1 = nullptr;
2210-
GenTree* op2 = nullptr;
2211-
GenTree* op3 = nullptr;
2212-
GenTree* op4 = nullptr;
2204+
GenTree* op1 = nullptr;
2205+
GenTree* op2 = nullptr;
2206+
GenTree* op3 = nullptr;
2207+
GenTree* op4 = nullptr;
2208+
HWIntrinsicSignatureReader sigReader;
2209+
sigReader.Read(info.compCompHnd, sig);
22132210

22142211
switch (numArgs)
22152212
{

‎src/coreclr/jit/hwintrinsicarm64.cpp‎

Lines changed: 26 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -234,96 +234,51 @@ void Compiler::getHWIntrinsicImmOps(NamedIntrinsic intrinsic,
234234
// intrinsic -- NamedIntrinsic associated with the HWIntrinsic to lookup
235235
// sig -- signature of the intrinsic call.
236236
// immNumber -- Which immediate to use (1 for most intrinsics)
237-
// simdBaseType -- base type of the intrinsic
238-
// simdType -- vector size of the intrinsic
239-
// op1ClsHnd -- cls handler for op1
240-
// op2ClsHnd -- cls handler for op2
241-
// op2ClsHnd -- cls handler for op3
242237
// immSimdSize [IN/OUT] -- Size of the immediate to override
243238
// immSimdBaseType [IN/OUT] -- Base type of the immediate to override
244239
//
245-
void Compiler::getHWIntrinsicImmTypes(NamedIntrinsic intrinsic,
246-
CORINFO_SIG_INFO* sig,
247-
unsigned immNumber,
248-
var_types simdBaseType,
249-
CorInfoType simdBaseJitType,
250-
CORINFO_CLASS_HANDLE op1ClsHnd,
251-
CORINFO_CLASS_HANDLE op2ClsHnd,
252-
CORINFO_CLASS_HANDLE op3ClsHnd,
253-
unsigned* immSimdSize,
254-
var_types* immSimdBaseType)
240+
void Compiler::getHWIntrinsicImmTypes(NamedIntrinsic intrinsic,
241+
CORINFO_SIG_INFO* sig,
242+
unsigned immNumber,
243+
unsigned* immSimdSize,
244+
var_types* immSimdBaseType)
255245
{
256246
HWIntrinsicCategory category = HWIntrinsicInfo::lookupCategory(intrinsic);
257247

258248
if (category == HW_Category_SIMDByIndexedElement)
259249
{
260250
assert(immNumber == 1);
251+
*immSimdSize = 0;
252+
CORINFO_ARG_LIST_HANDLE immArg = sig->args;
261253

262-
CorInfoType indexedElementBaseJitType;
263-
var_types indexedElementBaseType;
264-
*immSimdSize = 0;
265-
266-
if (sig->numArgs == 2)
267-
{
268-
indexedElementBaseJitType = getBaseJitTypeAndSizeOfSIMDType(op1ClsHnd, immSimdSize);
269-
indexedElementBaseType = JitType2PreciseVarType(indexedElementBaseJitType);
270-
}
271-
else if (sig->numArgs == 3)
254+
switch (sig->numArgs)
272255
{
273-
indexedElementBaseJitType = getBaseJitTypeAndSizeOfSIMDType(op2ClsHnd, immSimdSize);
274-
indexedElementBaseType = JitType2PreciseVarType(indexedElementBaseJitType);
275-
}
276-
else
277-
{
278-
assert(sig->numArgs == 4);
279-
indexedElementBaseJitType = getBaseJitTypeAndSizeOfSIMDType(op3ClsHnd, immSimdSize);
280-
indexedElementBaseType = JitType2PreciseVarType(indexedElementBaseJitType);
281-
282-
if (intrinsic == NI_Dp_DotProductBySelectedQuadruplet)
283-
{
284-
assert(((simdBaseType == TYP_INT) && (indexedElementBaseType == TYP_BYTE)) ||
285-
((simdBaseType == TYP_UINT) && (indexedElementBaseType == TYP_UBYTE)));
286-
// The second source operand of sdot, udot instructions is an indexed 32-bit element.
287-
indexedElementBaseType = simdBaseType;
288-
}
289-
290-
if (intrinsic == NI_Sve_DotProductBySelectedScalar)
256+
case 4:
257+
immArg = info.compCompHnd->getArgNext(immArg);
258+
FALLTHROUGH;
259+
case 3:
260+
immArg = info.compCompHnd->getArgNext(immArg);
261+
FALLTHROUGH;
262+
case 2:
291263
{
292-
assert(((simdBaseType == TYP_INT) && (indexedElementBaseType == TYP_BYTE)) ||
293-
((simdBaseType == TYP_UINT) && (indexedElementBaseType == TYP_UBYTE)) ||
294-
((simdBaseType == TYP_LONG) && (indexedElementBaseType == TYP_SHORT)) ||
295-
((simdBaseType == TYP_ULONG) && (indexedElementBaseType == TYP_USHORT)));
296-
297-
// The second source operand of sdot, udot instructions is an indexed 32-bit element.
298-
indexedElementBaseType = simdBaseType;
264+
CORINFO_CLASS_HANDLE typeHnd = info.compCompHnd->getArgClass(sig, immArg);
265+
getBaseJitTypeAndSizeOfSIMDType(typeHnd, immSimdSize);
266+
break;
299267
}
268+
default:
269+
unreached();
300270
}
301-
302-
if (intrinsic == NI_Sve2_MultiplyBySelectedScalar ||
303-
intrinsic == NI_Sve2_MultiplyBySelectedScalarWideningEven ||
304-
intrinsic == NI_Sve2_MultiplyBySelectedScalarWideningEvenAndAdd ||
305-
intrinsic == NI_Sve2_MultiplyBySelectedScalarWideningEvenAndSubtract ||
306-
intrinsic == NI_Sve2_MultiplyBySelectedScalarWideningOdd ||
307-
intrinsic == NI_Sve2_MultiplyBySelectedScalarWideningOddAndAdd ||
308-
intrinsic == NI_Sve2_MultiplyBySelectedScalarWideningOddAndSubtract ||
309-
intrinsic == NI_Sve2_MultiplyDoublingWideningBySelectedScalarAndAddSaturateEven ||
310-
intrinsic == NI_Sve2_MultiplyDoublingWideningBySelectedScalarAndAddSaturateOdd ||
311-
intrinsic == NI_Sve2_MultiplyDoublingWideningBySelectedScalarAndSubtractSaturateEven ||
312-
intrinsic == NI_Sve2_MultiplyDoublingWideningBySelectedScalarAndSubtractSaturateOdd ||
313-
intrinsic == NI_Sve2_MultiplySubtractBySelectedScalar)
314-
{
315-
indexedElementBaseType = simdBaseType;
316-
}
317-
318-
assert(indexedElementBaseType == simdBaseType);
319271
}
320272
else if (intrinsic == NI_AdvSimd_Arm64_InsertSelectedScalar)
321273
{
322274
if (immNumber == 2)
323275
{
324-
CorInfoType otherBaseJitType = getBaseJitTypeAndSizeOfSIMDType(op3ClsHnd, immSimdSize);
325-
*immSimdBaseType = JitType2PreciseVarType(otherBaseJitType);
326-
assert(otherBaseJitType == simdBaseJitType);
276+
CORINFO_ARG_LIST_HANDLE immArg = sig->args;
277+
immArg = info.compCompHnd->getArgNext(immArg);
278+
immArg = info.compCompHnd->getArgNext(immArg);
279+
CORINFO_CLASS_HANDLE typeHnd = info.compCompHnd->getArgClass(sig, immArg);
280+
CorInfoType otherBaseJitType = getBaseJitTypeAndSizeOfSIMDType(typeHnd, immSimdSize);
281+
*immSimdBaseType = JitType2PreciseVarType(otherBaseJitType);
327282
}
328283
// For imm1 use default simd sizes.
329284
}

‎src/coreclr/jit/rationalize.cpp‎

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -464,30 +464,6 @@ void Rationalizer::RewriteHWIntrinsicAsUserCall(GenTree** use, ArrayStack<GenTre
464464
break;
465465
}
466466

467-
CORINFO_CLASS_HANDLE op1ClsHnd = NO_CLASS_HANDLE;
468-
CORINFO_CLASS_HANDLE op2ClsHnd = NO_CLASS_HANDLE;
469-
CORINFO_CLASS_HANDLE op3ClsHnd = NO_CLASS_HANDLE;
470-
471-
size_t argCount = operandCount - (sigInfo.hasThis() ? 1 : 0);
472-
473-
if (argCount > 0)
474-
{
475-
CORINFO_ARG_LIST_HANDLE args = sigInfo.args;
476-
comp->info.compCompHnd->getArgType(&sigInfo, args, &op1ClsHnd);
477-
478-
if (argCount > 1)
479-
{
480-
args = comp->info.compCompHnd->getArgNext(args);
481-
comp->info.compCompHnd->getArgType(&sigInfo, args, &op2ClsHnd);
482-
483-
if (argCount > 2)
484-
{
485-
args = comp->info.compCompHnd->getArgNext(args);
486-
comp->info.compCompHnd->getArgType(&sigInfo, args, &op3ClsHnd);
487-
}
488-
}
489-
}
490-
491467
// Position of the immediates from top of stack
492468
int imm1Pos = -1;
493469
int imm2Pos = -1;
@@ -511,8 +487,7 @@ void Rationalizer::RewriteHWIntrinsicAsUserCall(GenTree** use, ArrayStack<GenTre
511487

512488
if (immOp2 != nullptr)
513489
{
514-
comp->getHWIntrinsicImmTypes(intrinsicId, &sigInfo, 2, simdBaseType, simdBaseJitType, op1ClsHnd,
515-
op2ClsHnd, op3ClsHnd, &immSimdSize, &immSimdBaseType);
490+
comp->getHWIntrinsicImmTypes(intrinsicId, &sigInfo, 2, &immSimdSize, &immSimdBaseType);
516491
HWIntrinsicInfo::lookupImmBounds(intrinsicId, immSimdSize, immSimdBaseType, 2, &immLowerBound,
517492
&immUpperBound);
518493

@@ -527,8 +502,7 @@ void Rationalizer::RewriteHWIntrinsicAsUserCall(GenTree** use, ArrayStack<GenTre
527502
immSimdBaseType = simdBaseType;
528503
}
529504

530-
comp->getHWIntrinsicImmTypes(intrinsicId, &sigInfo, 1, simdBaseType, simdBaseJitType, op1ClsHnd, op2ClsHnd,
531-
op3ClsHnd, &immSimdSize, &immSimdBaseType);
505+
comp->getHWIntrinsicImmTypes(intrinsicId, &sigInfo, 1, &immSimdSize, &immSimdBaseType);
532506
HWIntrinsicInfo::lookupImmBounds(intrinsicId, immSimdSize, immSimdBaseType, 1, &immLowerBound,
533507
&immUpperBound);
534508
#endif

0 commit comments

Comments
 (0)