Visibly fail if attempting to encode register AH,BH,CH,DH in a REX-prefixed instruction.

[dougk]

Summary

Ideally this could be caught in the asm parser, but that would essentially require that the parser ask the emitter if there is a valid encoding.
So the next best thing is to fail in the emitter rather than encode something that is not the instruction the user wrote.

Fixes PR25003

[craig.topper]

lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp

76

The HasRex argument to this function isn't used by the body.

1138

I feel like you can just detect the error inside of DetermineREXPrefix rather than thread the EmittedREX everywhere. It examines every operand already to calculate the REX prefix bits. While examining every operand for REX can it also see if any are high registers and at the end if the REX is non-zero and any of the registers was a high register throw the error there?

[craig.topper]

lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp

1138

In fact you could probably just detect it while that function looks for SPL, BPL, SIL, or DIL.

[dougk]

Simpler fix per Craig Topper

[craig.topper]

LGTM

lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp

if ((BaseReg.getReg() != 0 &&
          X86MCRegisterClasses[X86::GR16RegClassID].contains(BaseReg.getReg())) ||
         (IndexReg.getReg() != 0 &&
          X86MCRegisterClasses[X86::GR16RegClassID].contains(IndexReg.getReg())))
       return true;
     return false;
   }
 
   unsigned GetX86RegNum(const MCOperand &MO) const {
     return Ctx.getRegisterInfo()->getEncodingValue(MO.getReg()) & 0x7;
   }
 
   // On regular x86, both XMM0-XMM7 and XMM8-XMM15 are encoded in the range
   // 0-7 and the difference between the 2 groups is given by the REX prefix.
   // In the VEX prefix, registers are seen sequencially from 0-15 and encoded
   // in 1's complement form, example:
   //
   //  ModRM field => XMM9 => 1

 }
 
 /// DetermineREXPrefix - Determine if the MCInst has to be encoded with a X86-64
 /// REX prefix which specifies 1) 64-bit instructions, 2) non-default operand
 /// size, and 3) use of X86-64 extended registers.
 static unsigned DetermineREXPrefix(const MCInst &MI, uint64_t TSFlags,
                                    const MCInstrDesc &Desc) {
   unsigned REX = 0;
+  bool UsesHighByteReg = false;
+
   if (TSFlags & X86II::REX_W)
     REX |= 1 << 3; // set REX.W
 
   if (MI.getNumOperands() == 0) return REX;
 
   unsigned NumOps = MI.getNumOperands();
   // FIXME: MCInst should explicitize the two-addrness.
   bool isTwoAddr = NumOps > 1 &&
                       Desc.getOperandConstraint(1, MCOI::TIED_TO) != -1;
 
   // If it accesses SPL, BPL, SIL, or DIL, then it requires a 0x40 REX prefix.
   unsigned i = isTwoAddr ? 1 : 0;
   for (; i != NumOps; ++i) {
     const MCOperand &MO = MI.getOperand(i);
     if (!MO.isReg()) continue;
     unsigned Reg = MO.getReg();
+    if (Reg == X86::AH || Reg == X86::BH || Reg == X86::CH || Reg == X86::DH)
+      UsesHighByteReg = true;
     if (!X86II::isX86_64NonExtLowByteReg(Reg)) continue;
     // FIXME: The caller of DetermineREXPrefix slaps this prefix onto anything
     // that returns non-zero.
     REX |= 0x40; // REX fixed encoding prefix
     break;
   }
 
   switch (TSFlags & X86II::FormMask) {
default:
     i = isTwoAddr ? 2 : 1;
     for (unsigned e = NumOps; i != e; ++i) {
       const MCOperand &MO = MI.getOperand(i);
       if (MO.isReg() && X86II::isX86_64ExtendedReg(MO.getReg()))
         REX |= 1 << 2; // set REX.R
     }
     break;
   }
+  if (REX && UsesHighByteReg)
+    report_fatal_error("Cannot encode high byte register in REX-prefixed instruction");
+
   return REX;
 }
 
 /// EmitSegmentOverridePrefix - Emit segment override opcode prefix as needed
 void X86MCCodeEmitter::EmitSegmentOverridePrefix(unsigned &CurByte,
                                                  unsigned SegOperand,
                                                  const MCInst &MI,
                                                  raw_ostream &OS) const {
void X86MCCodeEmitter::EmitOpcodePrefix(uint64_t TSFlags, unsigned &CurByte,
   case X86II::XD:   // F2
     EmitByte(0xF2, CurByte, OS);
     break;
   }
 
   // Handle REX prefix.
   // FIXME: Can this come before F2 etc to simplify emission?
   if (is64BitMode(STI)) {
     if (unsigned REX = DetermineREXPrefix(MI, TSFlags, Desc))
       EmitByte(0x40 | REX, CurByte, OS);
   }
 
   // 0x0F escape code must be emitted just before the opcode.
   switch (TSFlags & X86II::OpMapMask) {
   case X86II::TB:  // Two-byte opcode map
   case X86II::T8:  // 0F 38
   case X86II::TA:  // 0F 3A

test/MC/X86/encoder-fail.s

+// RUN: not llvm-mc -triple x86_64-unknown-unknown --show-encoding %s 2>&1 | FileCheck %s
+// CHECK: LLVM ERROR: Cannot encode high byte register in REX-prefixed instruction
+ movzx %dh, %rsi