[PATCH v3 12/11] Fix incorrect encoding of relaxed instructions (PR18303)
David Woodhouse
dwmw2 at infradead.org
Sat Dec 21 12:22:50 PST 2013
When .code64 or .code32 (or .code16) directives occur in an assembler
file, all relaxed instructions will be encoded in whichever mode was
active at the end of the file. Which is kind of broken.
http://llvm.org/bugs/show_bug.cgi?id=18303
Fix this by storing the active feature set *in* the MCInst, so that it
can be correctly honoured when the instruction is re-encoded after
relaxation.
Storing the feature set is optional; users which don't have a way to
change the feature set will not need to bother as the SubtargetInfo will
always have valid information and the code emitter will use that
instead.
(This should probably go earlier in the sequence before the addition
of .code16. I'll do so if it masses muster.)
diff --git a/include/llvm/MC/MCInst.h b/include/llvm/MC/MCInst.h
index 4766815..de2c6f2 100644
--- a/include/llvm/MC/MCInst.h
+++ b/include/llvm/MC/MCInst.h
@@ -151,8 +151,11 @@ class MCInst {
unsigned Opcode;
SMLoc Loc;
SmallVector<MCOperand, 8> Operands;
+ /// The current set of available features.
+ uint64_t AvailableFeatures;
+ bool AvailableFeaturesSet;
public:
- MCInst() : Opcode(0) {}
+ MCInst() : Opcode(0), AvailableFeatures(0), AvailableFeaturesSet(false) {}
void setOpcode(unsigned Op) { Opcode = Op; }
unsigned getOpcode() const { return Opcode; }
@@ -160,6 +163,13 @@ public:
void setLoc(SMLoc loc) { Loc = loc; }
SMLoc getLoc() const { return Loc; }
+ void setAvailableFeatures(uint64_t features) {
+ AvailableFeatures = features;
+ AvailableFeaturesSet = true;
+ }
+ bool isAvailableFeaturesSet() const { return AvailableFeaturesSet; }
+ uint64_t getAvailableFeatures() const { return AvailableFeatures; }
+
const MCOperand &getOperand(unsigned i) const { return Operands[i]; }
MCOperand &getOperand(unsigned i) { return Operands[i]; }
unsigned getNumOperands() const { return Operands.size(); }
diff --git a/lib/Target/X86/AsmParser/X86AsmParser.cpp b/lib/Target/X86/AsmParser/X86AsmParser.cpp
index 01746fa..0b432bf 100644
--- a/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ b/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -2420,6 +2420,7 @@ MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
Op->getToken() == "finit" || Op->getToken() == "fsave" ||
Op->getToken() == "fstenv" || Op->getToken() == "fclex") {
MCInst Inst;
+ Inst.setAvailableFeatures(STI.getFeatureBits());
Inst.setOpcode(X86::WAIT);
Inst.setLoc(IDLoc);
if (!MatchingInlineAsm)
@@ -2443,6 +2444,7 @@ MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
bool WasOriginallyInvalidOperand = false;
MCInst Inst;
+ Inst.setAvailableFeatures(STI.getFeatureBits());
// First, try a direct match.
switch (MatchInstructionImpl(Operands, Inst,
diff --git a/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp b/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
index 4392de2..84ccf39 100644
--- a/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
+++ b/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
@@ -42,20 +42,23 @@ public:
~X86MCCodeEmitter() {}
- bool is64BitMode() const {
- // FIXME: Can tablegen auto-generate this?
- return (STI.getFeatureBits() & X86::Mode64Bit) != 0;
+ bool is64BitMode(const MCInst &MI) const {
+ uint64_t features = MI.isAvailableFeaturesSet() ?
+ MI.getAvailableFeatures() : STI.getFeatureBits();
+ return (features & X86::Mode64Bit) != 0;
}
- bool is32BitMode() const {
- // FIXME: Can tablegen auto-generate this?
- return (STI.getFeatureBits() & X86::Mode64Bit) == 0 &&
- (STI.getFeatureBits() & X86::Mode16Bit) == 0;
+ bool is32BitMode(const MCInst &MI) const {
+ uint64_t features = MI.isAvailableFeaturesSet() ?
+ MI.getAvailableFeatures() : STI.getFeatureBits();
+ return (features & X86::Mode64Bit) == 0 &&
+ (features & X86::Mode16Bit) == 0;
}
- bool is16BitMode() const {
- // FIXME: Can tablegen auto-generate this?
- return (STI.getFeatureBits() & X86::Mode16Bit) != 0;
+ bool is16BitMode(const MCInst &MI) const {
+ uint64_t features = MI.isAvailableFeaturesSet() ?
+ MI.getAvailableFeatures() : STI.getFeatureBits();
+ return (features & X86::Mode16Bit) != 0;
}
/// Is16BitMemOperand - Return true if the specified instruction has
@@ -65,7 +68,7 @@ public:
const MCOperand &IndexReg = MI.getOperand(Op+X86::AddrIndexReg);
const MCOperand &Disp = MI.getOperand(Op+X86::AddrDisp);
- if (is16BitMode() && BaseReg.getReg() == 0 &&
+ if (is16BitMode(MI) && BaseReg.getReg() == 0 &&
Disp.isImm() && Disp.getImm() < 0x10000)
return true;
if ((BaseReg.getReg() != 0 &&
@@ -387,7 +390,7 @@ void X86MCCodeEmitter::EmitMemModRMByte(const MCInst &MI, unsigned Op,
// Handle %rip relative addressing.
if (BaseReg == X86::RIP) { // [disp32+RIP] in X86-64 mode
- assert(is64BitMode() && "Rip-relative addressing requires 64-bit mode");
+ assert(is64BitMode(MI) && "Rip-relative addressing requires 64-bit mode");
assert(IndexReg.getReg() == 0 && "Invalid rip-relative address");
EmitByte(ModRMByte(0, RegOpcodeField, 5), CurByte, OS);
@@ -486,7 +489,7 @@ void X86MCCodeEmitter::EmitMemModRMByte(const MCInst &MI, unsigned Op,
BaseRegNo != N86::ESP &&
// If there is no base register and we're in 64-bit mode, we need a SIB
// byte to emit an addr that is just 'disp32' (the non-RIP relative form).
- (!is64BitMode() || BaseReg != 0)) {
+ (!is64BitMode(MI) || BaseReg != 0)) {
if (BaseReg == 0) { // [disp32] in X86-32 mode
EmitByte(ModRMByte(0, RegOpcodeField, 5), CurByte, OS);
@@ -1165,18 +1168,18 @@ void X86MCCodeEmitter::EmitOpcodePrefix(uint64_t TSFlags, unsigned &CurByte,
// The AdSize prefix is only for 32-bit and 64-bit modes; in 16-bit mode we
// need the address override only for JECXZ instead. Since it's only one
// instruction, we special-case it rather than introducing an AdSize16 bit.
- if ((!is16BitMode() && TSFlags & X86II::AdSize) ||
- (is16BitMode() && MI.getOpcode() == X86::JECXZ_32)) {
+ if ((!is16BitMode(MI) && TSFlags & X86II::AdSize) ||
+ (is16BitMode(MI) && MI.getOpcode() == X86::JECXZ_32)) {
need_address_override = true;
} else if (MemOperand == -1) {
need_address_override = false;
- } else if (is64BitMode()) {
+ } else if (is64BitMode(MI)) {
assert(!Is16BitMemOperand(MI, MemOperand));
need_address_override = Is32BitMemOperand(MI, MemOperand);
- } else if (is16BitMode()) {
+ } else if (is16BitMode(MI)) {
assert(!Is64BitMemOperand(MI, MemOperand));
need_address_override = !Is16BitMemOperand(MI, MemOperand);
- } else if (is32BitMode()) {
+ } else if (is32BitMode(MI)) {
assert(!Is64BitMemOperand(MI, MemOperand));
need_address_override = Is16BitMemOperand(MI, MemOperand);
} else {
@@ -1187,7 +1190,7 @@ void X86MCCodeEmitter::EmitOpcodePrefix(uint64_t TSFlags, unsigned &CurByte,
EmitByte(0x67, CurByte, OS);
// Emit the operand size opcode prefix as needed.
- if (TSFlags & (is16BitMode() ? X86II::OpSize16 : X86II::OpSize))
+ if (TSFlags & (is16BitMode(MI) ? X86II::OpSize16 : X86II::OpSize))
EmitByte(0x66, CurByte, OS);
bool Need0FPrefix = false;
@@ -1234,7 +1237,7 @@ void X86MCCodeEmitter::EmitOpcodePrefix(uint64_t TSFlags, unsigned &CurByte,
// Handle REX prefix.
// FIXME: Can this come before F2 etc to simplify emission?
- if (is64BitMode()) {
+ if (is64BitMode(MI)) {
if (unsigned REX = DetermineREXPrefix(MI, TSFlags, Desc))
EmitByte(0x40 | REX, CurByte, OS);
}
--
David Woodhouse Open Source Technology Centre
David.Woodhouse at intel.com Intel Corporation
More information about the llvm-commits
mailing list