Skip to content

[X86][MC][AsmParser] Reject H-byte regs with VEX/EVEX-encoded 8-bit RR (NDD) #160039

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

Merged
merged 7 commits into from
Sep 25, 2025
Merged

[X86][MC][AsmParser] Reject H-byte regs with VEX/EVEX-encoded 8-bit RR (NDD) #160039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e1d2d25
[X86][MC][AsmParser] Reject H-byte regs with VEX/EVEX-encoded 8-bit R…
woruyu Sep 22, 2025
c6821f1
fix: details
woruyu Sep 22, 2025
6ce586e
fix: review
woruyu Sep 22, 2025
189ccb4
fix: review
woruyu Sep 24, 2025
59d2ffa
fix: review
woruyu Sep 24, 2025
a39aae5
fix: review
woruyu Sep 24, 2025
130d292
fix: combine all check
woruyu Sep 25, 2025
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
19 changes: 13 additions & 6 deletions llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4018,9 +4018,14 @@ bool X86AsmParser::validateInstruction(MCInst &Inst, const OperandVector &Ops) {
return Error(Ops[0]->getStartLoc(), "all tmm registers must be distinct");
}

// Check that we aren't mixing AH/BH/CH/DH with REX prefix. We only need to
// check this with the legacy encoding, VEX/EVEX/XOP don't use REX.
if ((TSFlags & X86II::EncodingMask) == 0) {
// High 8-bit regs (AH/BH/CH/DH) are incompatible with encodings that imply
// extended prefixes:
// * Legacy path that would emit a REX (e.g. uses r8..r15 or sil/dil/bpl/spl)
// * EVEX
// * REX2
// VEX/XOP don't use REX; they are excluded from the legacy check.
const unsigned Enc = TSFlags & X86II::EncodingMask;
if (Enc != X86II::VEX && Enc != X86II::XOP) {
MCRegister HReg;
bool UsesRex = TSFlags & X86II::REX_W;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems an incomplete fix. In addition to REX_W, we may use REX prefix when other bit is set, e.g. REX_R/X/B.

Copy link
Contributor

@phoebewang phoebewang Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://godbolt.org/z/98xdvEbn3

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't understand this. Because I think REX_R/X/B means you use r8..r15 in opreand, so in current codes for add ah, byte ptr [r8], you will get UsesRex = true,HReg is noZero, so you will get error msg in the second if branch.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I mean we don't need extra change here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I missed the line 4041

unsigned NumOps = Inst.getNumOperands();
Expand All @@ -4036,11 +4041,13 @@ bool X86AsmParser::validateInstruction(MCInst &Inst, const OperandVector &Ops) {
UsesRex = true;
}

if (UsesRex && HReg) {
if (HReg &&
(Enc == X86II::EVEX || ForcedOpcodePrefix == OpcodePrefix_REX2 ||
ForcedOpcodePrefix == OpcodePrefix_REX || UsesRex)) {
StringRef RegName = X86IntelInstPrinter::getRegisterName(HReg);
return Error(Ops[0]->getStartLoc(),
"can't encode '" + RegName + "' in an instruction requiring "
"REX prefix");
"can't encode '" + RegName.str() +
"' in an instruction requiring EVEX/REX2/REX prefix");
}
}

Expand Down
32 changes: 27 additions & 5 deletions llvm/test/MC/X86/encoder-fail.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,38 @@
// RUN: not llvm-mc -triple x86_64-unknown-unknown --show-encoding %s 2>&1 | FileCheck %s
// RUN: not llvm-mc -triple x86_64-unknown-unknown --show-encoding -x86-asm-syntax=intel %s 2>&1 | FileCheck %s --check-prefix=CHECK-INTEL

// CHECK: error: can't encode 'dh' in an instruction requiring REX prefix
// CHECK: error: can't encode 'dh' in an instruction requiring EVEX/REX2/REX prefix
movzx %dh, %rsi

// CHECK: error: can't encode 'ah' in an instruction requiring REX prefix
// CHECK: error: can't encode 'ah' in an instruction requiring EVEX/REX2/REX prefix
movzx %ah, %r8d

// CHECK: error: can't encode 'bh' in an instruction requiring REX prefix
// CHECK: error: can't encode 'bh' in an instruction requiring EVEX/REX2/REX prefix
add %bh, %sil

// CHECK: error: can't encode 'ch' in an instruction requiring REX prefix
// CHECK: error: can't encode 'ch' in an instruction requiring EVEX/REX2/REX prefix
mov %ch, (%r8)

// CHECK: error: can't encode 'dh' in an instruction requiring REX prefix
// CHECK: error: can't encode 'dh' in an instruction requiring EVEX/REX2/REX prefix
mov %dh, (%rax,%r8)

// CHECK-INTEL: error: can't encode 'ah' in an instruction requiring EVEX/REX2/REX prefix
add ah, ah, ah

// CHECK-INTEL: error: can't encode 'ah' in an instruction requiring EVEX/REX2/REX prefix
and ah, byte ptr [-13426159], ah

// CHECK-INTEL: error: can't encode 'ah' in an instruction requiring EVEX/REX2/REX prefix
ccmpa {dfv=of,cf} byte ptr [r8 + 4*rax + 291], ah

// CHECK-INTEL: error: can't encode 'ah' in an instruction requiring EVEX/REX2/REX prefix
ccmpae {dfv=of,cf} byte ptr [r8 + 4*rax + 291], ah

// CHECK-INTEL: error: can't encode 'ah' in an instruction requiring EVEX/REX2/REX prefix
sar ah, byte ptr [-13426159]

// CHECK-INTEL: error: can't encode 'ah' in an instruction requiring EVEX/REX2/REX prefix
{rex2} add ah, al

// CHECK-INTEL: error: can't encode 'ah' in an instruction requiring EVEX/REX2/REX prefix
{rex} add ah, al
Loading