From 92b5a2eb1646b3c1173a5ff3c0073f24ed5ee6a4 Mon Sep 17 00:00:00 2001 From: Bill Wendling Date: Wed, 3 Nov 2010 01:49:29 +0000 Subject: [PATCH] The MC code couldn't handle ARM LDR instructions with negative offsets: vldr.64 d1, [r0, #-32] The problem was with how the addressing mode 5 encodes the offsets. This change makes sure that the way offsets are handled in addressing mode 5 is consistent throughout the MC code. It involves re-refactoring the "getAddrModeImmOpValue" method into an "Imm12" and "addressing mode 5" version. But not to worry! The majority of the duplicated code has been unified. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@118144 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/ARM/ARMCodeEmitter.cpp | 41 +++++++++---- lib/Target/ARM/ARMInstrInfo.td | 20 +++---- lib/Target/ARM/ARMInstrVFP.td | 29 ++++++--- lib/Target/ARM/ARMMCCodeEmitter.cpp | 84 +++++++++++++++++++++------ lib/Target/ARM/AsmParser/ARMAsmParser.cpp | 16 ++++- lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp | 2 +- test/MC/ARM/simple-fp-encoding.s | 21 ++++++- 7 files changed, 160 insertions(+), 53 deletions(-) diff --git a/lib/Target/ARM/ARMCodeEmitter.cpp b/lib/Target/ARM/ARMCodeEmitter.cpp index 0584dec5146..15b3aab5b3a 100644 --- a/lib/Target/ARM/ARMCodeEmitter.cpp +++ b/lib/Target/ARM/ARMCodeEmitter.cpp @@ -177,25 +177,44 @@ namespace { const { return 0; } unsigned getBitfieldInvertedMaskOpValue(const MachineInstr &MI, unsigned Op) const { return 0; } - uint32_t getAddrModeImmOpValue(const MachineInstr &MI, unsigned Op) const { - // {20-17} = reg - // {16} = (U)nsigned (add == '1', sub == '0') - // {15-0} = imm + + unsigned getAddrModeImm12OpValue(const MachineInstr &MI, unsigned Op) + const { + // {17-13} = reg + // {12} = (U)nsigned (add == '1', sub == '0') + // {11-0} = imm12 const MachineOperand &MO = MI.getOperand(Op); const MachineOperand &MO1 = MI.getOperand(Op + 1); if (!MO.isReg()) { emitConstPoolAddress(MO.getIndex(), ARM::reloc_arm_cp_entry); return 0; } - unsigned Reg = getARMRegisterNumbering(MO.getReg()); - int32_t Imm = MO1.getImm(); + int32_t Imm12 = MO1.getImm(); uint32_t Binary; - Binary = Imm & 0xffff; - if (Imm >= 0) - Binary |= (1 << 16); - - Binary |= (Reg << 17); + Binary = Imm12 & 0xfff; + if (Imm12 >= 0) + Binary |= (1 << 12); + Binary |= (Reg << 13); + return Binary; + } + uint32_t getAddrMode5OpValue(const MachineInstr &MI, unsigned Op) const { + // {12-9} = reg + // {8} = (U)nsigned (add == '1', sub == '0') + // {7-0} = imm12 + const MachineOperand &MO = MI.getOperand(Op); + const MachineOperand &MO1 = MI.getOperand(Op + 1); + if (!MO.isReg()) { + emitConstPoolAddress(MO.getIndex(), ARM::reloc_arm_cp_entry); + return 0; + } + unsigned Reg = getARMRegisterNumbering(MO.getReg()); + int32_t Imm8 = MO1.getImm(); + uint32_t Binary; + Binary = Imm8 & 0xff; + if (Imm8 >= 0) + Binary |= (1 << 8); + Binary |= (Reg << 9); return Binary; } unsigned getNEONVcvtImm32OpValue(const MachineInstr &MI, unsigned Op) diff --git a/lib/Target/ARM/ARMInstrInfo.td b/lib/Target/ARM/ARMInstrInfo.td index a44d2c392c6..b3301656d2a 100644 --- a/lib/Target/ARM/ARMInstrInfo.td +++ b/lib/Target/ARM/ARMInstrInfo.td @@ -398,7 +398,7 @@ def addrmode_imm12 : Operand, // #0 and #-0 differently. We flag #-0 as the magic value INT32_MIN. All other // immediate values are as normal. - string EncoderMethod = "getAddrModeImmOpValue"; + string EncoderMethod = "getAddrModeImm12OpValue"; let PrintMethod = "printAddrModeImm12Operand"; let MIOperandInfo = (ops GPR:$base, i32imm:$offsimm); } @@ -462,7 +462,7 @@ def addrmode5 : Operand, let PrintMethod = "printAddrMode5Operand"; let MIOperandInfo = (ops GPR:$base, i32imm); let ParserMatchClass = ARMMemMode5AsmOperand; - string EncoderMethod = "getAddrModeImmOpValue"; + string EncoderMethod = "getAddrMode5OpValue"; } // addrmode6 := reg with optional writeback @@ -828,20 +828,20 @@ multiclass AI_ldr1 { - bits<4> Rt; - bits<32> addr; - let Inst{23} = addr{16}; // U (add = ('U' == 1)) - let Inst{19-16} = addr{20-17}; // Rn + bits<4> Rt; + bits<17> addr; + let Inst{23} = addr{12}; // U (add = ('U' == 1)) + let Inst{19-16} = addr{16-13}; // Rn let Inst{15-12} = Rt; let Inst{11-0} = addr{11-0}; // imm12 } def rs : AIldst1<0b011, opc22, 1, (outs GPR:$Rt), (ins ldst_so_reg:$shift), AddrModeNone, LdFrm, iir, opc, "\t$Rt, $shift", [(set GPR:$Rt, (opnode ldst_so_reg:$shift))]> { - bits<4> Rt; - bits<32> shift; - let Inst{23} = shift{16}; // U (add = ('U' == 1)) - let Inst{19-16} = shift{20-17}; // Rn + bits<4> Rt; + bits<17> shift; + let Inst{23} = shift{12}; // U (add = ('U' == 1)) + let Inst{19-16} = shift{16-13}; // Rn let Inst{11-0} = shift{11-0}; } } diff --git a/lib/Target/ARM/ARMInstrVFP.td b/lib/Target/ARM/ARMInstrVFP.td index 4cf139ea965..65f640ecd1f 100644 --- a/lib/Target/ARM/ARMInstrVFP.td +++ b/lib/Target/ARM/ARMInstrVFP.td @@ -51,25 +51,38 @@ def vfp_f64imm : Operand, // let canFoldAsLoad = 1, isReMaterializable = 1 in { + def VLDRD : ADI5<0b1101, 0b01, (outs DPR:$Dd), (ins addrmode5:$addr), IIC_fpLoad64, "vldr", ".64\t$Dd, $addr", [(set DPR:$Dd, (f64 (load addrmode5:$addr)))]> { // Instruction operands. - bits<5> Dd; - bits<32> addr; + bits<5> Dd; + bits<13> addr; // Encode instruction operands. - let Inst{23} = addr{16}; // U (add = (U == '1')) + let Inst{23} = addr{8}; // U (add = (U == '1')) let Inst{22} = Dd{4}; - let Inst{19-16} = addr{20-17}; // Rn + let Inst{19-16} = addr{12-9}; // Rn let Inst{15-12} = Dd{3-0}; let Inst{7-0} = addr{7-0}; // imm8 } -def VLDRS : ASI5<0b1101, 0b01, (outs SPR:$dst), (ins addrmode5:$addr), - IIC_fpLoad32, "vldr", ".32\t$dst, $addr", - [(set SPR:$dst, (load addrmode5:$addr))]>; -} // canFoldAsLoad +def VLDRS : ASI5<0b1101, 0b01, (outs SPR:$Sd), (ins addrmode5:$addr), + IIC_fpLoad32, "vldr", ".32\t$Sd, $addr", + [(set SPR:$Sd, (load addrmode5:$addr))]> { + // Instruction operands. + bits<5> Sd; + bits<13> addr; + + // Encode instruction operands. + let Inst{23} = addr{8}; // U (add = (U == '1')) + let Inst{22} = Sd{0}; + let Inst{19-16} = addr{12-9}; // Rn + let Inst{15-12} = Sd{4-1}; + let Inst{7-0} = addr{7-0}; // imm8 +} + +} // End of 'let canFoldAsLoad = 1, isReMaterializable = 1 in' def VSTRD : ADI5<0b1101, 0b00, (outs), (ins DPR:$src, addrmode5:$addr), IIC_fpStore64, "vstr", ".64\t$src, $addr", diff --git a/lib/Target/ARM/ARMMCCodeEmitter.cpp b/lib/Target/ARM/ARMMCCodeEmitter.cpp index 6844cc71b73..fcdc02e677b 100644 --- a/lib/Target/ARM/ARMMCCodeEmitter.cpp +++ b/lib/Target/ARM/ARMMCCodeEmitter.cpp @@ -49,8 +49,15 @@ public: /// operand requires relocation, record the relocation and return zero. unsigned getMachineOpValue(const MCInst &MI,const MCOperand &MO) const; - /// getAddrModeImmOpValue - Return encoding info for 'reg +/- imm' operand. - uint32_t getAddrModeImmOpValue(const MCInst &MI, unsigned Op) const; + bool EncodeAddrModeOpValues(const MCInst &MI, unsigned OpIdx, + unsigned &Reg, unsigned &Imm) const; + + /// getAddrModeImm12OpValue - Return encoding info for 'reg +/- imm12' + /// operand. + uint32_t getAddrModeImm12OpValue(const MCInst &MI, unsigned OpIdx) const; + + /// getAddrMode5OpValue - Return encoding info for 'reg +/- imm8' operand. + uint32_t getAddrMode5OpValue(const MCInst &MI, unsigned OpIdx) const; /// getCCOutOpValue - Return encoding of the 's' bit. unsigned getCCOutOpValue(const MCInst &MI, unsigned Op) const { @@ -170,37 +177,76 @@ unsigned ARMMCCodeEmitter::getMachineOpValue(const MCInst &MI, } /// getAddrModeImmOpValue - Return encoding info for 'reg +/- imm' operand. -uint32_t ARMMCCodeEmitter::getAddrModeImmOpValue(const MCInst &MI, - unsigned OpIdx) const { - // {20-17} = reg - // {16} = (U)nsigned (add == '1', sub == '0') - // {15-0} = imm +bool ARMMCCodeEmitter::EncodeAddrModeOpValues(const MCInst &MI, unsigned OpIdx, + unsigned &Reg, + unsigned &Imm) const { const MCOperand &MO = MI.getOperand(OpIdx); const MCOperand &MO1 = MI.getOperand(OpIdx + 1); - uint32_t Binary = 0; // If The first operand isn't a register, we have a label reference. if (!MO.isReg()) { - Binary |= ARM::PC << 17; // Rn is PC. + Reg = ARM::PC; // Rn is PC. + Imm = 0; // FIXME: Add a fixup referencing the label. - return Binary; + return true; } - unsigned Reg = getARMRegisterNumbering(MO.getReg()); - int32_t Imm = MO1.getImm(); - bool isAdd = Imm >= 0; + Reg = getARMRegisterNumbering(MO.getReg()); + + int32_t SImm = MO1.getImm(); + bool isAdd = true; // Special value for #-0 - if (Imm == INT32_MIN) - Imm = 0; + if (SImm == INT32_MIN) + SImm = 0; // Immediate is always encoded as positive. The 'U' bit controls add vs sub. - if (Imm < 0) Imm = -Imm; + if (SImm < 0) { + SImm = -SImm; + isAdd = false; + } + + Imm = SImm; + return isAdd; +} - Binary = Imm & 0xffff; +/// getAddrModeImm12OpValue - Return encoding info for 'reg +/- imm12' operand. +uint32_t ARMMCCodeEmitter::getAddrModeImm12OpValue(const MCInst &MI, + unsigned OpIdx) const { + // {17-13} = reg + // {12} = (U)nsigned (add == '1', sub == '0') + // {11-0} = imm12 + unsigned Reg, Imm12; + bool isAdd = EncodeAddrModeOpValues(MI, OpIdx, Reg, Imm12); + + if (Reg == ARM::PC) + return ARM::PC << 13; // Rn is PC; + + uint32_t Binary = Imm12 & 0xfff; + // Immediate is always encoded as positive. The 'U' bit controls add vs sub. if (isAdd) - Binary |= (1 << 16); - Binary |= (Reg << 17); + Binary |= (1 << 12); + Binary |= (Reg << 13); + return Binary; +} + +/// getAddrMode5OpValue - Return encoding info for 'reg +/- imm12' operand. +uint32_t ARMMCCodeEmitter::getAddrMode5OpValue(const MCInst &MI, + unsigned OpIdx) const { + // {12-9} = reg + // {8} = (U)nsigned (add == '1', sub == '0') + // {7-0} = imm8 + unsigned Reg, Imm8; + EncodeAddrModeOpValues(MI, OpIdx, Reg, Imm8); + + if (Reg == ARM::PC) + return ARM::PC << 13; // Rn is PC; + + uint32_t Binary = ARM_AM::getAM5Offset(Imm8); + // Immediate is always encoded as positive. The 'U' bit controls add vs sub. + if (ARM_AM::getAM5Op(Imm8) == ARM_AM::add) + Binary |= (1 << 8); + Binary |= (Reg << 9); return Binary; } diff --git a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp index 4552374bcc7..f5dc38bcfed 100644 --- a/lib/Target/ARM/AsmParser/ARMAsmParser.cpp +++ b/lib/Target/ARM/AsmParser/ARMAsmParser.cpp @@ -8,6 +8,7 @@ //===----------------------------------------------------------------------===// #include "ARM.h" +#include "ARMAddressingModes.h" #include "ARMSubtarget.h" #include "llvm/MC/MCParser/MCAsmLexer.h" #include "llvm/MC/MCParser/MCAsmParser.h" @@ -260,16 +261,25 @@ public: Inst.addOperand(MCOperand::CreateReg(Mem.BaseRegNum)); assert(!Mem.OffsetIsReg && "invalid mode 5 operand"); + // FIXME: #-0 is encoded differently than #0. Does the parser preserve // the difference? if (Mem.Offset) { const MCConstantExpr *CE = dyn_cast(Mem.Offset); - assert(CE && "non-constant mode 5 offset operand!"); + assert(CE && "Non-constant mode 5 offset operand!"); + // The MCInst offset operand doesn't include the low two bits (like // the instruction encoding). - Inst.addOperand(MCOperand::CreateImm(CE->getValue() / 4)); - } else + int64_t Offset = CE->getValue() / 4; + if (Offset >= 0) + Inst.addOperand(MCOperand::CreateImm(ARM_AM::getAM5Opc(ARM_AM::add, + Offset))); + else + Inst.addOperand(MCOperand::CreateImm(ARM_AM::getAM5Opc(ARM_AM::sub, + -Offset))); + } else { Inst.addOperand(MCOperand::CreateImm(0)); + } } virtual void dump(raw_ostream &OS) const; diff --git a/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp b/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp index 4c233a6184f..790557037c4 100644 --- a/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp +++ b/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp @@ -310,7 +310,7 @@ void ARMInstPrinter::printAddrMode5Operand(const MCInst *MI, unsigned OpNum, if (unsigned ImmOffs = ARM_AM::getAM5Offset(MO2.getImm())) { O << ", #" << ARM_AM::getAddrOpcStr(ARM_AM::getAM5Op(MO2.getImm())) - << ImmOffs*4; + << ImmOffs * 4; } O << "]"; } diff --git a/test/MC/ARM/simple-fp-encoding.s b/test/MC/ARM/simple-fp-encoding.s index cd1ccac0f0a..c3a8ceab33a 100644 --- a/test/MC/ARM/simple-fp-encoding.s +++ b/test/MC/ARM/simple-fp-encoding.s @@ -162,8 +162,9 @@ vldr.64 d17, [r0] @ CHECK: vldr.64 d1, [r2, #32] @ encoding: [0x08,0x1b,0x92,0xed] +@ CHECK: vldr.64 d1, [r2, #-32] @ encoding: [0x08,0x1b,0x12,0xed] vldr.64 d1, [r2, #32] - + vldr.64 d1, [r2, #-32] @ CHECK: vldr.64 d2, [r3] @ encoding: [0x00,0x2b,0x93,0xed] vldr.64 d2, [r3] @@ -174,3 +175,21 @@ vldr.64 d3, [pc] vldr.64 d3, [pc,#0] vldr.64 d3, [pc,#-0] + +@ CHECK: vldr.32 s13, [r0] @ encoding: [0x00,0x6a,0xd0,0xed] + vldr.32 s13, [r0] + +@ CHECK: vldr.32 s1, [r2, #32] @ encoding: [0x08,0x0a,0xd2,0xed] +@ CHECK: vldr.32 s1, [r2, #-32] @ encoding: [0x08,0x0a,0x52,0xed] + vldr.32 s1, [r2, #32] + vldr.32 s1, [r2, #-32] + +@ CHECK: vldr.32 s2, [r3] @ encoding: [0x00,0x1a,0x93,0xed] + vldr.32 s2, [r3] + +@ CHECK: vldr.32 s5, [pc] @ encoding: [0x00,0x2a,0xdf,0xed] +@ CHECK: vldr.32 s5, [pc] @ encoding: [0x00,0x2a,0xdf,0xed] +@ CHECK: vldr.32 s5, [pc] @ encoding: [0x00,0x2a,0xdf,0xed] + vldr.32 s5, [pc] + vldr.32 s5, [pc,#0] + vldr.32 s5, [pc,#-0] -- 2.11.0