-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[Arm64] ASIMD By Element Intrinsics #36916
Changes from 75 commits
66f2c16
e1f7713
58cc8ad
1c815af
ee63943
ae0226d
b2375a3
dce2801
afd0116
7319bff
4007bcc
9221b42
44dd74b
f331f34
e275811
fa4db87
a0e9c44
3098adf
21e3340
d5eab60
ed6873c
6d8dd30
4774f36
ab5bf86
a345590
ed5d929
edca2d7
ef16dd7
561db04
8c7ba8d
bb46f60
3bb7b3c
326ed04
7532703
2a2ad56
11faee7
fb62d4f
8aee5c7
80200b1
cb3f4ad
f4a4874
74780e0
ba806ab
5f59750
ddace96
1d02229
2e4608c
8b043ed
3436926
7875ad9
18acc05
fec69da
5cc03af
71af924
e274d9c
e3627f1
2dc7cd8
c5be864
ca5e301
bd0ff14
a9df368
d7305a4
aaad148
6c21a9f
3008b76
0b57f20
b04a6a8
e569448
82bb818
c942634
21d25f9
45eefb7
0f56a99
716c20c
283e686
bd40e7c
ee7db3c
b01be48
bb35382
a4b6601
950613f
c312d27
af9f146
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 | ||||
---|---|---|---|---|---|---|
|
@@ -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))); | ||||||
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? 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) | ||||||
|
@@ -5827,7 +5827,6 @@ void emitter::emitIns_R_R_R( | |||||
elemsize = EA_1BYTE; | ||||||
opt = optMakeArrangement(size, elemsize); | ||||||
} | ||||||
assert(isValidArrangement(size, opt)); | ||||||
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'm curious why these asserts were removed - are they invalid or somehow redundant? 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. Good point! For some reason I mistakenly though that they are redundant - I will revert this commit. |
||||||
fmt = IF_DV_3C; | ||||||
break; | ||||||
} | ||||||
|
@@ -5851,7 +5850,6 @@ void emitter::emitIns_R_R_R( | |||||
elemsize = EA_1BYTE; | ||||||
opt = optMakeArrangement(size, elemsize); | ||||||
} | ||||||
assert(isValidArrangement(size, opt)); | ||||||
fmt = IF_DV_3C; | ||||||
break; | ||||||
|
||||||
|
@@ -6247,7 +6245,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; | ||||||
} | ||||||
|
@@ -6368,6 +6366,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) && (reg3 >= REG_V16)) | ||||||
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. To make the conditions consistent, perhaps this could be
Suggested change
|
||||||
{ | ||||||
assert(!"Invalid reg3"); | ||||||
} | ||||||
fmt = IF_DV_3HI; | ||||||
break; | ||||||
|
||||||
|
@@ -6384,6 +6387,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) && (reg3 >= REG_V16)) | ||||||
{ | ||||||
assert(!"Invalid reg3"); | ||||||
} | ||||||
fmt = IF_DV_3HI; | ||||||
break; | ||||||
|
||||||
|
@@ -10966,7 +10974,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)) | ||
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 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? 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! 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) | ||
|
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.