Skip to content

Commit

Permalink
[X86] Preserve redundant Address-Size override prefix
Browse files Browse the repository at this point in the history 
Print and emit redundant Address-Size override prefix if it's set on the
instruction.

Reviewed By: skan

Differential Revision: https://reviews.llvm.org/D120592
  • Loading branch information
@aaupov
aaupov committed Mar 16, 2022
1 parent e5b1b9e commit 1d37198
Show file tree
Hide file tree
Showing 10 changed files with 166 additions and 128 deletions.
2 changes: 1 addition & 1 deletion llvm/lib/Target/X86/MCTargetDesc/X86ATTInstPrinter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void X86ATTInstPrinter::printInst(const MCInst *MI, uint64_t Address,
if (CommentStream)
HasCustomInstComment = EmitAnyX86InstComments(MI, *CommentStream, MII);

printInstFlags(MI, OS);
printInstFlags(MI, OS, STI);

// Output CALLpcrel32 as "callq" in 64-bit mode.
// In Intel annotation it's always emitted as "call".
Expand Down
22 changes: 19 additions & 3 deletions llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@
#include "llvm/MC/MCInst.h"
#include "llvm/MC/MCInstrDesc.h"
#include "llvm/MC/MCInstrInfo.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/MC/MCSubtargetInfo.h"
#include "llvm/Support/Casting.h"
#include <cstdint>
#include "llvm/Support/raw_ostream.h"
#include <cassert>
#include <cstdint>

using namespace llvm;

Expand Down Expand Up @@ -349,7 +350,8 @@ void X86InstPrinterCommon::printOptionalSegReg(const MCInst *MI, unsigned OpNo,
}
}

