From 3cdaff65da3e1d7bd08ea3c64c128e7056531986 Mon Sep 17 00:00:00 2001 From: Ruihan-Yin Date: Mon, 7 Aug 2023 15:27:52 -0700 Subject: [PATCH 1/4] Add `emitDispEmbBroadcastCount` to display the embedded broadcasted element count. --- src/coreclr/jit/emit.h | 7 +++++++ src/coreclr/jit/emitxarch.cpp | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h index fb3ba74e0e44b9..d23d4641549cb2 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -1541,6 +1541,12 @@ class emitter _idEvexbContext = 1; assert(_idEvexbContext == 1); } + void idClearEvexbContext() + { + assert(_idEvexbContext == 1); + _idEvexbContext = 0; + assert(_idEvexbContext == 0); + } #endif #ifdef TARGET_ARMARCH @@ -2107,6 +2113,7 @@ class emitter void emitDispInsAddr(BYTE* code); void emitDispInsOffs(unsigned offs, bool doffs); void emitDispInsHex(instrDesc* id, BYTE* code, size_t sz); + void emitDispEmbBroadcastCount(instrDesc* id); void emitDispIns(instrDesc* id, bool isNew, bool doffs, diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 5ef0e20eb9de21..c62a808573dcc9 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -10722,6 +10722,20 @@ void emitter::emitDispInsHex(instrDesc* id, BYTE* code, size_t sz) } } +// emitDispEmbBroadcastCount: Display the tag where embedded broadcast is activated to show how many elements are broadcasted. +// +// Arguments: +// id - The instruction descriptor +// +void emitter::emitDispEmbBroadcastCount(instrDesc* id) +{ + ssize_t baseSize = GetInputSizeInBytes(id); + id->idClearEvexbContext(); + ssize_t vectorSize = (ssize_t)emitGetMemOpSize(id); + id->idSetEvexbContext(); + printf(" {1to%d}", vectorSize / baseSize); +} + //-------------------------------------------------------------------- // emitDispIns: Dump the given instruction to jitstdout. // @@ -11123,6 +11137,11 @@ void emitter::emitDispIns( { printf("%s, %s, %s", emitRegName(id->idReg1(), attr), emitRegName(id->idReg2(), attr), sstr); emitDispAddrMode(id); + if(id->idIsEvexbContext()) + { + // we should have the assumption that we are under embedded broadcast use case. + emitDispEmbBroadcastCount(id); + } break; } @@ -11395,6 +11414,11 @@ void emitter::emitDispIns( printf("%s, %s, %s", emitRegName(id->idReg1(), attr), emitRegName(id->idReg2(), attr), sstr); emitDispFrameRef(id->idAddr()->iiaLclVar.lvaVarNum(), id->idAddr()->iiaLclVar.lvaOffset(), id->idDebugOnlyInfo()->idVarRefOffs, asmfm); + if(id->idIsEvexbContext()) + { + // we should have the assumption that we are under embedded broadcast use case. + emitDispEmbBroadcastCount(id); + } break; } @@ -11899,6 +11923,11 @@ void emitter::emitDispIns( printf("%s, %s, %s", emitRegName(id->idReg1(), attr), emitRegName(id->idReg2(), attr), sstr); offs = emitGetInsDsp(id); emitDispClsVar(id->idAddr()->iiaFieldHnd, offs, ID_INFO_DSP_RELOC); + if(id->idIsEvexbContext()) + { + // we should have the assumption that we are under embedded broadcast use case. + emitDispEmbBroadcastCount(id); + } break; } From 2bc44141b2ae1d5e8dffb1da6e9f3208c2aa90f9 Mon Sep 17 00:00:00 2001 From: Ruihan-Yin Date: Tue, 8 Aug 2023 10:54:51 -0700 Subject: [PATCH 2/4] Resolve comments: rename the previous `emitGetMemOpSize` to `emitGetBaseMemOpSize`, the new `emitGetMemOpSize` will check if EVEX.b is set and return the memory size accordingly. --- src/coreclr/jit/emit.h | 38 ++++++++++++++++++++++++++--------- src/coreclr/jit/emitxarch.cpp | 29 ++++++++++++-------------- 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h index d23d4641549cb2..b59775491b5eb5 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -2060,6 +2060,7 @@ class emitter unsigned emitGetInsCIargs(instrDesc* id); inline emitAttr emitGetMemOpSize(instrDesc* id) const; + inline emitAttr emitGetBaseMemOpSize(instrDesc*) const; // Return the argument count for a direct call "id". int emitGetInsCDinfo(instrDesc* id); @@ -3755,21 +3756,15 @@ inline unsigned emitter::emitGetInsCIargs(instrDesc* id) //----------------------------------------------------------------------------- // emitGetMemOpSize: Get the memory operand size of instrDesc. // -// Note: vextractf128 has a 128-bit output (register or memory) but a 256-bit input (register). -// vinsertf128 is the inverse with a 256-bit output (register), a 256-bit input(register), -// and a 128-bit input (register or memory). -// Similarly, vextractf64x4 has a 256-bit output and 128-bit input and vinsertf64x4 the inverse -// This method is mainly used for such instructions to return the appropriate memory operand -// size, otherwise returns the regular operand size of the instruction. +// Note: there are cases when embedded broadcast is enabled, so the memory operand +// size is different from the intrinsic simd size, we will check here if emitter is +// emiting a embedded broadcast enabled instruction. // Arguments: // id - Instruction descriptor // emitAttr emitter::emitGetMemOpSize(instrDesc* id) const { - - emitAttr defaultSize = id->idOpSize(); - instruction ins = id->idIns(); if (id->idIsEvexbContext()) { // should have the assumption that Evex.b now stands for the embedded broadcast context. @@ -3786,6 +3781,31 @@ emitAttr emitter::emitGetMemOpSize(instrDesc* id) const unreached(); } } + else + { + return emitGetBaseMemOpSize(id); + } +} + +//----------------------------------------------------------------------------- +// emitGetMemOpSize: Get the memory operand size of instrDesc. +// +// Note: vextractf128 has a 128-bit output (register or memory) but a 256-bit input (register). +// vinsertf128 is the inverse with a 256-bit output (register), a 256-bit input(register), +// and a 128-bit input (register or memory). +// Similarly, vextractf64x4 has a 256-bit output and 128-bit input and vinsertf64x4 the inverse +// This method is mainly used for such instructions to return the appropriate memory operand +// size, otherwise returns the regular operand size of the instruction. + +// Arguments: +// id - Instruction descriptor +// +emitAttr emitter::emitGetBaseMemOpSize(instrDesc* id) const +{ + + emitAttr defaultSize = id->idOpSize(); + instruction ins = id->idIns(); + switch (ins) { case INS_pextrb: diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index c62a808573dcc9..9e8bacddbeaac0 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -10722,17 +10722,16 @@ void emitter::emitDispInsHex(instrDesc* id, BYTE* code, size_t sz) } } -// emitDispEmbBroadcastCount: Display the tag where embedded broadcast is activated to show how many elements are broadcasted. +// emitDispEmbBroadcastCount: Display the tag where embedded broadcast is activated to show how many elements are +// broadcasted. // // Arguments: // id - The instruction descriptor // void emitter::emitDispEmbBroadcastCount(instrDesc* id) { - ssize_t baseSize = GetInputSizeInBytes(id); - id->idClearEvexbContext(); - ssize_t vectorSize = (ssize_t)emitGetMemOpSize(id); - id->idSetEvexbContext(); + ssize_t baseSize = GetInputSizeInBytes(id); + ssize_t vectorSize = (ssize_t)emitGetBaseMemOpSize(id); printf(" {1to%d}", vectorSize / baseSize); } @@ -11137,12 +11136,11 @@ void emitter::emitDispIns( { printf("%s, %s, %s", emitRegName(id->idReg1(), attr), emitRegName(id->idReg2(), attr), sstr); emitDispAddrMode(id); - if(id->idIsEvexbContext()) + if (!id->idIsEvexbContext()) { - // we should have the assumption that we are under embedded broadcast use case. - emitDispEmbBroadcastCount(id); + break; } - break; + emitDispEmbBroadcastCount(id); } case IF_RRD_ARD_RRD: @@ -11414,12 +11412,11 @@ void emitter::emitDispIns( printf("%s, %s, %s", emitRegName(id->idReg1(), attr), emitRegName(id->idReg2(), attr), sstr); emitDispFrameRef(id->idAddr()->iiaLclVar.lvaVarNum(), id->idAddr()->iiaLclVar.lvaOffset(), id->idDebugOnlyInfo()->idVarRefOffs, asmfm); - if(id->idIsEvexbContext()) + if (!id->idIsEvexbContext()) { - // we should have the assumption that we are under embedded broadcast use case. - emitDispEmbBroadcastCount(id); + break; } - break; + emitDispEmbBroadcastCount(id); } case IF_RWR_RRD_SRD_CNS: @@ -11923,11 +11920,11 @@ void emitter::emitDispIns( printf("%s, %s, %s", emitRegName(id->idReg1(), attr), emitRegName(id->idReg2(), attr), sstr); offs = emitGetInsDsp(id); emitDispClsVar(id->idAddr()->iiaFieldHnd, offs, ID_INFO_DSP_RELOC); - if(id->idIsEvexbContext()) + if (!id->idIsEvexbContext()) { - // we should have the assumption that we are under embedded broadcast use case. - emitDispEmbBroadcastCount(id); + break; } + emitDispEmbBroadcastCount(id); break; } From 27627db59e63f8d5a98161aadd4225d219b133bd Mon Sep 17 00:00:00 2001 From: Ruihan-Yin Date: Tue, 8 Aug 2023 11:05:25 -0700 Subject: [PATCH 3/4] bug fixes: added 2 missing break --- src/coreclr/jit/emitxarch.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 9e8bacddbeaac0..a9842fec2845c5 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -11141,6 +11141,7 @@ void emitter::emitDispIns( break; } emitDispEmbBroadcastCount(id); + break; } case IF_RRD_ARD_RRD: @@ -11417,6 +11418,7 @@ void emitter::emitDispIns( break; } emitDispEmbBroadcastCount(id); + break; } case IF_RWR_RRD_SRD_CNS: From 3048bb4cca617b3f952aa958b8c662dfcd8e22e1 Mon Sep 17 00:00:00 2001 From: Ruihan-Yin Date: Wed, 9 Aug 2023 11:30:20 -0700 Subject: [PATCH 4/4] resolve comments: 1. remove un-needed helper function. 2. move the Evexb check into the display function. --- src/coreclr/jit/emit.h | 6 ------ src/coreclr/jit/emitxarch.cpp | 16 ++++------------ 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/src/coreclr/jit/emit.h b/src/coreclr/jit/emit.h index b59775491b5eb5..5bb32461a9363f 100644 --- a/src/coreclr/jit/emit.h +++ b/src/coreclr/jit/emit.h @@ -1541,12 +1541,6 @@ class emitter _idEvexbContext = 1; assert(_idEvexbContext == 1); } - void idClearEvexbContext() - { - assert(_idEvexbContext == 1); - _idEvexbContext = 0; - assert(_idEvexbContext == 0); - } #endif #ifdef TARGET_ARMARCH diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index a9842fec2845c5..980d40a47ac318 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -10730,6 +10730,10 @@ void emitter::emitDispInsHex(instrDesc* id, BYTE* code, size_t sz) // void emitter::emitDispEmbBroadcastCount(instrDesc* id) { + if (!id->idIsEvexbContext()) + { + return; + } ssize_t baseSize = GetInputSizeInBytes(id); ssize_t vectorSize = (ssize_t)emitGetBaseMemOpSize(id); printf(" {1to%d}", vectorSize / baseSize); @@ -11136,10 +11140,6 @@ void emitter::emitDispIns( { printf("%s, %s, %s", emitRegName(id->idReg1(), attr), emitRegName(id->idReg2(), attr), sstr); emitDispAddrMode(id); - if (!id->idIsEvexbContext()) - { - break; - } emitDispEmbBroadcastCount(id); break; } @@ -11413,10 +11413,6 @@ void emitter::emitDispIns( printf("%s, %s, %s", emitRegName(id->idReg1(), attr), emitRegName(id->idReg2(), attr), sstr); emitDispFrameRef(id->idAddr()->iiaLclVar.lvaVarNum(), id->idAddr()->iiaLclVar.lvaOffset(), id->idDebugOnlyInfo()->idVarRefOffs, asmfm); - if (!id->idIsEvexbContext()) - { - break; - } emitDispEmbBroadcastCount(id); break; } @@ -11922,10 +11918,6 @@ void emitter::emitDispIns( printf("%s, %s, %s", emitRegName(id->idReg1(), attr), emitRegName(id->idReg2(), attr), sstr); offs = emitGetInsDsp(id); emitDispClsVar(id->idAddr()->iiaFieldHnd, offs, ID_INFO_DSP_RELOC); - if (!id->idIsEvexbContext()) - { - break; - } emitDispEmbBroadcastCount(id); break; }