Skip to content

Ensure that x64 disassembly names operands at the size actually encoded #131428

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.

Sign up for GitHub

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

Open
tannergooding wants to merge 4 commits into
base: main
Choose a base branch
from
+159 −29
Open

Ensure that x64 disassembly names operands at the size actually encoded #131428

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9920f67
Fix x64 disasm printing 64-bit operands for movzx and GC-typed xor
tannergooding Jul 27, 2026
3d6cd43
Ensure that SIMD disassembly shows the lane selector and correct GPR …
tannergooding Jul 27, 2026
80dd120
Ensure that SIMD disassembly names each operand at its own size
tannergooding Jul 27, 2026
8946af8
Ensure that broadcast disassembly names the source as xmm
tannergooding Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 159 additions & 29 deletions src/coreclr/jit/emitxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12229,6 +12229,10 @@ void emitter::emitDispConstant(const instrDesc* id, bool skipComma) const
case INS_roundps:
case INS_roundsd:
case INS_roundss:
case INS_vextractf32x4:
case INS_vextracti32x4:
case INS_vinsertf32x4:
case INS_vinserti32x4:
{
// These instructions have pseudo-names, but still need to display the immediate
break;
Expand Down Expand Up @@ -12316,6 +12320,53 @@ void emitter::emitDispConstant(const instrDesc* id, bool skipComma) const
emitDispCommentForHandle(cnsVal.cnsVal, id->idDebugOnlyInfo()->idMemCookie, id->idDebugOnlyInfo()->idFlags);
}

//------------------------------------------------------------------------
// GetNarrowedDstAttr: Get the size to display for the destination of an
// instruction that writes fewer bytes than it reads, such as the packed
// double to single converts.
//
// Arguments:
// ins - the instruction
// srcAttr - the size of the source operand
//
// Return Value:
// The size of the destination, or srcAttr when the two match.
//
static emitAttr GetNarrowedDstAttr(instruction ins, emitAttr srcAttr)
{
switch (ins)
{
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:
{
return std::max(EA_16BYTE, EA_ATTR(EA_SIZE_IN_BYTES(srcAttr) / 2));
}

case INS_vcvtpd2ph:
case INS_vcvtqq2ph:
case INS_vcvtuqq2ph:
{
return std::max(EA_16BYTE, EA_ATTR(EA_SIZE_IN_BYTES(srcAttr) / 4));
}

default:
{
return srcAttr;
}
}
}

//--------------------------------------------------------------------
// emitDispIns: Dump the given instruction to jitstdout.
//
Expand Down Expand Up @@ -12448,6 +12499,13 @@ void emitter::emitDispIns(
}
}

// A GC type implies a pointer-sized operand, but the emitter narrows the recorded size when it
// drops REX.W (`xor reg, reg`), so the registers must be named at the size actually encoded.
if (EA_SIZE(attr) != id->idOpSize())
{
attr = id->idOpSize();
}

/* Now see what instruction format we've got */

// First print the implicit register usage
Expand Down Expand Up @@ -12532,16 +12590,25 @@ void emitter::emitDispIns(
}
else
#endif
if (ins == INS_movsx || ins == INS_movzx)
if (ins == INS_movsx)
{
attr = EA_PTRSIZE;
}
else if (ins == INS_movzx)
{
// movzx never encodes REX.W; the 32-bit write already zeroes the upper bits
attr = EA_4BYTE;
}
else if ((ins == INS_crc32) && (attr != EA_8BYTE))
{
// 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`
attr = EA_4BYTE;
}
else
{
attr = GetNarrowedDstAttr(ins, attr);
}
printf("%s", emitRegName(id->idReg1(), attr));
emitDispEmbMasking(id);
printf(", %s", sstr);
Expand Down Expand Up @@ -12800,16 +12867,25 @@ void emitter::emitDispIns(
}
else
#endif
if (ins == INS_movsx || ins == INS_movzx)
if (ins == INS_movsx)
{
attr = EA_PTRSIZE;
}
else if (ins == INS_movzx)
{
// movzx never encodes REX.W; the 32-bit write already zeroes the upper bits
attr = EA_4BYTE;
}
else if ((ins == INS_crc32) && (attr != EA_8BYTE))
{
// 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`
attr = EA_4BYTE;
}
else
{
attr = GetNarrowedDstAttr(ins, attr);
}

