From 29b8efdeb2581a207610c0bec95801b92c51ea9a Mon Sep 17 00:00:00 2001 From: Clinton Ingram Date: Tue, 31 Mar 2026 12:19:39 -0700 Subject: [PATCH 1/4] fix disasm of reg to reg SIMD narrowing/widening instructions --- src/coreclr/jit/emitxarch.cpp | 176 ++++++++++++++++++++++++++-------- 1 file changed, 138 insertions(+), 38 deletions(-) diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index ff65e2d883a25f..aaa855038e4242 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -13335,16 +13335,44 @@ void emitter::emitDispIns( } case IF_RRD_RRD: + { + if (ins == INS_bt) + { + // INS_bt operands are reversed. Display them in the normal order. + printf("%s, %s", emitRegName(id->idReg2(), attr), emitRegName(id->idReg1(), attr)); + break; + } + + FALLTHROUGH; + } + case IF_RWR_RRD: + { + if ((ins == INS_rol) || (ins == INS_ror) || (ins == INS_rcl) || (ins == INS_rcr) || (ins == INS_shl) || + (ins == INS_shr) || (ins == INS_sar)) + { + // This is the APX NDD form of these instructions. + printf("%s", emitRegName(id->idReg1(), attr)); + printf(", %s", emitRegName(id->idReg2(), attr)); + emitDispShift(ins, (BYTE)0); + break; + } + + FALLTHROUGH; + } + case IF_RRW_RRD: case IF_RRW_RRW: { + emitAttr tgtAttr = attr; + emitAttr srcAttr = attr; + switch (ins) { case INS_pmovmskb: { assert(!id->idIsEvexAaaContextSet()); - printf("%s, %s", emitRegName(id->idReg1(), EA_4BYTE), emitRegName(id->idReg2(), attr)); + tgtAttr = EA_4BYTE; break; } @@ -13354,7 +13382,7 @@ void emitter::emitDispIns( case INS_cvtsi2sd64: { assert(!id->idIsEvexAaaContextSet()); - printf("%s, %s", emitRegName(id->idReg1(), EA_16BYTE), emitRegName(id->idReg2(), attr)); + tgtAttr = EA_16BYTE; break; } @@ -13384,14 +13412,15 @@ void emitter::emitDispIns( case INS_vcvttss2usis64: { assert(!id->idIsEvexAaaContextSet()); - printf("%s, %s", emitRegName(id->idReg1(), attr), emitRegName(id->idReg2(), EA_16BYTE)); + srcAttr = EA_16BYTE; break; } #ifdef TARGET_AMD64 case INS_movsxd: { - printf("%s, %s", emitRegName(id->idReg1(), EA_8BYTE), emitRegName(id->idReg2(), EA_4BYTE)); + tgtAttr = EA_8BYTE; + srcAttr = EA_4BYTE; break; } #endif // TARGET_AMD64 @@ -13399,14 +13428,7 @@ void emitter::emitDispIns( case INS_movsx: case INS_movzx: { - printf("%s, %s", emitRegName(id->idReg1(), EA_PTRSIZE), emitRegName(id->idReg2(), attr)); - break; - } - - case INS_bt: - { - // INS_bt operands are reversed. Display them in the normal order. - printf("%s, %s", emitRegName(id->idReg2(), attr), emitRegName(id->idReg1(), attr)); + tgtAttr = EA_PTRSIZE; break; } @@ -13416,11 +13438,7 @@ void emitter::emitDispIns( { // The idReg1 is always 4 bytes, but the size of idReg2 can vary. // This logic ensures that we print `crc32 eax, bx` instead of `crc32 ax, bx` - printf("%s, %s", emitRegName(id->idReg1(), EA_4BYTE), emitRegName(id->idReg2(), attr)); - } - else - { - printf("%s, %s", emitRegName(id->idReg1(), attr), emitRegName(id->idReg2(), attr)); + tgtAttr = EA_4BYTE; } break; } @@ -13429,43 +13447,119 @@ void emitter::emitDispIns( case INS_vpbroadcastd_gpr: case INS_vpbroadcastw_gpr: { - printf("%s", emitRegName(id->idReg1(), attr)); - emitDispEmbMasking(id); - printf(", %s", emitRegName(id->idReg2(), EA_4BYTE)); + srcAttr = EA_4BYTE; break; } case INS_vpbroadcastq_gpr: { - printf("%s", emitRegName(id->idReg1(), attr)); - emitDispEmbMasking(id); - printf(", %s", emitRegName(id->idReg2(), EA_8BYTE)); + srcAttr = EA_8BYTE; break; } - case INS_rol: - case INS_ror: - case INS_rcl: - case INS_rcr: - case INS_shl: - case INS_shr: - case INS_sar: + case INS_cvtdq2pd: + case INS_cvtps2pd: + case INS_pmovsxbw: + case INS_pmovsxdq: + case INS_pmovsxwd: + case INS_pmovzxbw: + case INS_pmovzxdq: + case INS_pmovzxwd: + case INS_vcvtph2dq: + case INS_vcvtph2ps: + case INS_vcvtph2psx: + case INS_vcvtph2udq: + case INS_vcvtps2qq: + case INS_vcvtps2uqq: + case INS_vcvttph2dq: + case INS_vcvttph2udq: + case INS_vcvttps2qq: + case INS_vcvttps2qqs: + case INS_vcvttps2uqq: + case INS_vcvttps2uqqs: + case INS_vcvtudq2pd: { - printf("%s", emitRegName(id->idReg1(), attr)); - printf(", %s", emitRegName(id->idReg2(), attr)); - emitDispShift(ins, (BYTE)0); + srcAttr = static_cast(std::max(16U, static_cast EA_SIZE(attr) / 2U)); break; } - default: + case INS_pmovsxbd: + case INS_pmovsxwq: + case INS_pmovzxbd: + case INS_pmovzxwq: + case INS_vcvtph2pd: + case INS_vcvtph2qq: + case INS_vcvtph2uqq: + case INS_vcvttph2qq: + case INS_vcvttph2uqq: + { + srcAttr = static_cast(std::max(16U, static_cast EA_SIZE(attr) / 4U)); + break; + } + + case INS_pmovsxbq: + case INS_pmovzxbq: { - printf("%s", emitRegName(id->idReg1(), attr)); - emitDispEmbMasking(id); - printf(", %s", emitRegName(id->idReg2(), attr)); - emitDispEmbRounding(id); + srcAttr = static_cast(std::max(16U, static_cast EA_SIZE(attr) / 8U)); break; } + + case INS_cvtpd2dq: + case INS_cvtpd2ps: + case INS_cvttpd2dq: + case INS_vpmovdw: + case INS_vpmovqd: + case INS_vpmovsdw: + case INS_vpmovsqd: + case INS_vpmovswb: + case INS_vpmovusdw: + case INS_vpmovusqd: + case INS_vpmovuswb: + case INS_vpmovwb: + case INS_vcvtdq2ph: + case INS_vcvtpd2udq: + case INS_vcvtps2phx: + case INS_vcvtqq2ps: + case INS_vcvtudq2ph: + case INS_vcvtuqq2ps: + case INS_vcvttpd2dqs: + case INS_vcvttpd2udq: + case INS_vcvttpd2udqs: + { + tgtAttr = static_cast(std::max(16U, static_cast EA_SIZE(attr) / 2U)); + break; + } + + case INS_vcvtpd2ph: + case INS_vcvtqq2ph: + case INS_vcvtuqq2ph: + case INS_vpmovdb: + case INS_vpmovqw: + case INS_vpmovsdb: + case INS_vpmovsqw: + case INS_vpmovusdb: + case INS_vpmovusqw: + { + tgtAttr = static_cast(std::max(16U, static_cast EA_SIZE(attr) / 4U)); + break; + } + + case INS_vpmovqb: + case INS_vpmovsqb: + case INS_vpmovusqb: + { + tgtAttr = static_cast(std::max(16U, static_cast EA_SIZE(attr) / 8U)); + break; + } + + default: + break; } + + printf("%s", emitRegName(id->idReg1(), tgtAttr)); + emitDispEmbMasking(id); + printf(", %s", emitRegName(id->idReg2(), srcAttr)); + emitDispEmbRounding(id); break; } @@ -13636,6 +13730,12 @@ void emitter::emitDispIns( break; } + case INS_vcvtps2ph: + { + tgtAttr = static_cast(std::max(16U, static_castEA_SIZE(attr) / 2U)); + break; + } + default: { break; From e189502bb905800215b46c62a331fd69d7db6965 Mon Sep 17 00:00:00 2001 From: Clinton Ingram Date: Tue, 31 Mar 2026 12:30:37 -0700 Subject: [PATCH 2/4] parens --- src/coreclr/jit/emitxarch.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index aaa855038e4242..72bf2a2d4d0246 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -13479,7 +13479,7 @@ void emitter::emitDispIns( case INS_vcvttps2uqqs: case INS_vcvtudq2pd: { - srcAttr = static_cast(std::max(16U, static_cast EA_SIZE(attr) / 2U)); + srcAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 2U)); break; } @@ -13493,14 +13493,14 @@ void emitter::emitDispIns( case INS_vcvttph2qq: case INS_vcvttph2uqq: { - srcAttr = static_cast(std::max(16U, static_cast EA_SIZE(attr) / 4U)); + srcAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 4U)); break; } case INS_pmovsxbq: case INS_pmovzxbq: { - srcAttr = static_cast(std::max(16U, static_cast EA_SIZE(attr) / 8U)); + srcAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 8U)); break; } @@ -13526,7 +13526,7 @@ void emitter::emitDispIns( case INS_vcvttpd2udq: case INS_vcvttpd2udqs: { - tgtAttr = static_cast(std::max(16U, static_cast EA_SIZE(attr) / 2U)); + tgtAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 2U)); break; } @@ -13540,7 +13540,7 @@ void emitter::emitDispIns( case INS_vpmovusdb: case INS_vpmovusqw: { - tgtAttr = static_cast(std::max(16U, static_cast EA_SIZE(attr) / 4U)); + tgtAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 4U)); break; } @@ -13548,7 +13548,7 @@ void emitter::emitDispIns( case INS_vpmovsqb: case INS_vpmovusqb: { - tgtAttr = static_cast(std::max(16U, static_cast EA_SIZE(attr) / 8U)); + tgtAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 8U)); break; } @@ -13732,7 +13732,7 @@ void emitter::emitDispIns( case INS_vcvtps2ph: { - tgtAttr = static_cast(std::max(16U, static_castEA_SIZE(attr) / 2U)); + tgtAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 2U)); break; } From f5853f39ff58d9dc1d437cc48ef9e724ef187ea1 Mon Sep 17 00:00:00 2001 From: Clinton Ingram Date: Sat, 4 Apr 2026 15:11:07 -0700 Subject: [PATCH 3/4] use tuple type for size where possible --- src/coreclr/jit/emitxarch.cpp | 318 +++++++++++++++++----------------- 1 file changed, 160 insertions(+), 158 deletions(-) diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 72bf2a2d4d0246..7c8ce991c4b61c 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -13367,193 +13367,195 @@ void emitter::emitDispIns( emitAttr tgtAttr = attr; emitAttr srcAttr = attr; - switch (ins) - { - case INS_pmovmskb: - { - assert(!id->idIsEvexAaaContextSet()); - tgtAttr = EA_4BYTE; - break; - } + insTupleType tt = insTupleTypeInfo(ins); - case INS_cvtsi2ss32: - case INS_cvtsi2sd32: - case INS_cvtsi2ss64: - case INS_cvtsi2sd64: - { - assert(!id->idIsEvexAaaContextSet()); - tgtAttr = EA_16BYTE; - break; - } - - case INS_cvttsd2si32: - case INS_cvttsd2si64: - case INS_cvtsd2si32: - case INS_cvtsd2si64: - case INS_cvtss2si32: - case INS_cvtss2si64: - case INS_cvttss2si32: - case INS_cvttss2si64: - case INS_vcvtsd2usi32: - case INS_vcvtsd2usi64: - case INS_vcvtss2usi32: - case INS_vcvtss2usi64: - case INS_vcvttsd2usi32: - case INS_vcvttsd2usi64: - case INS_vcvttss2usi32: - case INS_vcvttss2usi64: - case INS_vcvttsd2sis32: - case INS_vcvttsd2sis64: - case INS_vcvttss2sis32: - case INS_vcvttss2sis64: - case INS_vcvttsd2usis32: - case INS_vcvttsd2usis64: - case INS_vcvttss2usis32: - case INS_vcvttss2usis64: + if (tt == INS_TT_NONE) + { + switch (ins) { - assert(!id->idIsEvexAaaContextSet()); - srcAttr = EA_16BYTE; - break; - } + case INS_pmovmskb: + { + assert(!id->idIsEvexAaaContextSet()); + tgtAttr = EA_4BYTE; + break; + } #ifdef TARGET_AMD64 - case INS_movsxd: - { - tgtAttr = EA_8BYTE; - srcAttr = EA_4BYTE; - break; - } + case INS_movsxd: + { + tgtAttr = EA_8BYTE; + srcAttr = EA_4BYTE; + break; + } #endif // TARGET_AMD64 - case INS_movsx: - case INS_movzx: - { - tgtAttr = EA_PTRSIZE; - break; - } + case INS_movsx: + case INS_movzx: + { + tgtAttr = EA_PTRSIZE; + break; + } - case INS_crc32: - { - if (attr != EA_8BYTE) + case INS_crc32: { - // The idReg1 is always 4 bytes, but the size of idReg2 can vary. - // This logic ensures that we print `crc32 eax, bx` instead of `crc32 ax, bx` - tgtAttr = EA_4BYTE; + tgtAttr = std::max(EA_4BYTE, EA_SIZE(attr)); + break; } - break; - } - case INS_vpbroadcastb_gpr: - case INS_vpbroadcastd_gpr: - case INS_vpbroadcastw_gpr: - { - srcAttr = EA_4BYTE; - break; + default: + break; } - case INS_vpbroadcastq_gpr: - { - srcAttr = EA_8BYTE; - break; - } + printf("%s", emitRegName(id->idReg1(), tgtAttr)); + printf(", %s", emitRegName(id->idReg2(), srcAttr)); + break; + } - case INS_cvtdq2pd: - case INS_cvtps2pd: - case INS_pmovsxbw: - case INS_pmovsxdq: - case INS_pmovsxwd: - case INS_pmovzxbw: - case INS_pmovzxdq: - case INS_pmovzxwd: - case INS_vcvtph2dq: - case INS_vcvtph2ps: - case INS_vcvtph2psx: - case INS_vcvtph2udq: - case INS_vcvtps2qq: - case INS_vcvtps2uqq: - case INS_vcvttph2dq: - case INS_vcvttph2udq: - case INS_vcvttps2qq: - case INS_vcvttps2qqs: - case INS_vcvttps2uqq: - case INS_vcvttps2uqqs: - case INS_vcvtudq2pd: - { - srcAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 2U)); - break; - } + unsigned inputSize = static_cast(GetInputSizeInBytes(id)); - case INS_pmovsxbd: - case INS_pmovsxwq: - case INS_pmovzxbd: - case INS_pmovzxwq: - case INS_vcvtph2pd: - case INS_vcvtph2qq: - case INS_vcvtph2uqq: - case INS_vcvttph2qq: - case INS_vcvttph2uqq: + switch (tt) + { + case INS_TT_HALF: + case INS_TT_HALF_MEM: { - srcAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 4U)); - break; - } + unsigned maskElemSize = XMM_REGSIZE_BYTES / CodeGenInterface::instKMaskBaseSize(ins); + emitAttr halfSize = std::max(EA_16BYTE, EA_ATTR(EA_SIZE_IN_BYTES(attr) / 2)); - case INS_pmovsxbq: - case INS_pmovzxbq: - { - srcAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 8U)); + if (inputSize < maskElemSize) + { + assert(inputSize == (maskElemSize / 2)); + srcAttr = halfSize; + } + else + { + tgtAttr = halfSize; + } break; } - case INS_cvtpd2dq: - case INS_cvtpd2ps: - case INS_cvttpd2dq: - case INS_vpmovdw: - case INS_vpmovqd: - case INS_vpmovsdw: - case INS_vpmovsqd: - case INS_vpmovswb: - case INS_vpmovusdw: - case INS_vpmovusqd: - case INS_vpmovuswb: - case INS_vpmovwb: - case INS_vcvtdq2ph: - case INS_vcvtpd2udq: - case INS_vcvtps2phx: - case INS_vcvtqq2ps: - case INS_vcvtudq2ph: - case INS_vcvtuqq2ps: - case INS_vcvttpd2dqs: - case INS_vcvttpd2udq: - case INS_vcvttpd2udqs: + case INS_TT_QUARTER_MEM: { - tgtAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 2U)); - break; - } + unsigned maskElemSize = XMM_REGSIZE_BYTES / CodeGenInterface::instKMaskBaseSize(ins); + emitAttr quarterSize = std::max(EA_16BYTE, EA_ATTR(EA_SIZE_IN_BYTES(attr) / 4)); - case INS_vcvtpd2ph: - case INS_vcvtqq2ph: - case INS_vcvtuqq2ph: - case INS_vpmovdb: - case INS_vpmovqw: - case INS_vpmovsdb: - case INS_vpmovsqw: - case INS_vpmovusdb: - case INS_vpmovusqw: - { - tgtAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 4U)); + if (inputSize < maskElemSize) + { + assert(inputSize == (maskElemSize / 4)); + srcAttr = quarterSize; + } + else + { + tgtAttr = quarterSize; + } break; } - case INS_vpmovqb: - case INS_vpmovsqb: - case INS_vpmovusqb: + case INS_TT_EIGHTH_MEM: { - tgtAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 8U)); + unsigned maskElemSize = XMM_REGSIZE_BYTES / CodeGenInterface::instKMaskBaseSize(ins); + emitAttr eighthSize = std::max(EA_16BYTE, EA_ATTR(EA_SIZE_IN_BYTES(attr) / 8)); + + if (inputSize < maskElemSize) + { + assert(inputSize == (maskElemSize / 8)); + srcAttr = eighthSize; + } + else + { + tgtAttr = eighthSize; + } break; } default: + { + switch (ins) + { + case INS_cvtsi2ss32: + case INS_cvtsi2sd32: + case INS_cvtsi2ss64: + case INS_cvtsi2sd64: + { + assert(!id->idIsEvexAaaContextSet()); + tgtAttr = EA_16BYTE; + break; + } + + case INS_cvttsd2si32: + case INS_cvttsd2si64: + case INS_cvtsd2si32: + case INS_cvtsd2si64: + case INS_cvtss2si32: + case INS_cvtss2si64: + case INS_cvttss2si32: + case INS_cvttss2si64: + case INS_vcvtsd2usi32: + case INS_vcvtsd2usi64: + case INS_vcvtss2usi32: + case INS_vcvtss2usi64: + case INS_vcvttsd2usi32: + case INS_vcvttsd2usi64: + case INS_vcvttss2usi32: + case INS_vcvttss2usi64: + case INS_vcvttsd2sis32: + case INS_vcvttsd2sis64: + case INS_vcvttss2sis32: + case INS_vcvttss2sis64: + case INS_vcvttsd2usis32: + case INS_vcvttsd2usis64: + case INS_vcvttss2usis32: + case INS_vcvttss2usis64: + { + assert(!id->idIsEvexAaaContextSet()); + srcAttr = EA_16BYTE; + break; + } + + case INS_vpbroadcastb_gpr: + case INS_vpbroadcastd_gpr: + case INS_vpbroadcastw_gpr: + { + srcAttr = EA_4BYTE; + break; + } + + case INS_vpbroadcastq_gpr: + { + srcAttr = EA_8BYTE; + break; + } + + case INS_cvtpd2dq: + case INS_cvtpd2ps: + case INS_cvttpd2dq: + case INS_vcvtdq2ph: + case INS_vcvtneps2bf16: + case INS_vcvtpd2udq: + case INS_vcvtps2phx: + case INS_vcvtqq2ps: + case INS_vcvttpd2dqs: + case INS_vcvttpd2udq: + case INS_vcvttpd2udqs: + case INS_vcvtudq2ph: + case INS_vcvtuqq2ps: + { + tgtAttr = std::max(EA_16BYTE, EA_ATTR(EA_SIZE_IN_BYTES(attr) / 2)); + break; + } + + case INS_vcvtpd2ph: + case INS_vcvtqq2ph: + case INS_vcvtuqq2ph: + { + tgtAttr = std::max(EA_16BYTE, EA_ATTR(EA_SIZE_IN_BYTES(attr) / 4)); + break; + } + + default: + break; + } break; + } } printf("%s", emitRegName(id->idReg1(), tgtAttr)); @@ -13732,7 +13734,7 @@ void emitter::emitDispIns( case INS_vcvtps2ph: { - tgtAttr = static_cast(std::max(16U, static_cast(EA_SIZE(attr)) / 2U)); + tgtAttr = static_cast(std::max(16U, EA_SIZE_IN_BYTES(attr) / 2U)); break; } From 3774779637c6dcc0623138a154da14aebfa65d5a Mon Sep 17 00:00:00 2001 From: Clinton Ingram Date: Sat, 4 Apr 2026 15:23:43 -0700 Subject: [PATCH 4/4] formatting --- src/coreclr/jit/emitxarch.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 7c8ce991c4b61c..bde4d3b41c7eab 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -13411,7 +13411,7 @@ void emitter::emitDispIns( break; } - unsigned inputSize = static_cast(GetInputSizeInBytes(id)); + unsigned inputSize = static_cast(GetInputSizeInBytes(id)); switch (tt) { @@ -13734,7 +13734,7 @@ void emitter::emitDispIns( case INS_vcvtps2ph: { - tgtAttr = static_cast(std::max(16U, EA_SIZE_IN_BYTES(attr) / 2U)); + tgtAttr = std::max(EA_16BYTE, EA_ATTR(EA_SIZE_IN_BYTES(attr) / 2)); break; }