Avx512 single kmask #82255
Avx512 single kmask #82255
Conversation
ghost
added
the
community-contribution
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak Issue DetailsDraft PR for testing. Has a dependency on #80960
|
src/coreclr/jit/lsra.h
Outdated
| @@ -35,6 +35,7 @@ const unsigned int RegisterTypeCount = 2; | |||
| typedef var_types RegisterType; | |||
| #define IntRegisterType TYP_INT | |||
| #define FloatRegisterType TYP_FLOAT | |||
| #define OpmaskRegisterType TYP_OPMASK | |||
There was a problem hiding this comment.
I would just rename this from OpmaskRegister to MaskRegister here and at other places like isMaskReg(), etc.
src/coreclr/jit/vartype.h
Outdated
| template <class T> | ||
| inline bool varTypeIsOpmask(T vt) | ||
| { | ||
| switch (TypeGet(vt)) |
There was a problem hiding this comment.
why not just return TypeGet(v) == TYP_OPMASK;?
| @@ -101,7 +101,8 @@ const char* CodeGen::genInsDisplayName(emitter::instrDesc* id) | |||
| static char buf[4][TEMP_BUFFER_LEN]; | |||
| const char* retbuf; | |||
|
|
|||
| if (GetEmitter()->IsVexEncodedInstruction(ins) && !GetEmitter()->IsBMIInstruction(ins)) | |||
| if (GetEmitter()->IsVexEncodedInstruction(ins) && !GetEmitter()->IsBMIInstruction(ins) && | |||
There was a problem hiding this comment.
Could you explain why is IsKInstruction() included in this condition?
There was a problem hiding this comment.
Because k instructions, i.e.g, kmov, kadd, do not get the v prefix.
| unreached(); | ||
| } | ||
|
|
||
| // opReg should be a kmask reg |
There was a problem hiding this comment.
What do you mean?
Also can we add an assert that assert(isOpmaskReg(maskReg))?
There was a problem hiding this comment.
I think I meant maskReg should be an OpmaskReg. I put the assert in and removed the comment.
| @@ -46,7 +51,7 @@ bool emitter::IsSSEOrAVXInstruction(instruction ins) | |||
| bool emitter::IsAvx512OrPriorInstruction(instruction ins) | |||
| { | |||
| // TODO-XArch-AVX512: Fix check once AVX512 instructions are added. | |||
| return (ins >= INS_FIRST_SSE_INSTRUCTION) && (ins <= INS_LAST_AVX512_INSTRUCTION); | |||
| return ((ins >= INS_FIRST_SSE_INSTRUCTION) && (ins <= INS_LAST_AVX512_INSTRUCTION)) || IsKInstruction(ins); | |||
There was a problem hiding this comment.
update the comment to reflect about the k-mask instruction.
| @@ -1121,6 +1137,8 @@ bool emitter::TakesRexWPrefix(instruction ins, emitAttr attr) | |||
| case INS_vpgatherqq: | |||
| case INS_vgatherdpd: | |||
| case INS_vgatherqpd: | |||
| case INS_vpmovw2m: | |||
There was a problem hiding this comment.
Just that I understand, the other 2 instructions i.e. vpmovb2m and vpmovd2m doesn't take Rex prefix?
There was a problem hiding this comment.
Correct. vpmovb2m and vpmovd2m have w bit set to 0.
src/coreclr/jit/emitxarch.cpp
Outdated
| @@ -9451,8 +9497,17 @@ const char* emitter::emitRegName(regNumber reg, emitAttr attr, bool varName) | |||
| #ifdef TARGET_AMD64 | |||
| char suffix = '\0'; | |||
|
|
|||
| // TODO-XARCH-AVX512 hacky, fix | |||
There was a problem hiding this comment.
We should fix this properly.
src/coreclr/jit/emitxarch.cpp
Outdated
| case INS_kmovd: | ||
| case INS_kmovq: | ||
| { | ||
| result.insLatency += PERFSCORE_LATENCY_1C; |
There was a problem hiding this comment.
We should add proper values as part of the PR.
ghost
added
the
needs-author-action
ghost
removed
the
needs-author-action
anthonycanino
force-pushed
the
avx512-single-kmask
branch
from
13002ae
to
d801c7d
Compare
ghost
locked as resolved and limited conversation to collaborators
Draft PR for testing. Has a dependency on #80960