void X86InstPrinterCommon::printInstFlags(const MCInst *MI, raw_ostream &O) {
void X86InstPrinterCommon::printInstFlags(const MCInst *MI, raw_ostream &O,
const MCSubtargetInfo &STI) {
const MCInstrDesc &Desc = MII.get(MI->getOpcode());
uint64_t TSFlags = Desc.TSFlags;
unsigned Flags = MI->getFlags();
Expand Down Expand Up @@ -379,6 +381,20 @@ void X86InstPrinterCommon::printInstFlags(const MCInst *MI, raw_ostream &O) {
O << "\t{disp8}";
else if (Flags & X86::IP_USE_DISP32)
O << "\t{disp32}";

// Determine where the memory operand starts, if present
int MemoryOperand = X86II::getMemoryOperandNo(TSFlags);
if (MemoryOperand != -1)
MemoryOperand += X86II::getOperandBias(Desc);

// Address-Size override prefix
if (Flags & X86::IP_HAS_AD_SIZE &&
!X86_MC::needsAddressSizeOverride(*MI, STI, MemoryOperand, TSFlags)) {
if (STI.hasFeature(X86::Mode16Bit) || STI.hasFeature(X86::Mode64Bit))
O << "\taddr32\t";
else if (STI.hasFeature(X86::Mode32Bit))
O << "\taddr16\t";
}
}

void X86InstPrinterCommon::printVKPair(const MCInst *MI, unsigned OpNo,
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ class X86InstPrinterCommon : public MCInstPrinter {
raw_ostream &O);

protected:
void printInstFlags(const MCInst *MI, raw_ostream &O);
void printInstFlags(const MCInst *MI, raw_ostream &O,
const MCSubtargetInfo &STI);
void printOptionalSegReg(const MCInst *MI, unsigned OpNo, raw_ostream &O);
void printVKPair(const MCInst *MI, unsigned OpNo, raw_ostream &OS);
};
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/X86/MCTargetDesc/X86IntelInstPrinter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ void X86IntelInstPrinter::printRegName(raw_ostream &OS, unsigned RegNo) const {
void X86IntelInstPrinter::printInst(const MCInst *MI, uint64_t Address,
StringRef Annot, const MCSubtargetInfo &STI,
raw_ostream &OS) {
printInstFlags(MI, OS);
printInstFlags(MI, OS, STI);

// In 16-bit mode, print data16 as data32.
if (MI->getOpcode() == X86::DATA16_PREFIX &&
Expand Down
103 changes: 3 additions & 100 deletions llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,65 +156,6 @@ static MCFixupKind getImmFixupKind(uint64_t TSFlags) {
return MCFixup::getKindForSize(Size, isPCRel);
}

/// \param Op operand # of the memory operand.
///
/// \returns true if the specified instruction has a 16-bit memory operand.
static bool is16BitMemOperand(const MCInst &MI, unsigned Op,
const MCSubtargetInfo &STI) {
const MCOperand &Base = MI.getOperand(Op + X86::AddrBaseReg);
const MCOperand &Index = MI.getOperand(Op + X86::AddrIndexReg);

unsigned BaseReg = Base.getReg();
unsigned IndexReg = Index.getReg();

if (STI.hasFeature(X86::Mode16Bit) && BaseReg == 0 && IndexReg == 0)
return true;
if ((BaseReg != 0 &&
X86MCRegisterClasses[X86::GR16RegClassID].contains(BaseReg)) ||
(IndexReg != 0 &&
X86MCRegisterClasses[X86::GR16RegClassID].contains(IndexReg)))
return true;
return false;
}

/// \param Op operand # of the memory operand.
///
/// \returns true if the specified instruction has a 32-bit memory operand.
static bool is32BitMemOperand(const MCInst &MI, unsigned Op) {
const MCOperand &BaseReg = MI.getOperand(Op + X86::AddrBaseReg);
const MCOperand &IndexReg = MI.getOperand(Op + X86::AddrIndexReg);

if ((BaseReg.getReg() != 0 &&
X86MCRegisterClasses[X86::GR32RegClassID].contains(BaseReg.getReg())) ||
(IndexReg.getReg() != 0 &&
X86MCRegisterClasses[X86::GR32RegClassID].contains(IndexReg.getReg())))
return true;
if (BaseReg.getReg() == X86::EIP) {
assert(IndexReg.getReg() == 0 && "Invalid eip-based address.");
return true;
}
if (IndexReg.getReg() == X86::EIZ)
return true;
return false;
}

/// \param Op operand # of the memory operand.
///
/// \returns true if the specified instruction has a 64-bit memory operand.
#ifndef NDEBUG
static bool is64BitMemOperand(const MCInst &MI, unsigned Op) {
const MCOperand &BaseReg = MI.getOperand(Op + X86::AddrBaseReg);
const MCOperand &IndexReg = MI.getOperand(Op + X86::AddrIndexReg);

if ((BaseReg.getReg() != 0 &&
X86MCRegisterClasses[X86::GR64RegClassID].contains(BaseReg.getReg())) ||
(IndexReg.getReg() != 0 &&
X86MCRegisterClasses[X86::GR64RegClassID].contains(IndexReg.getReg())))
return true;
return false;
}
#endif

enum GlobalOffsetTableExprKind { GOT_None, GOT_Normal, GOT_SymDiff };

/// Check if this expression starts with _GLOBAL_OFFSET_TABLE_ and if it is
Expand Down Expand Up @@ -463,7 +404,7 @@ void X86MCCodeEmitter::emitMemModRMByte(const MCInst &MI, unsigned Op,

// 16-bit addressing forms of the ModR/M byte have a different encoding for
// the R/M field and are far more limited in which registers can be used.
if (is16BitMemOperand(MI, Op, STI)) {
if (X86_MC::is16BitMemOperand(MI, Op, STI)) {
if (BaseReg) {
// For 32-bit addressing, the row and column values in Table 2-2 are
// basically the same. It's AX/CX/DX/BX/SP/BP/SI/DI in that order, with
Expand Down Expand Up @@ -672,27 +613,8 @@ bool X86MCCodeEmitter::emitPrefixImpl(unsigned &CurOp, const MCInst &MI,
emitByte(0xF2, OS);

// Emit the address size opcode prefix as needed.
bool NeedAddressOverride;
uint64_t AdSize = TSFlags & X86II::AdSizeMask;
if ((STI.hasFeature(X86::Mode16Bit) && AdSize == X86II::AdSize32) ||
(STI.hasFeature(X86::Mode32Bit) && AdSize == X86II::AdSize16) ||
(STI.hasFeature(X86::Mode64Bit) && AdSize == X86II::AdSize32)) {
NeedAddressOverride = true;
} else if (MemoryOperand < 0) {
NeedAddressOverride = false;
} else if (STI.hasFeature(X86::Mode64Bit)) {
assert(!is16BitMemOperand(MI, MemoryOperand, STI));
NeedAddressOverride = is32BitMemOperand(MI, MemoryOperand);
} else if (STI.hasFeature(X86::Mode32Bit)) {
assert(!is64BitMemOperand(MI, MemoryOperand));
NeedAddressOverride = is16BitMemOperand(MI, MemoryOperand, STI);
} else {
assert(STI.hasFeature(X86::Mode16Bit));
assert(!is64BitMemOperand(MI, MemoryOperand));
NeedAddressOverride = !is16BitMemOperand(MI, MemoryOperand, STI);
}

if (NeedAddressOverride)
if (X86_MC::needsAddressSizeOverride(MI, STI, MemoryOperand, TSFlags) ||
Flags & X86::IP_HAS_AD_SIZE)
emitByte(0x67, OS);

// Encoding type for this instruction.
Expand All @@ -708,39 +630,20 @@ bool X86MCCodeEmitter::emitPrefixImpl(unsigned &CurOp, const MCInst &MI,
default:
break;
case X86II::RawFrmDstSrc: {
unsigned siReg = MI.getOperand(1).getReg();
assert(((siReg == X86::SI && MI.getOperand(0).getReg() == X86::DI) ||
(siReg == X86::ESI && MI.getOperand(0).getReg() == X86::EDI) ||
(siReg == X86::RSI && MI.getOperand(0).getReg() == X86::RDI)) &&
"SI and DI register sizes do not match");
// Emit segment override opcode prefix as needed (not for %ds).
if (MI.getOperand(2).getReg() != X86::DS)
emitSegmentOverridePrefix(2, MI, OS);
// Emit AdSize prefix as needed.
if ((!STI.hasFeature(X86::Mode32Bit) && siReg == X86::ESI) ||
(STI.hasFeature(X86::Mode32Bit) && siReg == X86::SI))
emitByte(0x67, OS);
CurOp += 3; // Consume operands.
break;
}
case X86II::RawFrmSrc: {
unsigned siReg = MI.getOperand(0).getReg();
// Emit segment override opcode prefix as needed (not for %ds).
if (MI.getOperand(1).getReg() != X86::DS)
emitSegmentOverridePrefix(1, MI, OS);
// Emit AdSize prefix as needed.
if ((!STI.hasFeature(X86::Mode32Bit) && siReg == X86::ESI) ||
(STI.hasFeature(X86::Mode32Bit) && siReg == X86::SI))
emitByte(0x67, OS);
CurOp += 2; // Consume operands.
break;
}
case X86II::RawFrmDst: {
unsigned siReg = MI.getOperand(0).getReg();
// Emit AdSize prefix as needed.
if ((!STI.hasFeature(X86::Mode32Bit) && siReg == X86::EDI) ||
(STI.hasFeature(X86::Mode32Bit) && siReg == X86::DI))
emitByte(0x67, OS);
++CurOp; // Consume operand.
break;
}
Expand Down
91 changes: 91 additions & 0 deletions llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,97 @@ bool X86_MC::hasLockPrefix(const MCInst &MI) {
return MI.getFlags() & X86::IP_HAS_LOCK;
}

static bool isMemOperand(const MCInst &MI, unsigned Op, unsigned RegClassID) {
const MCOperand &Base = MI.getOperand(Op + X86::AddrBaseReg);
const MCOperand &Index = MI.getOperand(Op + X86::AddrIndexReg);
const MCRegisterClass &RC = X86MCRegisterClasses[RegClassID];

return (Base.isReg() && Base.getReg() != 0 && RC.contains(Base.getReg())) ||
(Index.isReg() && Index.getReg() != 0 && RC.contains(Index.getReg()));
}

bool X86_MC::is16BitMemOperand(const MCInst &MI, unsigned Op,
const MCSubtargetInfo &STI) {
const MCOperand &Base = MI.getOperand(Op + X86::AddrBaseReg);
const MCOperand &Index = MI.getOperand(Op + X86::AddrIndexReg);

if (STI.hasFeature(X86::Mode16Bit) && Base.isReg() && Base.getReg() == 0 &&
Index.isReg() && Index.getReg() == 0)
return true;
return isMemOperand(MI, Op, X86::GR16RegClassID);
}

bool X86_MC::is32BitMemOperand(const MCInst &MI, unsigned Op) {
const MCOperand &Base = MI.getOperand(Op + X86::AddrBaseReg);
const MCOperand &Index = MI.getOperand(Op + X86::AddrIndexReg);
if (Base.isReg() && Base.getReg() == X86::EIP) {
assert(Index.isReg() && Index.getReg() == 0 && "Invalid eip-based address");
return true;
}
if (Index.isReg() && Index.getReg() == X86::EIZ)
return true;
return isMemOperand(MI, Op, X86::GR32RegClassID);
}

#ifndef NDEBUG
bool X86_MC::is64BitMemOperand(const MCInst &MI, unsigned Op) {
return isMemOperand(MI, Op, X86::GR64RegClassID);
}
#endif

bool X86_MC::needsAddressSizeOverride(const MCInst &MI,
const MCSubtargetInfo &STI,
int MemoryOperand, uint64_t TSFlags) {
uint64_t AdSize = TSFlags & X86II::AdSizeMask;
bool Is16BitMode = STI.hasFeature(X86::Mode16Bit);
bool Is32BitMode = STI.hasFeature(X86::Mode32Bit);
bool Is64BitMode = STI.hasFeature(X86::Mode64Bit);
if ((Is16BitMode && AdSize == X86II::AdSize32) ||
(Is32BitMode && AdSize == X86II::AdSize16) ||
(Is64BitMode && AdSize == X86II::AdSize32))
return true;
uint64_t Form = TSFlags & X86II::FormMask;
switch (Form) {
default:
break;
case X86II::RawFrmDstSrc: {
unsigned siReg = MI.getOperand(1).getReg();
assert(((siReg == X86::SI && MI.getOperand(0).getReg() == X86::DI) ||
(siReg == X86::ESI && MI.getOperand(0).getReg() == X86::EDI) ||
(siReg == X86::RSI && MI.getOperand(0).getReg() == X86::RDI)) &&
"SI and DI register sizes do not match");
return (!Is32BitMode && siReg == X86::ESI) ||
(Is32BitMode && siReg == X86::SI);
}
case X86II::RawFrmSrc: {
unsigned siReg = MI.getOperand(0).getReg();
return (!Is32BitMode && siReg == X86::ESI) ||
(Is32BitMode && siReg == X86::SI);
}
case X86II::RawFrmDst: {
unsigned siReg = MI.getOperand(0).getReg();
return (!Is32BitMode && siReg == X86::EDI) ||
(Is32BitMode && siReg == X86::DI);
}
}

// Determine where the memory operand starts, if present.
if (MemoryOperand < 0)
return false;

if (STI.hasFeature(X86::Mode64Bit)) {
assert(!is16BitMemOperand(MI, MemoryOperand, STI));
return is32BitMemOperand(MI, MemoryOperand);
}
if (STI.hasFeature(X86::Mode32Bit)) {
assert(!is64BitMemOperand(MI, MemoryOperand));
return is16BitMemOperand(MI, MemoryOperand, STI);
}
assert(STI.hasFeature(X86::Mode16Bit));
assert(!is64BitMemOperand(MI, MemoryOperand));
return !is16BitMemOperand(MI, MemoryOperand, STI);
}

void X86_MC::initLLVMToSEHAndCVRegMapping(MCRegisterInfo *MRI) {
// FIXME: TableGen these.
for (unsigned Reg = X86::NoRegister + 1; Reg < X86::NUM_TARGET_REGS; ++Reg) {
Expand Down
22 changes: 22 additions & 0 deletions llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,28 @@ void initLLVMToSEHAndCVRegMapping(MCRegisterInfo *MRI);
/// Returns true if this instruction has a LOCK prefix.
bool hasLockPrefix(const MCInst &MI);

/// \param Op operand # of the memory operand.
///
/// \returns true if the specified instruction has a 16-bit memory operand.
bool is16BitMemOperand(const MCInst &MI, unsigned Op,
const MCSubtargetInfo &STI);

/// \param Op operand # of the memory operand.
///
/// \returns true if the specified instruction has a 32-bit memory operand.
bool is32BitMemOperand(const MCInst &MI, unsigned Op);

/// \param Op operand # of the memory operand.
///
/// \returns true if the specified instruction has a 64-bit memory operand.
#ifndef NDEBUG
bool is64BitMemOperand(const MCInst &MI, unsigned Op);
#endif

/// Returns true if this instruction needs an Address-Size override prefix.
bool needsAddressSizeOverride(const MCInst &MI, const MCSubtargetInfo &STI,
int MemoryOperand, uint64_t TSFlags);

/// Create a X86 MCSubtargetInfo instance. This is exposed so Asm parser, etc.
/// do not need to go through TargetRegistry.
MCSubtargetInfo *createX86MCSubtargetInfo(const Triple &TT, StringRef CPU,
Expand Down
5 changes: 5 additions & 0 deletions llvm/test/MC/Disassembler/X86/addr32.s
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Check that we don't accidentally strip addr32 prefix

# RUN: llvm-mc -disassemble %s -triple=x86_64 | FileCheck %s
# CHECK: addr32 callq
0x67 0xe8 0x00 0x00 0x00 0x00
10 changes: 5 additions & 5 deletions llvm/test/MC/X86/code16gcc.s
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
lodsb (%esi), %al
//CHECK: lodsb (%esi), %al # encoding: [0x67,0xac]
lodsl %gs:(%esi)
//CHECK: lodsl %gs:(%esi), %eax # encoding: [0x66,0x65,0x67,0xad]
//CHECK: lodsl %gs:(%esi), %eax # encoding: [0x67,0x66,0x65,0xad]
lods (%esi), %ax
//CHECK: lodsw (%esi), %ax # encoding: [0x67,0xad]
stosw
//CHECK: stosw %ax, %es:(%edi) # encoding: [0x67,0xab]
stos %eax, (%edi)
//CHECK: stosl %eax, %es:(%edi) # encoding: [0x66,0x67,0xab]
//CHECK: stosl %eax, %es:(%edi) # encoding: [0x67,0x66,0xab]
stosb %al, %es:(%edi)
//CHECK: stosb %al, %es:(%edi) # encoding: [0x67,0xaa]
scas %es:(%edi), %al
Expand All @@ -29,15 +29,15 @@
cmpsw (%edi), (%esi)
//CHECK: cmpsw %es:(%edi), (%esi) # encoding: [0x67,0xa7]
cmpsl %es:(%edi), %ss:(%esi)
//CHECK: cmpsl %es:(%edi), %ss:(%esi) # encoding: [0x66,0x36,0x67,0xa7]
//CHECK: cmpsl %es:(%edi), %ss:(%esi) # encoding: [0x67,0x66,0x36,0xa7]
movsb (%esi), (%edi)
//CHECK: movsb (%esi), %es:(%edi) # encoding: [0x67,0xa4]
movsl %gs:(%esi), (%edi)
//CHECK: movsl %gs:(%esi), %es:(%edi) # encoding: [0x66,0x65,0x67,0xa5]
//CHECK: movsl %gs:(%esi), %es:(%edi) # encoding: [0x67,0x66,0x65,0xa5]
outsb
//CHECK: outsb (%esi), %dx # encoding: [0x67,0x6e]
outsw %fs:(%esi), %dx
//CHECK: outsw %fs:(%esi), %dx # encoding: [0x64,0x67,0x6f]
//CHECK: outsw %fs:(%esi), %dx # encoding: [0x67,0x64,0x6f]
insw %dx, (%di)
//CHECK: insw %dx, %es:(%di) # encoding: [0x6d]
call $0x7ace,$0x7ace
Expand Down
Loading

0 comments on commit 1d37198

Please sign in to comment.