[ARM] Support fixup for Thumb2 modified immediate

[peter.smith]

Summary

This change adds a new fixup fixup_t2_so_imm for the t2_so_imm_asmoperand "T2SOImm". The fixup permits code such as:

.L1:
     sub r3, r3, #.L2 - .L1
.L2:

to assemble in Thumb2 as well as in ARM state. There isn't any relocation directive matching the fixup so the expression must be resolved to a constant at assembly time.

The operand predicate isT2SOImm() explicitly doesn't match expressions containing :upper16: and :lower16: as expressions with these operators must match the movt and movw instructions.

The test mov r0, foo2 in thumb2-diagnostics is moved to a new file as the fixup delays the error message till after the assembler has quit due to the other errors. As the mov instruction shares the t2_so_imm_asmoperand mov instructions with a non constant expression now match t2MOVi rather than t2MOVi16 so the error message is slightly different.

Fixes PR28647

[olista01]

test/MC/ARM/t2-modified-immediate-fixup.s

20

These register numbers don't match the input, has something overwritten the wrong part of the instructions?

[peter.smith]

Thanks for the spot, at a glance it looks like I've got the mask for the lowest immediate too large, I think it should be 0xff and not 0xfff. I'll check this afternoon and update.

[peter.smith]

Updated diff to correct immediate mask to 0xff which maps to imm8 in the ARM ARM, previously I had used 0xfff which caused the destination register to get corrupted when the high bits of the immediate were set.

Tests now have correct register numbers. Apologies for my previous blindness, I concentrated on the numbers and missed that the registers had been corrupted.

[olista01]

LGTM

lib/Target/ARM/AsmParser/ARMAsmParser.cpp

