-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Update the table-driven framework to support x86 imm-intrinsics #16183
Changes from all commits
92194fe
ea6f850
f1b0c3b
79385b2
7a24967
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3114,12 +3114,17 @@ class Compiler | |
| bool compSupportsHWIntrinsic(InstructionSet isa); | ||
| bool isScalarISA(InstructionSet isa); | ||
| static int ivalOfHWIntrinsic(NamedIntrinsic intrinsic); | ||
| unsigned simdSizeOfHWIntrinsic(NamedIntrinsic intrinsic, CORINFO_SIG_INFO* sig); | ||
| static int numArgsOfHWIntrinsic(NamedIntrinsic intrinsic); | ||
| static GenTree* lastOpOfHWIntrinsic(GenTreeHWIntrinsic* node, int numArgs); | ||
| static instruction insOfHWIntrinsic(NamedIntrinsic intrinsic, var_types type); | ||
| static HWIntrinsicCategory categoryOfHWIntrinsic(NamedIntrinsic intrinsic); | ||
| static HWIntrinsicFlag flagsOfHWIntrinsic(NamedIntrinsic intrinsic); | ||
| GenTree* getArgForHWIntrinsic(var_types argType, CORINFO_CLASS_HANDLE argClass); | ||
| GenTreeArgList* buildArgList(CORINFO_SIG_INFO* sig); | ||
| static int immUpperBoundOfHWIntrinsic(NamedIntrinsic intrinsic); | ||
| GenTree* impNonConstFallback(NamedIntrinsic intrinsic, var_types simdType, var_types baseType); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: It might be good to reorganize this code later. We have table lookup methods mixed in with importer methods. |
||
| static bool isImmHWIntrinsic(NamedIntrinsic intrinsic, GenTree* lastOp); | ||
| GenTree* addRangeCheckIfNeeded(NamedIntrinsic intrinsic, GenTree* lastOp, bool mustExpand); | ||
| #endif // _TARGET_XARCH_ | ||
| #ifdef _TARGET_ARM64_ | ||
| InstructionSet lookupHWIntrinsicISA(const char* className); | ||
|
|
@@ -10039,6 +10044,9 @@ class GenTreeVisitor | |
| #ifdef FEATURE_SIMD | ||
| case GT_SIMD_CHK: | ||
| #endif // FEATURE_SIMD | ||
| #ifdef FEATURE_HW_INTRINSICS | ||
| case GT_HW_INTRINSIC_CHK: | ||
| #endif // FEATURE_HW_INTRINSICS | ||
| { | ||
| GenTreeBoundsChk* const boundsChk = node->AsBoundsChk(); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -187,6 +187,14 @@ bool emitter::IsDstDstSrcAVXInstruction(instruction ins) | |
| case INS_psubusb: | ||
| case INS_psubusw: | ||
| case INS_psubw: | ||
| case INS_pslld: | ||
| case INS_psllq: | ||
| case INS_psllw: | ||
| case INS_psrld: | ||
| case INS_psrlq: | ||
| case INS_psrlw: | ||
| case INS_psrad: | ||
| case INS_psraw: | ||
| case INS_punpckhbw: | ||
| case INS_punpckhdq: | ||
| case INS_punpckhqdq: | ||
|
|
@@ -209,6 +217,11 @@ bool emitter::IsDstDstSrcAVXInstruction(instruction ins) | |
| case INS_vinsertf128: | ||
| case INS_vinserti128: | ||
| case INS_vperm2i128: | ||
| case INS_vpsrlvd: | ||
| case INS_vpsrlvq: | ||
| case INS_vpsravd: | ||
| case INS_vpsllvd: | ||
| case INS_vpsllvq: | ||
| case INS_xorpd: | ||
| case INS_xorps: | ||
| return IsAVXInstruction(ins); | ||
|
|
@@ -366,7 +379,7 @@ bool TakesRexWPrefix(instruction ins, emitAttr attr) | |
| // size specification (128 vs. 256 bits) and the operand size specification (32 vs. 64 bits), where both are | ||
| // required, the instruction must be created with the register size attribute (EA_16BYTE or EA_32BYTE), | ||
| // and here we must special case these by the opcode. | ||
| if (ins == INS_vpermq) | ||
| if (ins == INS_vpermq || ins == INS_vpsrlvq || ins == INS_vpsllvq) | ||
| { | ||
| return true; | ||
| } | ||
|
|
@@ -5479,6 +5492,50 @@ void emitter::emitIns_SIMD_R_R_R(instruction ins, emitAttr attr, regNumber reg, | |
| } | ||
| } | ||
|
|
||
| static bool isSseShift(instruction ins) | ||
| { | ||
| switch (ins) | ||
| { | ||
| case INS_psrldq: | ||
| case INS_pslldq: | ||
| case INS_psrld: | ||
| case INS_psrlw: | ||
| case INS_psrlq: | ||
| case INS_pslld: | ||
| case INS_psllw: | ||
| case INS_psllq: | ||
| case INS_psrad: | ||
| case INS_psraw: | ||
| return true; | ||
| default: | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| void emitter::emitIns_SIMD_R_R_I(instruction ins, emitAttr attr, regNumber reg, regNumber reg1, int ival) | ||
| { | ||
| // TODO-XARCH refactoring emitIns_R_R_I to handle SSE2/AVX2 shift as well as emitIns_R_I | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about this isn't working today?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SSE2 shift instructions require special encoding, we need to improve
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we log an actual bug, as well as this TODO?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| bool isShift = isSseShift(ins); | ||
| if (UseVEXEncoding() && !isShift) | ||
| { | ||
| emitIns_R_R_I(ins, attr, reg, reg1, ival); | ||
| } | ||
| else | ||
| { | ||
| if (reg1 != reg) | ||
| { | ||
| emitIns_R_R(INS_movaps, attr, reg, reg1); | ||
| } | ||
| // TODO-XARCH-BUG emitOutputRI cannot work with SSE2 shift instruction on imm8 > 127, so we replace it by the | ||
| // semantic alternatives. https://github.com/dotnet/coreclr/issues/16543 | ||
| if (isShift && ival > 127) | ||
| { | ||
| ival = 127; | ||
| } | ||
| emitIns_R_I(ins, attr, reg, ival); | ||
| } | ||
| } | ||
|
|
||
| void emitter::emitIns_SIMD_R_R_R_R( | ||
| instruction ins, emitAttr attr, regNumber reg, regNumber reg1, regNumber reg2, regNumber reg3) | ||
| { | ||
|
|
@@ -10746,6 +10803,7 @@ BYTE* emitter::emitOutputRI(BYTE* dst, instrDesc* id) | |
| regOpcode = (regNumber)6; | ||
| break; | ||
| case INS_psrad: | ||
| case INS_psraw: | ||
| regOpcode = (regNumber)4; | ||
| break; | ||
| default: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this one not be static anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is calling
getSIMDTypeSizeInBytes.