[PATCH v3 12/11] Fix incorrect encoding of relaxed instructions (PR18303)
David Woodhouse
dwmw2 at infradead.org
Sun Jan 5 02:55:23 PST 2014
On Mon, 2013-12-23 at 23:27 +0000, David Woodhouse wrote:
> 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:
> @@ -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
Incremental patch... ?
From e8646d2b6bbaeade2f33888a5aee89e537f49785 Mon Sep 17 00:00:00 2001
From: David Woodhouse <David.Woodhouse at intel.com>
Date: Sun, 5 Jan 2014 10:33:19 +0000
Subject: [PATCH] Make AvailableFeatures argument to EncodeInstruction
mandatory
The fix for PR18303 added a new form of MCCodeEmitter::EncodeInstruction()
which takes an AvailableFeatures argument. Clean that up somewhat by making
the new form mandatory and implementing it in every back end, then providing
an inline version with the old prototype that just calls the real function
with this.getAvailableFeatures() as the new argument.
This addresses the feedback I *anticipated* getting in response to the
original patch, which never actually came. Since I'm fairly clueless about
C++, and especially about the collective taste of the LLVM project, I
figured it was best to actually wait to be told how to do it.
But in the face of resounding silence, I might as well just have a go...
---
include/llvm/MC/MCCodeEmitter.h | 10 +++++-----
lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp | 6 ++++--
lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp | 6 ++++--
lib/Target/Mips/MCTargetDesc/MipsMCCodeEmitter.cpp | 6 ++++--
lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp | 3 ++-
lib/Target/R600/MCTargetDesc/R600MCCodeEmitter.cpp | 6 ++++--
lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp | 6 ++++--
lib/Target/Sparc/MCTargetDesc/SparcMCCodeEmitter.cpp | 6 ++++--
lib/Target/SystemZ/MCTargetDesc/SystemZMCCodeEmitter.cpp | 6 ++++--
lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp | 11 +----------
10 files changed, 36 insertions(+), 30 deletions(-)
diff --git a/include/llvm/MC/MCCodeEmitter.h b/include/llvm/MC/MCCodeEmitter.h
index 1b5bc50..9c041eb 100644
--- a/include/llvm/MC/MCCodeEmitter.h
+++ b/include/llvm/MC/MCCodeEmitter.h
@@ -36,12 +36,12 @@ public:
/// EncodeInstruction - Encode the given \p Inst to bytes on the output
/// 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);
+ uint64_t AvailableFeatures) const = 0;
+
+ inline void EncodeInstruction(const MCInst &Inst, raw_ostream &OS,
+ SmallVectorImpl<MCFixup> &Fixups) {
+ return EncodeInstruction(Inst, OS, Fixups, getAvailableFeatures());
}
/// getAvailableFeatures - Return the current feature set, for use when an
diff --git a/lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp b/lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp
index b41c566..2bb0a78 100644
--- a/lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp
+++ b/lib/Target/AArch64/MCTargetDesc/AArch64MCCodeEmitter.cpp
@@ -121,7 +121,8 @@ public:
void EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const;
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const;
template<int hasRs, int hasRt2> unsigned
fixLoadStoreExclusive(const MCInst &MI, unsigned EncodedValue) const;
@@ -545,7 +546,8 @@ MCCodeEmitter *llvm::createAArch64MCCodeEmitter(const MCInstrInfo &MCII,
void AArch64MCCodeEmitter::
EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const {
if (MI.getOpcode() == AArch64::TLSDESCCALL) {
// This is a directive which applies an R_AARCH64_TLSDESC_CALL to the
// following (BLR) instruction. It doesn't emit any code itself so it
diff --git a/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp b/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
index 4382d0d..03f7556 100644
--- a/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
+++ b/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
@@ -333,7 +333,8 @@ public:
}
void EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const;
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvaiableFeatures) const;
};
} // end anonymous namespace
@@ -1524,7 +1525,8 @@ getShiftRight64Imm(const MCInst &MI, unsigned Op,
void ARMMCCodeEmitter::
EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const {
// Pseudo instructions don't get encoded.
const MCInstrDesc &Desc = MCII.get(MI.getOpcode());
uint64_t TSFlags = Desc.TSFlags;
diff --git a/lib/Target/Mips/MCTargetDesc/MipsMCCodeEmitter.cpp b/lib/Target/Mips/MCTargetDesc/MipsMCCodeEmitter.cpp
index aad777d..9186ca6 100644
--- a/lib/Target/Mips/MCTargetDesc/MipsMCCodeEmitter.cpp
+++ b/lib/Target/Mips/MCTargetDesc/MipsMCCodeEmitter.cpp
@@ -71,7 +71,8 @@ public:
}
void EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const;
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const;
// getBinaryCodeForInstr - TableGen'erated function for getting the
// binary encoding for an instruction.
@@ -215,7 +216,8 @@ static void LowerDextDins(MCInst& InstIn) {
/// Size the instruction with Desc.getSize().
void MipsMCCodeEmitter::
EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const
{
// Non-pseudo instructions that get changed for direct object
diff --git a/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp b/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp
index 66ebfd2..3f0a944 100644
--- a/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp
+++ b/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp
@@ -76,7 +76,8 @@ public:
uint64_t getBinaryCodeForInstr(const MCInst &MI,
SmallVectorImpl<MCFixup> &Fixups) const;
void EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const {
// For fast-isel, a float COPY_TO_REGCLASS can survive this long.
// It's just a nop to keep the register classes happy, so don't
// generate anything.
diff --git a/lib/Target/R600/MCTargetDesc/R600MCCodeEmitter.cpp b/lib/Target/R600/MCTargetDesc/R600MCCodeEmitter.cpp
index dd8df65..873d3ab 100644
--- a/lib/Target/R600/MCTargetDesc/R600MCCodeEmitter.cpp
+++ b/lib/Target/R600/MCTargetDesc/R600MCCodeEmitter.cpp
@@ -44,7 +44,8 @@ public:
/// \brief Encode the instruction and write it to the OS.
virtual void EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const;
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const;
/// \returns the encoding for an MCOperand.
virtual uint64_t getMachineOpValue(const MCInst &MI, const MCOperand &MO,
@@ -87,7 +88,8 @@ MCCodeEmitter *llvm::createR600MCCodeEmitter(const MCInstrInfo &MCII,
}
void R600MCCodeEmitter::EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const {
const MCInstrDesc &Desc = MCII.get(MI.getOpcode());
if (MI.getOpcode() == AMDGPU::RETURN ||
MI.getOpcode() == AMDGPU::FETCH_CLAUSE ||
diff --git a/lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp b/lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp
index 5af8320..b08f8eb 100644
--- a/lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp
+++ b/lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp
@@ -55,7 +55,8 @@ public:
/// \breif Encode the instruction and write it to the OS.
virtual void EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const;
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const;
/// \returns the encoding for an MCOperand.
virtual uint64_t getMachineOpValue(const MCInst &MI, const MCOperand &MO,
@@ -125,7 +126,8 @@ uint32_t SIMCCodeEmitter::getLitEncoding(const MCOperand &MO) const {
}
void SIMCCodeEmitter::EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const {
uint64_t Encoding = getBinaryCodeForInstr(MI, Fixups);
const MCInstrDesc &Desc = MCII.get(MI.getOpcode());
diff --git a/lib/Target/Sparc/MCTargetDesc/SparcMCCodeEmitter.cpp b/lib/Target/Sparc/MCTargetDesc/SparcMCCodeEmitter.cpp
index 641aed4..8493eb8 100644
--- a/lib/Target/Sparc/MCTargetDesc/SparcMCCodeEmitter.cpp
+++ b/lib/Target/Sparc/MCTargetDesc/SparcMCCodeEmitter.cpp
@@ -38,7 +38,8 @@ public:
~SparcMCCodeEmitter() {}
void EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const;
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const;
// getBinaryCodeForInstr - TableGen'erated function for getting the
// binary encoding for an instruction.
@@ -67,7 +68,8 @@ MCCodeEmitter *llvm::createSparcMCCodeEmitter(const MCInstrInfo &MCII,
void SparcMCCodeEmitter::
EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const {
unsigned Bits = getBinaryCodeForInstr(MI, Fixups);
// Output the constant in big endian byte order.
diff --git a/lib/Target/SystemZ/MCTargetDesc/SystemZMCCodeEmitter.cpp b/lib/Target/SystemZ/MCTargetDesc/SystemZMCCodeEmitter.cpp
index f07ea7b..2bbf36d 100644
--- a/lib/Target/SystemZ/MCTargetDesc/SystemZMCCodeEmitter.cpp
+++ b/lib/Target/SystemZ/MCTargetDesc/SystemZMCCodeEmitter.cpp
@@ -35,7 +35,8 @@ public:
// OVerride MCCodeEmitter.
virtual void EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const
LLVM_OVERRIDE;
private:
@@ -91,7 +92,8 @@ MCCodeEmitter *llvm::createSystemZMCCodeEmitter(const MCInstrInfo &MCII,
void SystemZMCCodeEmitter::
EncodeInstruction(const MCInst &MI, raw_ostream &OS,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t AvailableFeatures) const {
uint64_t Bits = getBinaryCodeForInstr(MI, Fixups);
unsigned Size = MCII.get(MI.getOpcode()).getSize();
// Big-endian insertion of Size bytes.
diff --git a/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp b/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
index 4abd115..f31219e 100644
--- a/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
+++ b/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
@@ -153,11 +153,8 @@ public:
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 Features) const;
uint64_t getAvailableFeatures() const;
@@ -1285,12 +1282,6 @@ uint64_t X86MCCodeEmitter::getAvailableFeatures() const {
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();
--
1.8.3.1
--
David Woodhouse Open Source Technology Centre
David.Woodhouse at intel.com Intel Corporation
-------------- 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/20140105/3af02796/attachment.bin>
More information about the llvm-commits
mailing list