Implement Shuffle* Sse2 Intel hardware intrinsics #15777
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5535,15 +5535,28 @@ static bool isSseShift(instruction ins) | |
| } | ||
| } | ||
|
|
||
| //------------------------------------------------------------------------ | ||
| // IsDstSrcImmAvxInstruction: check if instruction has RM R I format | ||
| // for all encodings: EVEX, VEX and legacy SSE | ||
|
There was a problem hiding this comment.
This comment is not correct, certain instructions can be "ins R RM I". There was a problem hiding this comment. @fiigii Can I correct it in next PR. I need There was a problem hiding this comment. Not sure, this PR is still waiting for MS maintainers' final review.
Actually, you can copy the one-line definition of Shuffle into your current workspace, and rebase after this PR gets merged. |
||
| // | ||
| // Arguments: | ||
| // instruction -- processor instruction to check | ||
| // | ||
| // Return Value: | ||
| // true if instruction has RRI format | ||
|
There was a problem hiding this comment. The return vale comment should be modified as well (VEX.NDS can be RRI). There was a problem hiding this comment. Will fix. Question as above: could we postpone it to my next PR? |
||
| // | ||
| static bool IsDstSrcImmAvxInstruction(instruction ins) | ||
| { | ||
| switch (ins) | ||
| { | ||
| case INS_extractps: | ||
| case INS_pextrb: | ||
| case INS_pextrw: | ||
| case INS_pextrd: | ||
| case INS_pextrq: | ||
| case INS_pshufd: | ||
| case INS_pshufhw: | ||
| case INS_pshuflw: | ||
| return true; | ||
| default: | ||
| return false; | ||
|
|
@@ -5554,7 +5567,7 @@ void emitter::emitIns_SIMD_R_R_I(instruction ins, emitAttr attr, regNumber reg, | |
| { | ||
| // TODO-XARCH refactoring emitIns_R_R_I to handle SSE2/AVX2 shift as well as emitIns_R_I | ||
| bool isShift = isSseShift(ins); | ||
| if (IsDstSrcImmAvxInstruction(ins) || (UseVEXEncoding() && !isShift)) | ||
| { | ||
| emitIns_R_R_I(ins, attr, reg, reg1, ival); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -219,19 +219,64 @@ unsigned Compiler::simdSizeOfHWIntrinsic(NamedIntrinsic intrinsic, CORINFO_SIG_I | |
| } | ||
|
|
||
| //------------------------------------------------------------------------ | ||
| // numArgsOfHWIntrinsic: get the number of arguments based on table and | ||
|
There was a problem hiding this comment. Maybe reword to: There was a problem hiding this comment. Sounds better that current one - will change. Next PR would be OK? There was a problem hiding this comment. If you are going to push any more changes, I would fix it this PR. Otherwise I think next PR is fine (provided @CarolEidt agrees). |
||
| // if numArgs is -1 check number of arguments using GenTreeHWIntrinsic | ||
| // node unless it is nullptr | ||
| // | ||
| // Arguments: | ||
| // intrinsic -- id of the intrinsic function | ||
| // node -- GenTreeHWIntrinsic* node with nullptr default value | ||
| // | ||
| // Return Value: | ||
| // number of arguments | ||
| // | ||
| int Compiler::numArgsOfHWIntrinsic(NamedIntrinsic intrinsic, GenTreeHWIntrinsic* node) | ||
| { | ||
| assert(intrinsic != NI_Illegal); | ||
| assert(intrinsic > NI_HW_INTRINSIC_START && intrinsic < NI_HW_INTRINSIC_END); | ||
|
|
||
| int numArgs = hwIntrinsicInfoArray[intrinsic - NI_HW_INTRINSIC_START - 1].numArgs; | ||
| if (numArgs >= 0) | ||
| { | ||
| return numArgs; | ||
| } | ||
|
|
||
| noway_assert(node != nullptr); | ||
|
There was a problem hiding this comment. Add |
||
| assert(numArgs == -1); | ||
|
|
||
| GenTree* op1 = node->gtGetOp1(); | ||
| GenTree* op2 = node->gtGetOp2(); | ||
|
|
||
| if (op2 != nullptr) | ||
| { | ||
| return 2; | ||
| } | ||
|
|
||
| if (op1 != nullptr) | ||
| { | ||
| if (op1->OperIsList()) | ||
| { | ||
| numArgs = 0; | ||
| GenTreeArgList* list = op1->AsArgList(); | ||
|
|
||
| while (list != nullptr) | ||
| { | ||
| numArgs++; | ||
| list = list->Rest(); | ||
| } | ||
|
|
||
| assert(numArgs > 0); | ||
| return numArgs; | ||
| } | ||
| else | ||
| { | ||
| return 1; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| return 0; | ||
| } | ||
| } | ||
|
|
||
| //------------------------------------------------------------------------ | ||
|
|
||
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.
Please remove
= nullptr.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.
Will do.