Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 5.6k
[Arm64] ASIMD By Element Intrinsics#36916
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
66f2c16e1f771358cc8ad1c815afee63943ae0226db2375a3dce2801afd01167319bff4007bcc9221b4244dd74bf331f34e275811fa4db87a0e9c443098adf21e3340d5eab60ed6873c6d8dd304774f36ab5bf86a345590ed5d929edca2d7ef16dd7561db048c7ba8dbb46f603bb7b3c326ed0475327032a2ad5611faee7fb62d4f8aee5c780200b1cb3f4adf4a487474780e0ba806ab5f59750ddace961d022292e4608c8b043ed34369267875ad918acc05fec69da5cc03af71af924e274d9ce3627f12dc7cd8c5be864ca5e301bd0ff14a9df368d7305a4aaad1486c21a9f3008b760b57f20b04a6a8e56944882bb818c94263421d25f945eefb70f56a99716c20c283e686bd40e7cee7db3cb01be48bb35382a4b6601950613fc312d27af9f146File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Diff view
Diff view
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -817,7 +817,7 @@ void emitter::emitInsSanityCheck(instrDesc* id) | ||
| assert(isVectorRegister(id->idReg2())); | ||
| assert(isVectorRegister(id->idReg3())); | ||
| elemsize = optGetElemsize(id->idInsOpt()); | ||
| assert(isValidVectorIndex(id->idOpSize(), elemsize, emitGetInsSC(id))); | ||
| assert(isValidVectorIndex(EA_16BYTE, elemsize, emitGetInsSC(id))); | ||
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. Was this changed to account for the simd8 result with simd16 selection? ContributorAuthor 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. The size of the destination/source registers shouldn't matter here. For example, FMLA Vd.T, Vn.T, Vm.Ts[index]index is always encoded as H:L (2 bits) when T is either 2S or 4S (and Ts is S). In other words, the range of valid values for index is computed based on the assumption that Vm is 16 bytes. | ||
| break; | ||
| case IF_DV_3C: // DV_3C .Q.........mmmmm ......nnnnnddddd Vd Vn Vm (vector) | ||
| @@ -6247,7 +6247,7 @@ void emitter::emitIns_R_R_R_I(instruction ins, | ||
| assert(isValidArrangement(size, opt)); | ||
| elemsize = optGetElemsize(opt); | ||
| assert(isValidVectorElemsizeFloat(elemsize)); | ||
| assert(isValidVectorIndex(size, elemsize, imm)); | ||
| assert(isValidVectorIndex(EA_16BYTE, elemsize, imm)); | ||
| assert(opt != INS_OPTS_1D); // Reserved encoding | ||
| fmt = IF_DV_3BI; | ||
| } | ||
| @@ -6277,7 +6277,7 @@ void emitter::emitIns_R_R_R_I(instruction ins, | ||
| // Only has encodings for H or S elemsize | ||
| assert((elemsize == EA_2BYTE) || (elemsize == EA_4BYTE)); | ||
| // Only has encodings for V0..V15 | ||
| if ((elemsize == EA_2BYTE) && (reg3 >= REG_V16)) | ||
| if ((elemsize == EA_2BYTE) && ((genRegMask(reg3) & RBM_ASIMD_INDEXED_H_ELEMENT_ALLOWED_REGS) == 0)) | ||
| { | ||
| noway_assert(!"Invalid reg3"); | ||
| } | ||
| @@ -6368,6 +6368,11 @@ void emitter::emitIns_R_R_R_I(instruction ins, | ||
| assert((opt == INS_OPTS_4H) || (opt == INS_OPTS_2S)); | ||
| elemsize = optGetElemsize(opt); | ||
| assert(isValidVectorIndex(EA_16BYTE, elemsize, imm)); | ||
| // Restricted to V0-V15 when element size is H | ||
| if ((elemsize == EA_2BYTE) && ((genRegMask(reg3) & RBM_ASIMD_INDEXED_H_ELEMENT_ALLOWED_REGS) == 0)) | ||
| { | ||
| assert(!"Invalid reg3"); | ||
| } | ||
| fmt = IF_DV_3HI; | ||
| break; | ||
| @@ -6384,6 +6389,11 @@ void emitter::emitIns_R_R_R_I(instruction ins, | ||
| assert((opt == INS_OPTS_8H) || (opt == INS_OPTS_4S)); | ||
| elemsize = optGetElemsize(opt); | ||
| assert(isValidVectorIndex(EA_16BYTE, elemsize, imm)); | ||
| // Restricted to V0-V15 when element size is H | ||
| if ((elemsize == EA_2BYTE) && ((genRegMask(reg3) & RBM_ASIMD_INDEXED_H_ELEMENT_ALLOWED_REGS) == 0)) | ||
| { | ||
| assert(!"Invalid reg3"); | ||
| } | ||
| fmt = IF_DV_3HI; | ||
| break; | ||
| @@ -10966,7 +10976,7 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp) | ||
| code = emitInsCode(ins, fmt); | ||
| imm = emitGetInsSC(id); | ||
| elemsize = optGetElemsize(id->idInsOpt()); | ||
| assert(isValidVectorIndex(id->idOpSize(), elemsize, imm)); | ||
| assert(isValidVectorIndex(EA_16BYTE, elemsize, imm)); | ||
| code |= insEncodeVectorsize(id->idOpSize()); // Q | ||
| code |= insEncodeFloatElemsize(elemsize); // X | ||
| code |= insEncodeFloatIndex(elemsize, imm); // L H | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -4728,8 +4728,8 @@ struct GenTreeJitIntrinsic : public GenTreeOp | ||
| ClassLayout* m_layout; | ||
| union { | ||
| var_types gtOtherBaseType; // For AVX2 Gather* intrinsics | ||
| regNumberSmall gtOtherReg; // For intrinsics that return 2 registers | ||
| var_types gtAuxiliaryType; // For intrinsics than need another type (e.g. Avx2.Gather* or SIMD (by element)) | ||
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. Why rename to ContributorAuthor 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. Since I didn't touch gtOtherReg in other files. The intent of the renaming was to get rid of BaseType since for SIMD By Element intrinsics I use this field to encode a SIMD type of an indexed element while in some other case we do use it to keep the "other" base type (e.g. in case of wide/long intrinsics). I could've renamed it to gtOtherType. Do you want me to rename the gtOtherReg -> gtAuxiliaryReg? 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. Was mostly just interested. This makes sense, thanks! Contributor 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. I like the renaming. I think it's reasonable to keep the reg as | ||
| regNumberSmall gtOtherReg; // For intrinsics that return 2 registers | ||
| }; | ||
| public: | ||
| @@ -4766,14 +4766,14 @@ struct GenTreeJitIntrinsic : public GenTreeOp | ||
| assert(gtOtherReg == reg); | ||
| } | ||
| var_types GetOtherBaseType() const | ||
| var_types GetAuxiliaryType() const | ||
| { | ||
| return gtOtherBaseType; | ||
| return gtAuxiliaryType; | ||
| } | ||
| void SetOtherBaseType(var_types type) | ||
| void SetAuxiliaryType(var_types type) | ||
| { | ||
| gtOtherBaseType = type; | ||
| gtAuxiliaryType = type; | ||
| } | ||
| GenTreeJitIntrinsic(genTreeOps oper, var_types type, GenTree* op1, GenTree* op2, var_types baseType, unsigned size) | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
It's minor, but thanks for renaming this - even if/when it was the last op, semantically it's more important that it's an immediate.