[llvm-commits] [PATCH] thumb t2b fix
Tim Northover
t.p.northover at gmail.com
Tue Sep 18 09:04:26 PDT 2012
Hi all,
> Please review and commit this patch.
>
> It fixes the thumb t2b instruction, where the immediate value was being
> incorrectly encoded and decoded.
I can do the committing again, but is there any chance we could have
some non-ARM review on this?
I think it fixes the important codegen errors around the Thumb2 branch
instruction, but there's a change to how fixups are handled which is
larger than we're comfortable with committing blind and that test is
an order of magnitude longer than most (any alternative to .space to
make llvm-mc think a label is PC-relative and far away without
actually generating the intervening gap?)
Cheers.
Tim.
(Attached file is identical to Chris's original).
-------------- next part --------------
Index: test/MC/ARM/thumb-t2b.txt
===================================================================
--- test/MC/ARM/thumb-t2b.txt (revision 0)
+++ test/MC/ARM/thumb-t2b.txt (revision 0)
@@ -0,0 +1,7 @@
+@ RUN: llvm-mc -triple thumbv7 -filetype=obj %s | llvm-objdump -disassemble -triple thumbv7 - | FileCheck %s
+
+b.w _bar
+.zero 12582912
+_bar:
+
+# CHECK: a0 f0 00 90 b.w #12582912
Index: test/MC/ARM/basic-thumb2-instructions.s
===================================================================
--- test/MC/ARM/basic-thumb2-instructions.s (revision 163390)
+++ test/MC/ARM/basic-thumb2-instructions.s (working copy)
@@ -217,12 +217,12 @@
beq.w _bar
bmi.w #-183396
-@ CHECK: b.w _bar @ encoding: [A,0xf0'A',A,0x90'A']
+@ CHECK: b.w _bar @ encoding: [A,0xf0'A',A,0xb8'A']
@ fixup A - offset: 0, value: _bar, kind: fixup_t2_uncondbranch
@ CHECK: beq.w _bar @ encoding: [A,0xf0'A',A,0x80'A']
@ fixup A - offset: 0, value: _bar, kind: fixup_t2_condbranch
@ CHECK: it eq @ encoding: [0x08,0xbf]
-@ CHECK: beq.w _bar @ encoding: [A,0xf0'A',A,0x90'A']
+@ CHECK: beq.w _bar @ encoding: [A,0xf0'A',A,0xb8'A']
@ fixup A - offset: 0, value: _bar, kind: fixup_t2_uncondbranch
@ CHECK: bmi.w #-183396 @ encoding: [0x13,0xf5,0xce,0xa9]
Index: test/MC/Disassembler/ARM/thumb-t2b-reencoding.txt
===================================================================
--- test/MC/Disassembler/ARM/thumb-t2b-reencoding.txt (revision 0)
+++ test/MC/Disassembler/ARM/thumb-t2b-reencoding.txt (revision 0)
@@ -0,0 +1,21 @@
+# RUN: llvm-mc -triple thumbv7 -show-encoding -disassemble < %s | FileCheck %s
+
+0x00 0xf0 0x00 0x90
+0x00 0xf0 0x00 0x98
+0x00 0xf0 0x00 0xb0
+0x00 0xf0 0x00 0xb8
+
+# CHECK: b.w #12582912 @ encoding: [0x00,0xf0,0x00,0x90]
+# CHECK: b.w #8388608 @ encoding: [0x00,0xf0,0x00,0x98]
+# CHECK: b.w #4194304 @ encoding: [0x00,0xf0,0x00,0xb0]
+# CHECK: b.w #0 @ encoding: [0x00,0xf0,0x00,0xb8]
+
+0x00 0xf4 0x00 0x90
+0x00 0xf4 0x00 0x98
+0x00 0xf4 0x00 0xb0
+0x00 0xf4 0x00 0xb8
+
+# CHECK: b.w #-16777216 @ encoding: [0x00,0xf4,0x00,0x90]
+# CHECK: b.w #-12582912 @ encoding: [0x00,0xf4,0x00,0x98]
+# CHECK: b.w #-8388608 @ encoding: [0x00,0xf4,0x00,0xb0]
+# CHECK: b.w #-4194304 @ encoding: [0x00,0xf4,0x00,0xb8]
Index: lib/Target/ARM/ARMInstrThumb2.td
===================================================================
--- lib/Target/ARM/ARMInstrThumb2.td (revision 163390)
+++ lib/Target/ARM/ARMInstrThumb2.td (working copy)
@@ -3243,11 +3243,11 @@
let Inst{15-14} = 0b10;
let Inst{12} = 1;
- bits<20> target;
- let Inst{26} = target{19};
- let Inst{11} = target{18};
- let Inst{13} = target{17};
- let Inst{21-16} = target{16-11};
+ bits<24> target;
+ let Inst{26} = target{23};
+ let Inst{13} = target{22};
+ let Inst{11} = target{21};
+ let Inst{25-16} = target{20-11};
let Inst{10-0} = target{10-0};
let DecoderMethod = "DecodeT2BInstruction";
}
Index: lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp
===================================================================
--- lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp (revision 163390)
+++ lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp (working copy)
@@ -627,6 +627,8 @@
SmallVectorImpl<MCFixup> &Fixups) const {
unsigned Val =
::getBranchTargetOpValue(MI, OpIdx, ARM::fixup_t2_uncondbranch, Fixups);
+ Val >>= 1; // T4 branch immediate value has a '0' appended to the end.
+
bool I = (Val & 0x800000);
bool J1 = (Val & 0x400000);
bool J2 = (Val & 0x200000);
Index: lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
===================================================================
--- lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp (revision 163390)
+++ lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp (working copy)
@@ -247,7 +247,11 @@
}
static unsigned adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
- MCContext *Ctx = NULL) {
+ uint64_t &Clear, MCContext *Ctx = NULL) {
+ // The number of fixups which can clear bits is very small. Default to no
+ // mask.
+ Clear = 0;
+
unsigned Kind = Fixup.getKind();
switch (Kind) {
default:
@@ -361,6 +365,9 @@
Value = Value - 4;
Value >>= 1; // Low bit is not encoded.
+ // Clear all the bits that represent the immediate value.
+ Clear = ~0x9000f000;
+
uint32_t out = 0;
bool I = Value & 0x800000;
bool J1 = Value & 0x400000;
@@ -549,7 +556,8 @@
// Try to get the encoded value for the fixup as-if we're mapping it into
// the instruction. This allows adjustFixupValue() to issue a diagnostic
// if the value aren't invalid.
- (void)adjustFixupValue(Fixup, Value, &Asm.getContext());
+ uint64_t Clear;
+ (void)adjustFixupValue(Fixup, Value, Clear, &Asm.getContext());
}
namespace {
@@ -575,16 +583,19 @@
void ELFARMAsmBackend::applyFixup(const MCFixup &Fixup, char *Data,
unsigned DataSize, uint64_t Value) const {
unsigned NumBytes = 4; // FIXME: 2 for Thumb
- Value = adjustFixupValue(Fixup, Value);
- if (!Value) return; // Doesn't change encoding.
+ uint64_t Clear;
+ Value = adjustFixupValue(Fixup, Value, Clear);
+ if (!Value && !Clear) return; // Doesn't change encoding.
unsigned Offset = Fixup.getOffset();
// For each byte of the fragment that the fixup touches, mask in the bits from
// the fixup value. The Value has been "split up" into the appropriate
// bitfields above.
- for (unsigned i = 0; i != NumBytes; ++i)
+ for (unsigned i = 0; i != NumBytes; ++i) {
+ Data[Offset + i] &= ~uint8_t((Clear >> (i * 8)) & 0xff);
Data[Offset + i] |= uint8_t((Value >> (i * 8)) & 0xff);
+ }
}
// FIXME: This should be in a separate file.
@@ -660,16 +671,19 @@
void DarwinARMAsmBackend::applyFixup(const MCFixup &Fixup, char *Data,
unsigned DataSize, uint64_t Value) const {
unsigned NumBytes = getFixupKindNumBytes(Fixup.getKind());
- Value = adjustFixupValue(Fixup, Value);
- if (!Value) return; // Doesn't change encoding.
+ uint64_t Clear;
+ Value = adjustFixupValue(Fixup, Value, Clear);
+ if (!Value && !Clear) return; // Doesn't change encoding.
unsigned Offset = Fixup.getOffset();
assert(Offset + NumBytes <= DataSize && "Invalid fixup offset!");
// For each byte of the fragment that the fixup touches, mask in the
// bits from the fixup value.
- for (unsigned i = 0; i != NumBytes; ++i)
+ for (unsigned i = 0; i != NumBytes; ++i) {
+ Data[Offset + i] &= ~uint8_t((Value >> (i * 8)) & 0xff);
Data[Offset + i] |= uint8_t((Value >> (i * 8)) & 0xff);
+ }
}
} // end anonymous namespace
Index: lib/Target/ARM/Disassembler/ARMDisassembler.cpp
===================================================================
--- lib/Target/ARM/Disassembler/ARMDisassembler.cpp (revision 163390)
+++ lib/Target/ARM/Disassembler/ARMDisassembler.cpp (working copy)
@@ -2090,14 +2090,17 @@
DecodeT2BInstruction(MCInst &Inst, unsigned Insn,
uint64_t Address, const void *Decoder) {
DecodeStatus S = MCDisassembler::Success;
- unsigned imm = (fieldFromInstruction(Insn, 0, 11) << 0) |
- (fieldFromInstruction(Insn, 11, 1) << 18) |
- (fieldFromInstruction(Insn, 13, 1) << 17) |
- (fieldFromInstruction(Insn, 16, 6) << 11) |
- (fieldFromInstruction(Insn, 26, 1) << 19);
- if (!tryAddingSymbolicOperand(Address, Address + SignExtend32<20>(imm<<1) + 4,
+
+ unsigned s = fieldFromInstruction(Insn, 26, 1);
+ unsigned i1 = !(fieldFromInstruction(Insn, 13, 1) ^ s) ? 1 << 22 : 0;
+ unsigned i2 = !(fieldFromInstruction(Insn, 11, 1) ^ s) ? 1 << 21 : 0;
+ unsigned imm = (fieldFromInstruction(Insn, 0, 11) << 0 ) |
+ (fieldFromInstruction(Insn, 16, 10) << 11) |
+ (s << 23) | i1 | i2;
+
+ if (!tryAddingSymbolicOperand(Address, Address + SignExtend32<25>(imm<<1) + 4,
true, 4, Inst, Decoder))
- Inst.addOperand(MCOperand::CreateImm(SignExtend32<20>(imm << 1)));
+ Inst.addOperand(MCOperand::CreateImm(SignExtend32<25>(imm << 1)));
return S;
}
More information about the llvm-commits
mailing list