[PATCH v3 12/11] Fix incorrect encoding of relaxed instructions (PR18303)
David Woodhouse
dwmw2 at infradead.org
Mon Dec 23 15:27:50 PST 2013
On Sun, 2013-12-22 at 00:21 +0000, David Woodhouse wrote:
>
> Jörg requested on IRC that I make the feature set mandatory for any
> target that elects to use it, rather than falling back to using the
> SubtargetInfo if no override has been set in the MCInst.
>
> Here's what that would look like...
I didn't like this; it seems to make it much more cumbersome. So I came
up with an alternative version which stores the available feature bits
in the MCRelaxableFragment instead — since I think that's the only place
it *matters* and we can't just use the current bits from the
SubtargetInfo.
This is what's now in http://git.infradead.org/users/dwmw2/llvm.git as
one of the two bug fixes that come before the bulk of the .code16 work:
---
include/llvm/MC/MCAssembler.h | 6 +++
include/llvm/MC/MCCodeEmitter.h | 11 +++++
lib/MC/MCAssembler.cpp | 3 +-
lib/MC/MCObjectStreamer.cpp | 1 +
lib/MC/MCPureStreamer.cpp | 1 +
lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp | 54 ++++++++++++++++--------
test/MC/X86/fixup-cpu-mode.s | 8 ++++
7 files changed, 66 insertions(+), 18 deletions(-)
create mode 100644 test/MC/X86/fixup-cpu-mode.s
diff --git a/include/llvm/MC/MCAssembler.h b/include/llvm/MC/MCAssembler.h
index 8735a55..44a6f87 100644
--- a/include/llvm/MC/MCAssembler.h
+++ b/include/llvm/MC/MCAssembler.h
@@ -294,6 +294,9 @@ class MCRelaxableFragment : public MCEncodedFragmentWithFixups {
/// Fixups - The list of fixups in this fragment.
SmallVector<MCFixup, 1> Fixups;
+ /// EmitterFeatures - The feature set for MCCodeEmitter to use.
+ uint64_t EmitterFeatures;
+
public:
MCRelaxableFragment(const MCInst &_Inst, MCSectionData *SD = 0)
: MCEncodedFragmentWithFixups(FT_Relaxable, SD), Inst(_Inst) {
@@ -305,6 +308,9 @@ public:
const MCInst &getInst() const { return Inst; }
void setInst(const MCInst& Value) { Inst = Value; }
+ uint64_t getEmitterFeatures() const { return EmitterFeatures; }
+ void setEmitterFeatures(uint64_t features) { EmitterFeatures = features; }
+
SmallVectorImpl<MCFixup> &getFixups() {
return Fixups;
}
diff --git a/include/llvm/MC/MCCodeEmitter.h b/include/llvm/MC/MCCodeEmitter.h
index 9bfa08e..1b5bc50 100644
--- a/include/llvm/MC/MCCodeEmitter.h
+++ b/include/llvm/MC/MCCodeEmitter.h
@@ -11,6 +11,7 @@
#define LLVM_MC_MCCODEEMITTER_H
#include "llvm/Support/Compiler.h"
+#include "llvm/Support/DataTypes.h"
namespace llvm {
class MCFixup;
@@ -36,6 +37,16 @@ public:
/// stream \p OS.
virtual void EncodeInstruction(const MCInst &Inst, raw_ostream &OS,
SmallVectorImpl<MCFixup> &Fixups) const = 0;
+
+ virtual void EncodeInstruction(const MCInst &Inst, raw_ostream &OS,
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const {
+ return EncodeInstruction(Inst, OS, Fixups);
+ }
+
+ /// getAvailableFeatures - Return the current feature set, for use when an
+ /// instruction is re-encoded later due to relaxation.
+ virtual uint64_t getAvailableFeatures() const { return 0; }
};
} // End llvm namespace
diff --git a/lib/MC/MCAssembler.cpp b/lib/MC/MCAssembler.cpp
index 68111f1..85ba214 100644
--- a/lib/MC/MCAssembler.cpp
+++ b/lib/MC/MCAssembler.cpp
@@ -875,7 +875,7 @@ bool MCAssembler::relaxInstruction(MCAsmLayout &Layout,
SmallVector<MCFixup, 4> Fixups;
SmallString<256> Code;
raw_svector_ostream VecOS(Code);
- getEmitter().EncodeInstruction(Relaxed, VecOS, Fixups);
+ getEmitter().EncodeInstruction(Relaxed, VecOS, Fixups, F.getEmitterFeatures());
VecOS.flush();
// Update the fragment.
@@ -1093,6 +1093,7 @@ void MCFragment::dump() {
OS << "\n ";
OS << " Inst:";
F->getInst().dump_pretty(OS);
+ OS << " Features:" << F->getEmitterFeatures();
break;
}
case MCFragment::FT_Org: {
diff --git a/lib/MC/MCObjectStreamer.cpp b/lib/MC/MCObjectStreamer.cpp
index bc14c2a..71ebc98 100644
--- a/lib/MC/MCObjectStreamer.cpp
+++ b/lib/MC/MCObjectStreamer.cpp
@@ -242,6 +242,7 @@ void MCObjectStreamer::EmitInstToFragment(const MCInst &Inst) {
getAssembler().getEmitter().EncodeInstruction(Inst, VecOS, IF->getFixups());
VecOS.flush();
IF->getContents().append(Code.begin(), Code.end());
+ IF->setEmitterFeatures(getAssembler().getEmitter().getAvailableFeatures());
}
#ifndef NDEBUG
diff --git a/lib/MC/MCPureStreamer.cpp b/lib/MC/MCPureStreamer.cpp
index f7bf002..d60872f 100644
--- a/lib/MC/MCPureStreamer.cpp
+++ b/lib/MC/MCPureStreamer.cpp
@@ -204,6 +204,7 @@ void MCPureStreamer::EmitInstToFragment(const MCInst &Inst) {
IF->getContents() = Code;
IF->getFixups() = Fixups;
+ IF->setEmitterFeatures(getAssembler().getEmitter().getAvailableFeatures());
}
void MCPureStreamer::EmitInstToData(const MCInst &Inst) {
diff --git a/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp b/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
index 0e48a35..962f124 100644
--- a/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
+++ b/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
@@ -42,14 +42,14 @@ public:
~X86MCCodeEmitter() {}
- bool is64BitMode() const {
+ bool is64BitMode(uint64_t Features) const {
// FIXME: Can tablegen auto-generate this?
- return (STI.getFeatureBits() & X86::Mode64Bit) != 0;
+ return (Features & X86::Mode64Bit) != 0;
}
- bool is32BitMode() const {
+ bool is32BitMode(uint64_t Features) const {
// FIXME: Can tablegen auto-generate this?
- return (STI.getFeatureBits() & X86::Mode64Bit) == 0;
+ return (Features & X86::Mode64Bit) == 0;
}
unsigned GetX86RegNum(const MCOperand &MO) const {
@@ -126,11 +126,18 @@ public:
void EmitMemModRMByte(const MCInst &MI, unsigned Op,
unsigned RegOpcodeField,
uint64_t TSFlags, unsigned &CurByte, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const;
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t Features) const;
void EncodeInstruction(const MCInst &MI, raw_ostream &OS,
SmallVectorImpl<MCFixup> &Fixups) const;
+ void EncodeInstruction(const MCInst &MI, raw_ostream &OS,
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const;
+
+ uint64_t getAvailableFeatures() const;
+
void EmitVEXOpcodePrefix(uint64_t TSFlags, unsigned &CurByte, int MemOperand,
const MCInst &MI, const MCInstrDesc &Desc,
raw_ostream &OS) const;
@@ -141,7 +148,7 @@ public:
void EmitOpcodePrefix(uint64_t TSFlags, unsigned &CurByte, int MemOperand,
const MCInst &MI, const MCInstrDesc &Desc,
- raw_ostream &OS) const;
+ raw_ostream &OS, uint64_t Features) const;
};
} // end anonymous namespace
@@ -366,7 +373,8 @@ void X86MCCodeEmitter::EmitMemModRMByte(const MCInst &MI, unsigned Op,
unsigned RegOpcodeField,
uint64_t TSFlags, unsigned &CurByte,
raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const{
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t Features) const{
const MCOperand &Disp = MI.getOperand(Op+X86::AddrDisp);
const MCOperand &Base = MI.getOperand(Op+X86::AddrBaseReg);
const MCOperand &Scale = MI.getOperand(Op+X86::AddrScaleAmt);
@@ -376,7 +384,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(Features) && "Rip-relative addressing requires 64-bit mode");
assert(IndexReg.getReg() == 0 && "Invalid rip-relative address");
EmitByte(ModRMByte(0, RegOpcodeField, 5), CurByte, OS);
@@ -475,7 +483,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(Features) || BaseReg != 0)) {
if (BaseReg == 0) { // [disp32] in X86-32 mode
EmitByte(ModRMByte(0, RegOpcodeField, 5), CurByte, OS);
@@ -1136,7 +1144,8 @@ void X86MCCodeEmitter::EmitSegmentOverridePrefix(uint64_t TSFlags,
void X86MCCodeEmitter::EmitOpcodePrefix(uint64_t TSFlags, unsigned &CurByte,
int MemOperand, const MCInst &MI,
const MCInstrDesc &Desc,
- raw_ostream &OS) const {
+ raw_ostream &OS,
+ uint64_t Features) const {
// Emit the lock opcode prefix as needed.
if (TSFlags & X86II::LOCK)
@@ -1155,10 +1164,10 @@ void X86MCCodeEmitter::EmitOpcodePrefix(uint64_t TSFlags, unsigned &CurByte,
need_address_override = true;
} else if (MemOperand == -1) {
need_address_override = false;
- } else if (is64BitMode()) {
+ } else if (is64BitMode(Features)) {
assert(!Is16BitMemOperand(MI, MemOperand));
need_address_override = Is32BitMemOperand(MI, MemOperand);
- } else if (is32BitMode()) {
+ } else if (is32BitMode(Features)) {
assert(!Is64BitMemOperand(MI, MemOperand));
need_address_override = Is16BitMemOperand(MI, MemOperand);
} else {
@@ -1216,7 +1225,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(Features)) {
if (unsigned REX = DetermineREXPrefix(MI, TSFlags, Desc))
EmitByte(0x40 | REX, CurByte, OS);
}
@@ -1245,9 +1254,20 @@ void X86MCCodeEmitter::EmitOpcodePrefix(uint64_t TSFlags, unsigned &CurByte,
}
}
+uint64_t X86MCCodeEmitter::getAvailableFeatures() const {
+ return STI.getFeatureBits();
+}
+
void X86MCCodeEmitter::
EncodeInstruction(const MCInst &MI, raw_ostream &OS,
SmallVectorImpl<MCFixup> &Fixups) const {
+ return EncodeInstruction(MI, OS, Fixups, STI.getFeatureBits());
+}
+
+void X86MCCodeEmitter::
+EncodeInstruction(const MCInst &MI, raw_ostream &OS,
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t Features) const {
unsigned Opcode = MI.getOpcode();
const MCInstrDesc &Desc = MCII.get(Opcode);
uint64_t TSFlags = Desc.TSFlags;
@@ -1280,7 +1300,7 @@ EncodeInstruction(const MCInst &MI, raw_ostream &OS,
if (MemoryOperand != -1) MemoryOperand += CurOp;
if (!HasVEXPrefix)
- EmitOpcodePrefix(TSFlags, CurByte, MemoryOperand, MI, Desc, OS);
+ EmitOpcodePrefix(TSFlags, CurByte, MemoryOperand, MI, Desc, OS, Features);
else
EmitVEXOpcodePrefix(TSFlags, CurByte, MemoryOperand, MI, Desc, OS);
@@ -1348,7 +1368,7 @@ EncodeInstruction(const MCInst &MI, raw_ostream &OS,
EmitMemModRMByte(MI, CurOp,
GetX86RegNum(MI.getOperand(SrcRegNum)),
- TSFlags, CurByte, OS, Fixups);
+ TSFlags, CurByte, OS, Fixups, Features);
CurOp = SrcRegNum + 1;
break;
@@ -1393,7 +1413,7 @@ EncodeInstruction(const MCInst &MI, raw_ostream &OS,
EmitByte(BaseOpcode, CurByte, OS);
EmitMemModRMByte(MI, FirstMemOp, GetX86RegNum(MI.getOperand(CurOp)),
- TSFlags, CurByte, OS, Fixups);
+ TSFlags, CurByte, OS, Fixups, Features);
CurOp += AddrOperands + 1;
if (HasVEX_4VOp3)
++CurOp;
@@ -1419,7 +1439,7 @@ EncodeInstruction(const MCInst &MI, raw_ostream &OS,
++CurOp;
EmitByte(BaseOpcode, CurByte, OS);
EmitMemModRMByte(MI, CurOp, (TSFlags & X86II::FormMask)-X86II::MRM0m,
- TSFlags, CurByte, OS, Fixups);
+ TSFlags, CurByte, OS, Fixups, Features);
CurOp += X86::AddrNumOperands;
break;
case X86II::MRM_C1: case X86II::MRM_C2: case X86II::MRM_C3:
diff --git a/test/MC/X86/fixup-cpu-mode.s b/test/MC/X86/fixup-cpu-mode.s
new file mode 100644
index 0000000..13e0d46
--- /dev/null
+++ b/test/MC/X86/fixup-cpu-mode.s
@@ -0,0 +1,8 @@
+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o %t
+// RUN: llvm-objdump -d %t | FileCheck %s
+
+//PR18303
+.global edata
+sub $edata, %r12 // CHECK: subq $0, %r12
+.code32
+
--
1.8.3.1
--
dwmw2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5745 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131223/3e55c52c/attachment.bin>
More information about the llvm-commits
mailing list