bool isAdrLabel() const {
     if (!isImm()) return false;
     const MCConstantExpr *CE = dyn_cast<MCConstantExpr>(getImm());
     if (!CE) return false;
     int64_t Value = CE->getValue();
     return (ARM_AM::getSOImmVal(Value) != -1 ||
             ARM_AM::getSOImmVal(-Value) != -1);
   }
   bool isT2SOImm() const {
+    // If we have an immediate that's not a constant, treat it as an expression
+    // needing a fixup.
+    if (isImm() && !isa<MCConstantExpr>(getImm())) {
+      // We want to avoid matching :upper16: and :lower16: as we want these
+      // expressions to match in isImm0_65535Expr()
+      const ARMMCExpr *ARM16Expr = dyn_cast<ARMMCExpr>(getImm());
+      return (!ARM16Expr || (ARM16Expr->getKind() != ARMMCExpr::VK_ARM_HI16 &&
+                             ARM16Expr->getKind() != ARMMCExpr::VK_ARM_LO16));
+    }
     if (!isImm()) return false;
     const MCConstantExpr *CE = dyn_cast<MCConstantExpr>(getImm());
     if (!CE) return false;
     int64_t Value = CE->getValue();
     return ARM_AM::getT2SOImmVal(Value) != -1;
   }
   bool isT2SOImmNot() const {
     if (!isImm()) return false;

   case ARM::t2ADDri:
   case ARM::t2SUBri: {
     // If the destination and first source operand are the same, and
     // the flags are compatible with the current IT status, use encoding T2
     // instead of T3. For compatibility with the system 'as'. Make sure the
     // wide encoding wasn't explicit.
     if (Inst.getOperand(0).getReg() != Inst.getOperand(1).getReg() ||
         !isARMLowRegister(Inst.getOperand(0).getReg()) ||
-        (unsigned)Inst.getOperand(2).getImm() > 255 ||
+        (Inst.getOperand(2).isImm() &&
+         (unsigned)Inst.getOperand(2).getImm() > 255) ||
         ((!inITBlock() && Inst.getOperand(5).getReg() != ARM::CPSR) ||
          (inITBlock() && Inst.getOperand(5).getReg() != 0)) ||
         (static_cast<ARMOperand &>(*Operands[3]).isToken() &&
          static_cast<ARMOperand &>(*Operands[3]).getToken() == ".w"))
       break;
     MCInst TmpInst;
     TmpInst.setOpcode(Inst.getOpcode() == ARM::t2ADDri ?
                       ARM::tADDi8 : ARM::tSUBi8);
case ARM::tPUSH: {
     Inst.insert(Inst.begin(), MCOperand::createReg(ARM::SP));
     Inst.insert(Inst.begin(), MCOperand::createReg(ARM::SP));
     return true;
   }
   case ARM::t2MOVi: {
     // If we can use the 16-bit encoding and the user didn't explicitly
     // request the 32-bit variant, transform it here.
     if (isARMLowRegister(Inst.getOperand(0).getReg()) &&
-        (unsigned)Inst.getOperand(1).getImm() <= 255 &&
+        (Inst.getOperand(1).isImm() &&
+         (unsigned)Inst.getOperand(1).getImm() <= 255) &&
         ((!inITBlock() && Inst.getOperand(2).getImm() == ARMCC::AL &&
           Inst.getOperand(4).getReg() == ARM::CPSR) ||
          (inITBlock() && Inst.getOperand(4).getReg() == 0)) &&
         (!static_cast<ARMOperand &>(*Operands[2]).isToken() ||
          static_cast<ARMOperand &>(*Operands[2]).getToken() != ".w")) {
       // The operands aren't in the same order for tMOVi8...
       MCInst TmpInst;
       TmpInst.setOpcode(ARM::tMOVi8);

lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp

const static MCFixupKindInfo InfosLE[ARM::NumTargetFixupKinds] = {
       {"fixup_arm_thumb_bcc", 0, 8, MCFixupKindInfo::FKF_IsPCRel},
       // movw / movt: 16-bits immediate but scattered into two chunks 0 - 12, 16
       // - 19.
       {"fixup_arm_movt_hi16", 0, 20, 0},
       {"fixup_arm_movw_lo16", 0, 20, 0},
       {"fixup_t2_movt_hi16", 0, 20, 0},
       {"fixup_t2_movw_lo16", 0, 20, 0},
       {"fixup_arm_mod_imm", 0, 12, 0},
+      {"fixup_t2_so_imm", 0, 26, 0},
   };
   const static MCFixupKindInfo InfosBE[ARM::NumTargetFixupKinds] = {
       // This table *must* be in the order that the fixup_* kinds are defined in
       // ARMFixupKinds.h.
       //
       // Name                      Offset (bits) Size (bits)     Flags
       {"fixup_arm_ldst_pcrel_12", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_t2_ldst_pcrel_12", 0, 32,
const static MCFixupKindInfo InfosBE[ARM::NumTargetFixupKinds] = {
       {"fixup_arm_thumb_bcc", 8, 8, MCFixupKindInfo::FKF_IsPCRel},
       // movw / movt: 16-bits immediate but scattered into two chunks 0 - 12, 16
       // - 19.
       {"fixup_arm_movt_hi16", 12, 20, 0},
       {"fixup_arm_movw_lo16", 12, 20, 0},
       {"fixup_t2_movt_hi16", 12, 20, 0},
       {"fixup_t2_movw_lo16", 12, 20, 0},
       {"fixup_arm_mod_imm", 20, 12, 0},
+      {"fixup_t2_so_imm", 26, 6, 0},
   };
 
   if (Kind < FirstTargetFixupKind)
     return MCAsmBackend::getFixupKindInfo(Kind);
 
   assert(unsigned(Kind - FirstTargetFixupKind) < getNumFixupKinds() &&
          "Invalid kind!");
   return (IsLittleEndian ? InfosLE : InfosBE)[Kind - FirstTargetFixupKind];
unsigned ARMAsmBackend::adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
   }
   case ARM::fixup_arm_mod_imm:
     Value = ARM_AM::getSOImmVal(Value);
     if (Value >> 12) {
       Ctx.reportError(Fixup.getLoc(), "out of range immediate fixup value");
       return 0;
     }
     return Value;
+  case ARM::fixup_t2_so_imm:
+    Value = ARM_AM::getT2SOImmVal(Value);
+    if ((int64_t)Value < 0) {
+      Ctx.reportError(Fixup.getLoc(), "out of range immediate fixup value");
+      return 0;
+    }
+    // Value will contain a 12-bit value broken up into a 4-bit shift in bits
+    // 11:8 and the 8-bit immediate in 0:7. The instruction has the immediate
+    // in 0:7. The 4-bit shift is split up into i:imm3 where i is placed at bit
+    // 10 of the upper half-word and imm3 is placed at 14:12 of the lower
+    // half-word.
+    uint64_t EncValue = 0;
+    EncValue |= (Value & 0x800) << 15;
+    EncValue |= (Value & 0x700) << 4;
+    EncValue |= (Value & 0xfff);
+    return swapHalfWords(EncValue, IsLittleEndian);
   }
 }
 
 void ARMAsmBackend::processFixupValue(const MCAssembler &Asm,
                                       const MCAsmLayout &Layout,
                                       const MCFixup &Fixup,
                                       const MCFragment *DF,
                                       const MCValue &Target, uint64_t &Value,
static unsigned getFixupKindNumBytes(unsigned Kind) {
   case ARM::fixup_t2_pcrel_9:
   case ARM::fixup_t2_adr_pcrel_12:
   case ARM::fixup_arm_thumb_bl:
   case ARM::fixup_arm_thumb_blx:
   case ARM::fixup_arm_movt_hi16:
   case ARM::fixup_arm_movw_lo16:
   case ARM::fixup_t2_movt_hi16:
   case ARM::fixup_t2_movw_lo16:
+  case ARM::fixup_t2_so_imm:
     return 4;
 
   case FK_SecRel_2:
     return 2;
   case FK_SecRel_4:
     return 4;
   }
 }
static unsigned getFixupKindContainerSizeBytes(unsigned Kind) {
   case ARM::fixup_t2_adr_pcrel_12:
   case ARM::fixup_arm_thumb_bl:
   case ARM::fixup_arm_thumb_blx:
   case ARM::fixup_arm_movt_hi16:
   case ARM::fixup_arm_movw_lo16:
   case ARM::fixup_t2_movt_hi16:
   case ARM::fixup_t2_movw_lo16:
   case ARM::fixup_arm_mod_imm:
+  case ARM::fixup_t2_so_imm:
     // Instruction size is 4 bytes.
     return 4;
   }
 }
 
 void ARMAsmBackend::applyFixup(const MCFixup &Fixup, char *Data,
                                unsigned DataSize, uint64_t Value, bool IsPCRel,
                                MCContext &Ctx) const {

lib/Target/ARM/MCTargetDesc/ARMFixupKinds.h

enum Fixups {
   fixup_arm_movt_hi16, // :upper16:
   fixup_arm_movw_lo16, // :lower16:
   fixup_t2_movt_hi16, // :upper16:
   fixup_t2_movw_lo16, // :lower16:
 
   // fixup_arm_mod_imm - Fixup for mod_imm
   fixup_arm_mod_imm,
 
+  // fixup_t2_so_imm - Fixup for Thumb2 8-bit rotated operand
+  fixup_t2_so_imm,
+
   // Marker
   LastTargetFixupKind,
   NumTargetFixupKinds = LastTargetFixupKind - FirstTargetFixupKind
 };
 }
 }
 
 #endif

lib/Target/ARM/MCTargetDesc/ARMMCCodeEmitter.cpp

unsigned getModImmOpValue(const MCInst &MI, unsigned Op,
     // Immediate is already in its encoded format
     return MO.getImm();
   }
 
   /// getT2SOImmOpValue - Return an encoded 12-bit shifted-immediate value.
   unsigned getT2SOImmOpValue(const MCInst &MI, unsigned Op,
                            SmallVectorImpl<MCFixup> &Fixups,
                            const MCSubtargetInfo &STI) const {
-    unsigned SoImm = MI.getOperand(Op).getImm();
+    const MCOperand &MO = MI.getOperand(Op);
+
+    // Support for fixups (MCFixup)
+    if (MO.isExpr()) {
+      const MCExpr *Expr = MO.getExpr();
+      // Fixups resolve to plain values that need to be encoded.
+      MCFixupKind Kind = MCFixupKind(ARM::fixup_t2_so_imm);
+      Fixups.push_back(MCFixup::create(0, Expr, Kind, MI.getLoc()));
+      return 0;
+    }
+    unsigned SoImm = MO.getImm();
     unsigned Encoded =  ARM_AM::getT2SOImmVal(SoImm);
     assert(Encoded != ~0U && "Not a Thumb2 so_imm value?");
     return Encoded;
   }
 
   unsigned getT2AddrModeSORegOpValue(const MCInst &MI, unsigned OpNum,
     SmallVectorImpl<MCFixup> &Fixups,
     const MCSubtargetInfo &STI) const;

test/MC/ARM/big-endian-thumb2-fixup.s

 
 @ARM::fixup_t2_adr_pcrel_12
 .section s_adr_pcrel_12,"ax",%progbits
   adr.w r0, adr_pcrel_12_label
   nop
   nop
 adr_pcrel_12_label:
 
+@ARM::fixup_t2_so_imm
+.section s_t2_so_imm,"ax",%progbits
+// CHECK-LABEL: Contents of section s_t2_so_imm
+// CHECK: 0000 f1033337
+  add r3, r3,val
+.equ val,0x37373737

test/MC/ARM/t2-modified-immediate-fixup-error1.s

+@ PR28647
+@ RUN: not llvm-mc -triple=thumbv7a-linux-gnueabi -filetype=obj < %s 2>&1 | FileCheck %s
+    .text
+    .syntax unified
+    .balign 2
+
+@ Error with unencodeable immediate
+    add r1, r2, sym0
+@ CHECK: error: out of range immediate fixup value
+    .equ sym0, 0x01abcdef
+.L2:
+    mov r0, .L2
+@ CHECK: error: unsupported relocation on symbol

test/MC/ARM/t2-modified-immediate-fixup-error2.s

+@ PR28647
+@ RUN: not llvm-mc -triple=thumbv7a-linux-gnueabi -filetype=obj < %s 2>&1 | FileCheck %s
+    .text
+    .syntax unified
+    .balign 2
+
+@ mov with :upper16: or :lower16: should not match mov with modified immediate
+    mov r0, :upper16: sym0
+@ CHECK: error: instruction requires: arm-mode
+    mov r0, :lower16: sym0
+@ CHECK: error: instruction requires: arm-mode
+    .equ sym0, 0x01abcdef

test/MC/ARM/t2-modified-immediate-fixup.s

+@ PR28647
+@ RUN: llvm-mc < %s -triple=thumbv7a-linux-gnueabi -filetype=obj -o - \
+@ RUN: | llvm-objdump --disassemble -triple=thumbv7a-linux-gnueabi - | FileCheck %s
+    .text
+    .syntax unified
+    .balign 2
+@ Thumb2 modified immediate instructions
+    add r1,r1, sym0
+    sub r1,r2, sym1
+    cmp r2,    sym2
+    and r4,r4, sym3
+    orr r8,r9, sym4
+    teq r1,    sym5
+    tst r1,    sym6
+    sbc r1,r1, sym7
+    adc r1,r0, sym8
+@CHECK: add.w   r1, r1, #255
+@CHECK: sub.w   r1, r2, #16711935
+@CHECK: cmp.w   r2, #4278255360
+@CHECK: and     r7, r4, #303174162
+@CHECK: orr     r12, r9, #2852126720
+@CHECK: teq.w   r1, #1426063360
+@CHECK: tst.w   r1, #713031680
+@CHECK: sbc     r9, r1, #2785280
+@CHECK: adc     pc, r0, #340
+
+.L1:
+    sub r3, r3, #.L2 - .L1
+.L2:
+@CHECK: sub.w   r3, r3, #4
+
+@ mov without :upper16: or :lower16: should match mov with modified immediate
+     mov r1, sym3
+@CHECK: mov.w   r3, #303174162
+
+@ Modified immediate constants
+    .equ sym0, 0x000000ff
+    .equ sym1, 0x00ff00ff
+    .equ sym2, 0xff00ff00
+    .equ sym3, 0x12121212
+    .equ sym4, 0xaa000000
+    .equ sym5, 0x55000000
+    .equ sym6, 0x2a800000
+    .equ sym7, 0x002a8000
+    .equ sym8, 0x00000154

test/MC/ARM/thumb2-diagnostics.s

 @ CHECK-ERRORS: error: branch target out of range
 @ CHECK-ERRORS: error: branch target out of range
 @ CHECK-ERRORS: error: branch target out of range
 @ CHECK-ERRORS: error: branch target out of range
 @ CHECK-ERRORS: error: branch target out of range
 @ CHECK-ERRORS: error: branch target out of range
 
 foo2:
-        mov r0, foo2
         movw r0, foo2
         movt r0, foo2
-@ CHECK-ERRORS: error: instruction requires: arm-mode
 @ CHECK-ERRORS: error: immediate expression for mov requires :lower16: or :upper16
 @ CHECK-ERRORS:                  ^
 @ CHECK-ERRORS: error: immediate expression for mov requires :lower16: or :upper16
 @ CHECK-ERRORS:                  ^
 
         and sp, r1, #80008000
         and pc, r1, #80008000
 @ CHECK-ERRORS: error: invalid operand for instruction