[PATCH v3 12/11] Fix incorrect encoding of relaxed instructions (PR18303)
David Woodhouse
dwmw2 at infradead.org
Tue Jan 7 05:01:00 PST 2014
On Tue, 2014-01-07 at 11:59 +0000, David Woodhouse wrote:
>
> For the problem I'm looking at, here's the test case for ARM:
>
> // RUN: llvm-mc -filetype=obj -triple thumbv7-linux-gnu %s -o %t
> // RUN: llvm-objdump -triple thumbv7-linux-gnu -d %t | FileCheck %s
>
> //PR18303
> .code 16
> .global edata
> b edata // CHECK: b.w
> .code 32
And here is a *partial* attempt at fixing it. It goes on top of my
generic patch at
http://git.infradead.org/users/dwmw2/llvm.git/commitdiff/87ed55e05 which
keeps the correct features in the MCRelaxableFragment, and is intended
to parallel the fix for X86, which can be seen in
http://git.infradead.org/users/dwmw2/llvm.git/commitdiff/a916969e
However, there's a problem:
lib/Target/ARM/MCTargetDesc/../ARMGenMCCodeEmitter.inc:3392:45: error:
too few arguments to
function call, expected 4, have 3
op = getHiLo16ImmOpValue(MI, 1, Fixups);
~~~~~~~~~~~~~~~~~~~ ^
I need to propagate the Features bits through the *generated* functions
too, which means that if this is the approach we want to take, it'd need
changes to TableGen to make getBinaryCodeForInstr() take the extra
argument, and pass it to all the EncoderMethod and PostEncoderMethod
calls...
This then means touching every back end to make the same changes there,
and expands the scope of the fix somewhat. Is there a better way?
---
lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp | 137 +++++++++++++----------
test/MC/ARM/fixup-cpu-mode.s | 9 ++
2 files changed, 87 insertions(+), 59 deletions(-)
create mode 100644 test/MC/ARM/fixup-cpu-mode.s
diff --git a/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp b/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
index 03f7556..b7de008 100644
--- a/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
+++ b/lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
@@ -49,12 +49,11 @@ public:
~ARMMCCodeEmitter() {}
- bool isThumb() const {
- // FIXME: Can tablegen auto-generate this?
- return (STI.getFeatureBits() & ARM::ModeThumb) != 0;
+ bool isThumb(uint64_t Features) const {
+ return (Features & ARM::ModeThumb) != 0;
}
- bool isThumb2() const {
- return isThumb() && (STI.getFeatureBits() & ARM::FeatureThumb2) != 0;
+ bool isThumb2(uint64_t Features) const {
+ return isThumb(Features) && (Features & ARM::FeatureThumb2) != 0;
}
bool isTargetDarwin() const {
Triple TT(STI.getTargetTriple());
@@ -77,7 +76,8 @@ public:
/// the specified operand. This is used for operands with :lower16: and
/// :upper16: prefixes.
uint32_t getHiLo16ImmOpValue(const MCInst &MI, unsigned OpIdx,
- SmallVectorImpl<MCFixup> &Fixups) const;
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t Features) const;
bool EncodeAddrModeOpValues(const MCInst &MI, unsigned OpIdx,
unsigned &Reg, unsigned &Imm,
@@ -108,7 +108,8 @@ public:
/// getBranchTargetOpValue - Return encoding info for 24-bit immediate
/// branch target.
uint32_t getBranchTargetOpValue(const MCInst &MI, unsigned OpIdx,
- SmallVectorImpl<MCFixup> &Fixups) const;
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t Features) const;
/// getUnconditionalBranchTargetOpValue - Return encoding info for 24-bit
/// immediate Thumb2 direct branch target.
@@ -137,7 +138,8 @@ public:
/// getAddrModeImm12OpValue - Return encoding info for 'reg +/- imm12'
/// operand.
uint32_t getAddrModeImm12OpValue(const MCInst &MI, unsigned OpIdx,
- SmallVectorImpl<MCFixup> &Fixups) const;
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t Features) const;
/// getThumbAddrModeRegRegOpValue - Return encoding for 'reg + reg' operand.
uint32_t getThumbAddrModeRegRegOpValue(const MCInst &MI, unsigned OpIdx,
@@ -225,7 +227,8 @@ public:
/// getAddrMode5OpValue - Return encoding info for 'reg +/- imm8' operand.
uint32_t getAddrMode5OpValue(const MCInst &MI, unsigned OpIdx,
- SmallVectorImpl<MCFixup> &Fixups) const;
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t Features) const;
/// getCCOutOpValue - Return encoding of the 's' bit.
unsigned getCCOutOpValue(const MCInst &MI, unsigned Op,
@@ -309,16 +312,21 @@ public:
SmallVectorImpl<MCFixup> &Fixups) const;
unsigned NEONThumb2DataIPostEncoder(const MCInst &MI,
- unsigned EncodedValue) const;
+ unsigned EncodedValue,
+ uint64_t Features) const;
unsigned NEONThumb2LoadStorePostEncoder(const MCInst &MI,
- unsigned EncodedValue) const;
+ unsigned EncodedValue,
+ uint64_t Features) const;
unsigned NEONThumb2DupPostEncoder(const MCInst &MI,
- unsigned EncodedValue) const;
+ unsigned EncodedValue,
+ uint64_t Features) const;
unsigned NEONThumb2V8PostEncoder(const MCInst &MI,
- unsigned EncodedValue) const;
+ unsigned EncodedValue,
+ uint64_t Features) const;
unsigned VFPThumb2PostEncoder(const MCInst &MI,
- unsigned EncodedValue) const;
+ unsigned EncodedValue,
+ uint64_t Features) const;
void EmitByte(unsigned char C, raw_ostream &OS) const {
OS << (char)C;
@@ -349,9 +357,10 @@ MCCodeEmitter *llvm::createARMMCCodeEmitter(const MCInstrInfo &MCII,
/// NEONThumb2DataIPostEncoder - Post-process encoded NEON data-processing
/// instructions, and rewrite them to their Thumb2 form if we are currently in
/// Thumb2 mode.
-unsigned ARMMCCodeEmitter::NEONThumb2DataIPostEncoder(const MCInst &MI,
- unsigned EncodedValue) const {
- if (isThumb2()) {
+unsigned ARMMCCodeEmitter::
+NEONThumb2DataIPostEncoder(const MCInst &MI, unsigned EncodedValue,
+ uint64_t Features) const {
+ if (isThumb2(Features)) {
// NEON Thumb2 data-processsing encodings are very simple: bit 24 is moved
// to bit 12 of the high half-word (i.e. bit 28), and bits 27-24 are
// set to 1111.
@@ -368,9 +377,10 @@ unsigned ARMMCCodeEmitter::NEONThumb2DataIPostEncoder(const MCInst &MI,
/// NEONThumb2LoadStorePostEncoder - Post-process encoded NEON load/store
/// instructions, and rewrite them to their Thumb2 form if we are currently in
/// Thumb2 mode.
-unsigned ARMMCCodeEmitter::NEONThumb2LoadStorePostEncoder(const MCInst &MI,
- unsigned EncodedValue) const {
- if (isThumb2()) {
+unsigned ARMMCCodeEmitter::
+NEONThumb2LoadStorePostEncoder(const MCInst &MI, unsigned EncodedValue,
+ uint64_t Features) const {
+ if (isThumb2(Features)) {
EncodedValue &= 0xF0FFFFFF;
EncodedValue |= 0x09000000;
}
@@ -381,9 +391,10 @@ unsigned ARMMCCodeEmitter::NEONThumb2LoadStorePostEncoder(const MCInst &MI,
/// NEONThumb2DupPostEncoder - Post-process encoded NEON vdup
/// instructions, and rewrite them to their Thumb2 form if we are currently in
/// Thumb2 mode.
-unsigned ARMMCCodeEmitter::NEONThumb2DupPostEncoder(const MCInst &MI,
- unsigned EncodedValue) const {
- if (isThumb2()) {
+unsigned ARMMCCodeEmitter::
+NEONThumb2DupPostEncoder(const MCInst &MI, unsigned EncodedValue,
+ uint64_t Features) const {
+ if (isThumb2(Features)) {
EncodedValue &= 0x00FFFFFF;
EncodedValue |= 0xEE000000;
}
@@ -393,9 +404,10 @@ unsigned ARMMCCodeEmitter::NEONThumb2DupPostEncoder(const MCInst &MI,
/// Post-process encoded NEON v8 instructions, and rewrite them to Thumb2 form
/// if we are in Thumb2.
-unsigned ARMMCCodeEmitter::NEONThumb2V8PostEncoder(const MCInst &MI,
- unsigned EncodedValue) const {
- if (isThumb2()) {
+unsigned ARMMCCodeEmitter::
+NEONThumb2V8PostEncoder(const MCInst &MI, unsigned EncodedValue,
+ uint64_t Features) const {
+ if (isThumb2(Features)) {
EncodedValue |= 0xC000000; // Set bits 27-26
}
@@ -405,8 +417,9 @@ unsigned ARMMCCodeEmitter::NEONThumb2V8PostEncoder(const MCInst &MI,
/// VFPThumb2PostEncoder - Post-process encoded VFP instructions and rewrite
/// them to their Thumb2 form if we are currently in Thumb2 mode.
unsigned ARMMCCodeEmitter::
-VFPThumb2PostEncoder(const MCInst &MI, unsigned EncodedValue) const {
- if (isThumb2()) {
+VFPThumb2PostEncoder(const MCInst &MI, unsigned EncodedValue,
+ uint64_t Features) const {
+ if (isThumb2(Features)) {
EncodedValue &= 0x0FFFFFFF;
EncodedValue |= 0xE0000000;
}
@@ -583,10 +596,9 @@ static bool HasConditionalBranch(const MCInst &MI) {
/// target.
uint32_t ARMMCCodeEmitter::
getBranchTargetOpValue(const MCInst &MI, unsigned OpIdx,
- SmallVectorImpl<MCFixup> &Fixups) const {
- // FIXME: This really, really shouldn't use TargetMachine. We don't want
- // coupling between MC and TM anywhere we can help it.
- if (isThumb2())
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t Features) const {
+ if (isThumb2(Features))
return
::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_t2_condbranch, Fixups);
return getARMBranchTargetOpValue(MI, OpIdx, Fixups);
@@ -600,10 +612,10 @@ getARMBranchTargetOpValue(const MCInst &MI, unsigned OpIdx,
const MCOperand MO = MI.getOperand(OpIdx);
if (MO.isExpr()) {
if (HasConditionalBranch(MI))
- return ::getBranchTargetOpValue(MI, OpIdx,
- ARM::fixup_arm_condbranch, Fixups);
- return ::getBranchTargetOpValue(MI, OpIdx,
- ARM::fixup_arm_uncondbranch, Fixups);
+ return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_arm_condbranch,
+ Fixups);
+ return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_arm_uncondbranch,
+ Fixups);
}
return MO.getImm() >> 2;
@@ -611,13 +623,14 @@ getARMBranchTargetOpValue(const MCInst &MI, unsigned OpIdx,
uint32_t ARMMCCodeEmitter::
getARMBLTargetOpValue(const MCInst &MI, unsigned OpIdx,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups) const {
const MCOperand MO = MI.getOperand(OpIdx);
if (MO.isExpr()) {
if (HasConditionalBranch(MI))
- return ::getBranchTargetOpValue(MI, OpIdx,
- ARM::fixup_arm_condbl, Fixups);
- return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_arm_uncondbl, Fixups);
+ return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_arm_condbl,
+ Fixups);
+ return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_arm_uncondbl,
+ Fixups);
}
return MO.getImm() >> 2;
@@ -625,10 +638,11 @@ getARMBLTargetOpValue(const MCInst &MI, unsigned OpIdx,
uint32_t ARMMCCodeEmitter::
getARMBLXTargetOpValue(const MCInst &MI, unsigned OpIdx,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups) const {
const MCOperand MO = MI.getOperand(OpIdx);
if (MO.isExpr())
- return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_arm_blx, Fixups);
+ return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_arm_blx,
+ Fixups);
return MO.getImm() >> 1;
}
@@ -637,12 +651,13 @@ getARMBLXTargetOpValue(const MCInst &MI, unsigned OpIdx,
/// immediate branch target.
uint32_t ARMMCCodeEmitter::
getUnconditionalBranchTargetOpValue(const MCInst &MI, unsigned OpIdx,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups) const {
unsigned Val = 0;
const MCOperand MO = MI.getOperand(OpIdx);
if(MO.isExpr())
- return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_t2_uncondbranch, Fixups);
+ return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_t2_uncondbranch,
+ Fixups);
else
Val = MO.getImm() >> 1;
@@ -706,7 +721,7 @@ getAdrLabelOpValue(const MCInst &MI, unsigned OpIdx,
/// target.
uint32_t ARMMCCodeEmitter::
getT2AdrLabelOpValue(const MCInst &MI, unsigned OpIdx,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups) const {
const MCOperand MO = MI.getOperand(OpIdx);
if (MO.isExpr())
return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_t2_adr_pcrel_12,
@@ -725,7 +740,7 @@ getT2AdrLabelOpValue(const MCInst &MI, unsigned OpIdx,
/// target.
uint32_t ARMMCCodeEmitter::
getThumbAdrLabelOpValue(const MCInst &MI, unsigned OpIdx,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups) const {
const MCOperand MO = MI.getOperand(OpIdx);
if (MO.isExpr())
return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_thumb_adr_pcrel_10,
@@ -751,7 +766,8 @@ getThumbAddrModeRegRegOpValue(const MCInst &MI, unsigned OpIdx,
/// getAddrModeImm12OpValue - Return encoding info for 'reg +/- imm12' operand.
uint32_t ARMMCCodeEmitter::
getAddrModeImm12OpValue(const MCInst &MI, unsigned OpIdx,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t Features) const {
// {17-13} = reg
// {12} = (U)nsigned (add == '1', sub == '0')
// {11-0} = imm12
@@ -768,7 +784,7 @@ getAddrModeImm12OpValue(const MCInst &MI, unsigned OpIdx,
isAdd = false ; // 'U' bit is set as part of the fixup.
MCFixupKind Kind;
- if (isThumb2())
+ if (isThumb2(Features))
Kind = MCFixupKind(ARM::fixup_t2_ldst_pcrel_12);
else
Kind = MCFixupKind(ARM::fixup_arm_ldst_pcrel_12);
@@ -898,7 +914,8 @@ static bool EvaluateAsPCRel(const MCExpr *Expr) {
uint32_t
ARMMCCodeEmitter::getHiLo16ImmOpValue(const MCInst &MI, unsigned OpIdx,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t Features) const {
// {20-16} = imm{15-12}
// {11-0} = imm{11-0}
const MCOperand &MO = MI.getOperand(OpIdx);
@@ -917,21 +934,21 @@ ARMMCCodeEmitter::getHiLo16ImmOpValue(const MCInst &MI, unsigned OpIdx,
default: llvm_unreachable("Unsupported ARMFixup");
case ARMMCExpr::VK_ARM_HI16:
if (!isTargetDarwin() && EvaluateAsPCRel(E))
- Kind = MCFixupKind(isThumb2()
+ Kind = MCFixupKind(isThumb2(Features)
? ARM::fixup_t2_movt_hi16_pcrel
: ARM::fixup_arm_movt_hi16_pcrel);
else
- Kind = MCFixupKind(isThumb2()
+ Kind = MCFixupKind(isThumb2(Features)
? ARM::fixup_t2_movt_hi16
: ARM::fixup_arm_movt_hi16);
break;
case ARMMCExpr::VK_ARM_LO16:
if (!isTargetDarwin() && EvaluateAsPCRel(E))
- Kind = MCFixupKind(isThumb2()
+ Kind = MCFixupKind(isThumb2(Features)
? ARM::fixup_t2_movw_lo16_pcrel
: ARM::fixup_arm_movw_lo16_pcrel);
else
- Kind = MCFixupKind(isThumb2()
+ Kind = MCFixupKind(isThumb2(Features)
? ARM::fixup_t2_movw_lo16
: ARM::fixup_arm_movw_lo16);
break;
@@ -944,11 +961,11 @@ ARMMCCodeEmitter::getHiLo16ImmOpValue(const MCInst &MI, unsigned OpIdx,
// the lower 16 bits of the expression regardless of whether
// we have a movt or a movw.
if (!isTargetDarwin() && EvaluateAsPCRel(E))
- Kind = MCFixupKind(isThumb2()
+ Kind = MCFixupKind(isThumb2(Features)
? ARM::fixup_t2_movw_lo16_pcrel
: ARM::fixup_arm_movw_lo16_pcrel);
else
- Kind = MCFixupKind(isThumb2()
+ Kind = MCFixupKind(isThumb2(Features)
? ARM::fixup_t2_movw_lo16
: ARM::fixup_arm_movw_lo16);
Fixups.push_back(MCFixup::Create(0, E, Kind, MI.getLoc()));
@@ -1124,14 +1141,16 @@ getAddrModePCOpValue(const MCInst &MI, unsigned OpIdx,
SmallVectorImpl<MCFixup> &Fixups) const {
const MCOperand MO = MI.getOperand(OpIdx);
if (MO.isExpr())
- return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_arm_thumb_cp, Fixups);
+ return ::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_arm_thumb_cp,
+ Fixups);
return (MO.getImm() >> 2);
}
/// getAddrMode5OpValue - Return encoding info for 'reg +/- imm10' operand.
uint32_t ARMMCCodeEmitter::
getAddrMode5OpValue(const MCInst &MI, unsigned OpIdx,
- SmallVectorImpl<MCFixup> &Fixups) const {
+ SmallVectorImpl<MCFixup> &Fixups,
+ uint64_t Features) const {
// {12-9} = reg
// {8} = (U)nsigned (add == '1', sub == '0')
// {7-0} = imm8
@@ -1147,7 +1166,7 @@ getAddrMode5OpValue(const MCInst &MI, unsigned OpIdx,
assert(MO.isExpr() && "Unexpected machine operand type!");
const MCExpr *Expr = MO.getExpr();
MCFixupKind Kind;
- if (isThumb2())
+ if (isThumb2(Features))
Kind = MCFixupKind(ARM::fixup_t2_pcrel_10);
else
Kind = MCFixupKind(ARM::fixup_arm_pcrel_10);
@@ -1542,7 +1561,7 @@ EncodeInstruction(const MCInst &MI, raw_ostream &OS,
uint32_t Binary = getBinaryCodeForInstr(MI, Fixups);
// Thumb 32-bit wide instructions need to emit the high order halfword
// first.
- if (isThumb() && Size == 4) {
+ if (isThumb(AvailableFeatures) && Size == 4) {
EmitConstant(Binary >> 16, 2, OS);
EmitConstant(Binary & 0xffff, 2, OS);
} else
diff --git a/test/MC/ARM/fixup-cpu-mode.s b/test/MC/ARM/fixup-cpu-mode.s
new file mode 100644
index 0000000..17f29f9
--- /dev/null
+++ b/test/MC/ARM/fixup-cpu-mode.s
@@ -0,0 +1,9 @@
+// RUN: llvm-mc -filetype=obj -triple thumbv7-linux-gnu %s -o %t
+// RUN: llvm-objdump -triple thumbv7-linux-gnu -d %t | FileCheck %s
+
+//PR18303
+.code 16
+.global edata
+b edata // CHECK: b.w
+.code 32
+
--
1.8.4.2
--
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/20140107/a0e07872/attachment.bin>
More information about the llvm-commits
mailing list