printf("%s", emitRegName(id->idReg1(), attr));
emitDispEmbMasking(id);
Expand Down Expand Up @@ -12966,6 +13042,8 @@ void emitter::emitDispIns(
{
switch (ins)
{
case INS_movmskpd:
case INS_movmskps:
case INS_pmovmskb:
{
assert(!id->idIsEvexAaaContextSet());
Expand All @@ -12983,12 +13061,18 @@ void emitter::emitDispIns(
#endif // TARGET_AMD64

case INS_movsx:
case INS_movzx:
{
tgtAttr = EA_PTRSIZE;
break;
}

case INS_movzx:
{
// movzx never encodes REX.W; the 32-bit write already zeroes the upper bits
tgtAttr = EA_4BYTE;
break;
}

case INS_crc32:
{
tgtAttr = std::max(EA_4BYTE, EA_SIZE(attr));
Expand Down Expand Up @@ -13062,6 +13146,8 @@ void emitter::emitDispIns(

default:
{
tgtAttr = GetNarrowedDstAttr(ins, attr);

switch (ins)
{
case INS_cvtsi2ss32:
Expand All @@ -13071,6 +13157,24 @@ void emitter::emitDispIns(
{
assert(!id->idIsEvexAaaContextSet());
tgtAttr = EA_16BYTE;
srcAttr = EA_ATTR(GetInputSizeInBytes(id));
break;
}

case INS_movd32:
case INS_movd64:
{
// Moves between a general purpose register and the low element of a vector
if (isGeneralRegister(id->idReg1()))
{
tgtAttr = EA_ATTR(GetInputSizeInBytes(id));
srcAttr = EA_16BYTE;
}
else
{
tgtAttr = EA_16BYTE;
srcAttr = EA_ATTR(GetInputSizeInBytes(id));
}
break;
}

Expand Down Expand Up @@ -13118,29 +13222,17 @@ void emitter::emitDispIns(
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:
case INS_vbroadcastf32x2:
case INS_vbroadcasti32x2:
case INS_vbroadcastsd:
case INS_vbroadcastss:
case INS_vpbroadcastb:
case INS_vpbroadcastd:
case INS_vpbroadcastq:
case INS_vpbroadcastw:
{
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));
// The source is the low element of a vector, so it is always xmm
srcAttr = EA_16BYTE;
break;
}

Expand Down Expand Up @@ -13177,16 +13269,45 @@ void emitter::emitDispIns(
std::swap(reg2, reg3);
}

emitAttr attr3 = attr;
emitAttr attr12 = attr;
emitAttr attr3 = attr;

if ((insTupleTypeInfo(ins) & INS_TT_MEM128) != 0)
{
// Shift instructions take xmm for the 3rd operand regardless of instruction size.
attr3 = EA_16BYTE;
}
else
{
switch (ins)
{
case INS_cvtsi2sd32:
case INS_cvtsi2sd64:
case INS_cvtsi2ss32:
case INS_cvtsi2ss64:
case INS_vcvtsi2sh32:
case INS_vcvtsi2sh64:
case INS_vcvtusi2sd32:
case INS_vcvtusi2sd64:
case INS_vcvtusi2sh32:
case INS_vcvtusi2sh64:
case INS_vcvtusi2ss32:
case INS_vcvtusi2ss64:
{
// The 3rd operand is the general purpose register being converted
attr12 = EA_16BYTE;
attr3 = EA_ATTR(GetInputSizeInBytes(id));
break;
}

printf("%s", emitRegName(id->idReg1(), attr));
default:
break;
}
}

printf("%s", emitRegName(id->idReg1(), attr12));
emitDispEmbMasking(id);
printf(", %s, ", emitRegName(reg2, attr));
printf(", %s, ", emitRegName(reg2, attr12));
printf("%s", emitRegName(reg3, attr3));
emitDispEmbRounding(id);
break;
Expand Down Expand Up @@ -13377,10 +13498,15 @@ void emitter::emitDispIns(
case IF_RWR_MRD:
case IF_RRW_MRD:
{
if (ins == INS_movsx || ins == INS_movzx)
if (ins == INS_movsx)
{
attr = EA_PTRSIZE;
}
else if (ins == INS_movzx)
{
// movzx never encodes REX.W; the 32-bit write already zeroes the upper bits
attr = EA_4BYTE;
}
#ifdef TARGET_AMD64
else if (ins == INS_movsxd)
{
Expand All @@ -13393,6 +13519,10 @@ void emitter::emitDispIns(
// This logic ensures that we print `crc32 eax, bx` instead of `crc32 ax, bx`
attr = EA_4BYTE;
}
else
{
attr = GetNarrowedDstAttr(ins, attr);
}
printf("%s", emitRegName(id->idReg1(), attr));
emitDispEmbMasking(id);
printf(", %s", sstr);
Expand Down
Loading