[ARM] LSL #0 is an alias of MOV

[john.brawn]

Summary

Currently we handle this correctly in arm, but in thumb we don't which leads to an unpredictable instruction being emitted for LSL #0 in an IT block and SP not being permitted in some cases when it should be.

For the thumb2 LSL we can handle this by making LSL #0 an alias of MOV in the .td file, but for thumb1 we need to handle it in checkTargetMatchPredicate to get the IT handling right. We also need to adjust the handling of MOV rd, rn, LSL #0 to avoid generating the 16-bit encoding in an IT block. We should also adjust it to allow SP in the same way that it is allowed in MOV rd, rn, but I haven't done that here because it looks like it would take quite a lot of work to get right.

Additionally correct the selection of the 16-bit shift instructions in processInstruction, where it was checking if the two registers were equal when it should have been checking if they were low. It appears that previously this code was never executed and the 16-bit encoding was selected by default, but the other changes I've done here have somehow made it start being used.

[rengolin]

LGTM, thanks!

llvm/trunk/lib/Target/ARM/ARMInstrThumb2.td

 def : T2Pat<(int_arm_usat GPR:$a, imm0_31:$pos), (t2USAT imm0_31:$pos, GPR:$a, 0)>;
 def : T2Pat<(ARMssatnoshift GPRnopc:$Rn, imm0_31:$imm),
              (t2SSAT imm0_31:$imm, GPRnopc:$Rn, 0)>;
 
 //===----------------------------------------------------------------------===//
 //  Shift and rotate Instructions.
 //
 
-defm t2LSL  : T2I_sh_ir<0b00, "lsl", imm0_31, shl>;
+defm t2LSL  : T2I_sh_ir<0b00, "lsl", imm1_31, shl>;
 defm t2LSR  : T2I_sh_ir<0b01, "lsr", imm_sr,  srl>;
 defm t2ASR  : T2I_sh_ir<0b10, "asr", imm_sr,  sra>;
 defm t2ROR  : T2I_sh_ir<0b11, "ror", imm0_31, rotr>;
 
+// LSL #0 is actually MOV, and has slightly different permitted registers to
+// LSL with non-zero shift
+def : t2InstAlias<"lsl${s}${p} $Rd, $Rm, #0",
+                  (t2MOVr GPRnopc:$Rd, GPRnopc:$Rm, pred:$p, cc_out:$s)>;
+def : t2InstAlias<"lsl${s}${p}.w $Rd, $Rm, #0",
+                  (t2MOVr GPRnopc:$Rd, GPRnopc:$Rm, pred:$p, cc_out:$s)>;
+
 // (rotr x, (and y, 0x...1f)) ==> (ROR x, y)
 def : T2Pat<(rotr rGPR:$lhs, (and rGPR:$rhs, lo5AllOne)),
             (t2RORrr rGPR:$lhs, rGPR:$rhs)>;
 
 let Uses = [CPSR] in {
 def t2RRX : T2sTwoReg<(outs rGPR:$Rd), (ins rGPR:$Rm), IIC_iMOVsi,
                    "rrx", "\t$Rd, $Rm",
                    [(set rGPR:$Rd, (ARMrrx rGPR:$Rm))]>, Sched<[WriteALU]> {
def : t2InstAlias<"mul${p} $Rn, $Rm",
                   (t2MUL rGPR:$Rn, rGPR:$Rm, rGPR:$Rn, pred:$p)>;
 
 // "neg" is and alias for "rsb rd, rn, #0"
 def : t2InstAlias<"neg${s}${p} $Rd, $Rm",
                   (t2RSBri rGPR:$Rd, rGPR:$Rm, 0, pred:$p, cc_out:$s)>;
 
 // MOV so_reg assembler pseudos. InstAlias isn't expressive enough for
 // these, unfortunately.
+// FIXME: LSL #0 in the shift should allow SP to be used as either the
+// source or destination (but not both).
 def t2MOVsi: t2AsmPseudo<"mov${p} $Rd, $shift",
                          (ins rGPR:$Rd, t2_so_reg:$shift, pred:$p)>;
 def t2MOVSsi: t2AsmPseudo<"movs${p} $Rd, $shift",
                           (ins rGPR:$Rd, t2_so_reg:$shift, pred:$p)>;
 
 def t2MOVsr: t2AsmPseudo<"mov${p} $Rd, $shift",
                          (ins rGPR:$Rd, so_reg_reg:$shift, pred:$p)>;
 def t2MOVSsr: t2AsmPseudo<"movs${p} $Rd, $shift",

llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp

case ARM::VST4qWB_register_Asm_32: {
     return true;
   }
 
   // Handle encoding choice for the shift-immediate instructions.
   case ARM::t2LSLri:
   case ARM::t2LSRri:
   case ARM::t2ASRri: {
     if (isARMLowRegister(Inst.getOperand(0).getReg()) &&
-        Inst.getOperand(0).getReg() == Inst.getOperand(1).getReg() &&
+        isARMLowRegister(Inst.getOperand(1).getReg()) &&
         Inst.getOperand(5).getReg() == (inITBlock() ? 0 : ARM::CPSR) &&
         !(static_cast<ARMOperand &>(*Operands[3]).isToken() &&
           static_cast<ARMOperand &>(*Operands[3]).getToken() == ".w")) {
       unsigned NewOpc;
       switch (Inst.getOpcode()) {
       default: llvm_unreachable("unexpected opcode");
       case ARM::t2LSLri: NewOpc = ARM::tLSLri; break;
       case ARM::t2LSRri: NewOpc = ARM::tLSRri; break;
case ARM::t2MOVSsi: {
     // registers.
     bool isNarrow = false;
     if (isARMLowRegister(Inst.getOperand(0).getReg()) &&
         isARMLowRegister(Inst.getOperand(1).getReg()) &&
         inITBlock() == (Inst.getOpcode() == ARM::t2MOVsi))
       isNarrow = true;
     MCInst TmpInst;
     unsigned newOpc;
-    switch(ARM_AM::getSORegShOp(Inst.getOperand(2).getImm())) {
+    unsigned Shift = ARM_AM::getSORegShOp(Inst.getOperand(2).getImm());
+    unsigned Amount = ARM_AM::getSORegOffset(Inst.getOperand(2).getImm());
+    bool isMov = false;
+    // MOV rd, rm, LSL #0 is actually a MOV instruction
+    if (Shift == ARM_AM::lsl && Amount == 0) {
+      isMov = true;
+      // The 16-bit encoding of MOV rd, rm, LSL #N is explicitly encoding T2 of
+      // MOV (register) in the ARMv8-A and ARMv8-M manuals, and immediate 0 is
+      // unpredictable in an IT block so the 32-bit encoding T3 has to be used
+      // instead.
+      if (inITBlock()) {
+        isNarrow = false;
+      }
+      newOpc = isNarrow ? ARM::tMOVSr : ARM::t2MOVr;
+    } else {
+      switch(Shift) {
       default: llvm_unreachable("unexpected opcode!");
       case ARM_AM::asr: newOpc = isNarrow ? ARM::tASRri : ARM::t2ASRri; break;
       case ARM_AM::lsr: newOpc = isNarrow ? ARM::tLSRri : ARM::t2LSRri; break;
       case ARM_AM::lsl: newOpc = isNarrow ? ARM::tLSLri : ARM::t2LSLri; break;
       case ARM_AM::ror: newOpc = ARM::t2RORri; isNarrow = false; break;
       case ARM_AM::rrx: isNarrow = false; newOpc = ARM::t2RRX; break;
       }
-    unsigned Amount = ARM_AM::getSORegOffset(Inst.getOperand(2).getImm());
+    }
     if (Amount == 32) Amount = 0;
     TmpInst.setOpcode(newOpc);
     TmpInst.addOperand(Inst.getOperand(0)); // Rd
-    if (isNarrow)
+    if (isNarrow && !isMov)
       TmpInst.addOperand(MCOperand::createReg(
           Inst.getOpcode() == ARM::t2MOVSsi ? ARM::CPSR : 0));
     TmpInst.addOperand(Inst.getOperand(1)); // Rn
-    if (newOpc != ARM::t2RRX)
+    if (newOpc != ARM::t2RRX && !isMov)
       TmpInst.addOperand(MCOperand::createImm(Amount));
     TmpInst.addOperand(Inst.getOperand(3)); // CondCode
     TmpInst.addOperand(Inst.getOperand(4));
     if (!isNarrow)
       TmpInst.addOperand(MCOperand::createReg(
           Inst.getOpcode() == ARM::t2MOVSsi ? ARM::CPSR : 0));
     Inst = TmpInst;
     return true;
if (MCID.TSFlags & ARMII::ThumbArithFlagSetting) {
     // If we're parsing Thumb2, which form is legal depends on whether we're
     // in an IT block.
     if (isThumbTwo() && Inst.getOperand(OpNo).getReg() != ARM::CPSR &&
         !inITBlock())
       return Match_RequiresITBlock;
     if (isThumbTwo() && Inst.getOperand(OpNo).getReg() == ARM::CPSR &&
         inITBlock())
       return Match_RequiresNotITBlock;
+    // LSL with zero immediate is not allowed in an IT block
+    if (Opc == ARM::tLSLri && Inst.getOperand(4).getImm() == 0 && inITBlock())
+      return Match_RequiresNotITBlock;
   } else if (isThumbOne()) {
     // Some high-register supporting Thumb1 encodings only allow both registers
     // to be from r0-r7 when in Thumb2.
     if (Opc == ARM::tADDhirr && !hasV6MOps() &&
         isARMLowRegister(Inst.getOperand(1).getReg()) &&
         isARMLowRegister(Inst.getOperand(2).getReg()))
       return Match_RequiresThumb2;
     // Others only require ARMv6 or later.

llvm/trunk/test/MC/ARM/basic-thumb2-instructions.s

@------------------------------------------------------------------------------
         asr r2, r3, #4
         asrs r2, r12, #15
 
         asr r3, #19
         asrs r8, #2
         asrs.w r7, #5
         asr.w r12, #21
 
+        asrs  r1, r2, #1
+        itt eq
+        asrseq r1, r2, #1
+        asreq r1, r2, #1
+
 @ CHECK: asr.w  r2, r3, #12             @ encoding: [0x4f,0xea,0x23,0x32]
 @ CHECK: asrs.w  r8, r3, #32             @ encoding: [0x5f,0xea,0x23,0x08]
 @ CHECK: asrs.w  r2, r3, #1              @ encoding: [0x5f,0xea,0x63,0x02]
 @ CHECK: asr.w  r2, r3, #4              @ encoding: [0x4f,0xea,0x23,0x12]
 @ CHECK: asrs.w  r2, r12, #15            @ encoding: [0x5f,0xea,0xec,0x32]
 
 @ CHECK: asr.w  r3, r3, #19             @ encoding: [0x4f,0xea,0xe3,0x43]
 @ CHECK: asrs.w  r8, r8, #2              @ encoding: [0x5f,0xea,0xa8,0x08]
 @ CHECK: asrs.w  r7, r7, #5              @ encoding: [0x5f,0xea,0x67,0x17]
 @ CHECK: asr.w  r12, r12, #21           @ encoding: [0x4f,0xea,0x6c,0x5c]
 
+@ CHECK: asrs   r1, r2, #1              @ encoding: [0x51,0x10]
+@ CHECK: itt    eq                      @ encoding: [0x04,0xbf]
+@ CHECK: asrseq.w r1, r2, #1            @ encoding: [0x5f,0xea,0x62,0x01]
+@ CHECK: asreq  r1, r2, #1              @ encoding: [0x51,0x10]
 
 @------------------------------------------------------------------------------
 @ ASR (register)
 @------------------------------------------------------------------------------
         asr r3, r4, r2
         asr.w r1, r2
         asrs r3, r4, r8
 
@------------------------------------------------------------------------------
         lsl r2, r3, #4
         lsls r2, r12, #15
 
         lsl r3, #19
         lsls r8, #2
         lsls.w r7, #5
         lsl.w r12, #21
 
+        lsls r1, r2, #1
+        itt eq
+        lslseq r1, r2, #1
+        lsleq r1, r2, #1
+
 @ CHECK: lsl.w  r2, r3, #12             @ encoding: [0x4f,0xea,0x03,0x32]
 @ CHECK: lsls.w  r8, r3, #31             @ encoding: [0x5f,0xea,0xc3,0x78]
 @ CHECK: lsls.w  r2, r3, #1              @ encoding: [0x5f,0xea,0x43,0x02]
 @ CHECK: lsl.w  r2, r3, #4              @ encoding: [0x4f,0xea,0x03,0x12]
 @ CHECK: lsls.w  r2, r12, #15            @ encoding: [0x5f,0xea,0xcc,0x32]
 
 @ CHECK: lsl.w  r3, r3, #19             @ encoding: [0x4f,0xea,0xc3,0x43]
 @ CHECK: lsls.w  r8, r8, #2              @ encoding: [0x5f,0xea,0x88,0x08]
 @ CHECK: lsls.w  r7, r7, #5              @ encoding: [0x5f,0xea,0x47,0x17]
 @ CHECK: lsl.w  r12, r12, #21           @ encoding: [0x4f,0xea,0x4c,0x5c]
 
+@ CHECK: lsls   r1, r2, #1              @ encoding: [0x51,0x00]
+@ CHECK: itt eq                         @ encoding: [0x04,0xbf]
+@ CHECK: lslseq.w r1, r2, #1            @ encoding: [0x5f,0xea,0x42,0x01]
+@ CHECK: lsleq  r1, r2, #1              @ encoding: [0x51,0x00]
 
 @------------------------------------------------------------------------------
 @ LSL (register)
 @------------------------------------------------------------------------------
         lsl r3, r4, r2
         lsl.w r1, r2
         lsls r3, r4, r8
 
@------------------------------------------------------------------------------
         lsr r2, r3, #4
         lsrs r2, r12, #15
 
         lsr r3, #19
         lsrs r8, #2
         lsrs.w r7, #5
         lsr.w r12, #21
 
+        lsrs  r1, r2, #1
+        itt eq
+        lsrseq r1, r2, #1
+        lsreq r1, r2, #1
+
 @ CHECK: lsr.w  r2, r3, #12             @ encoding: [0x4f,0xea,0x13,0x32]
 @ CHECK: lsrs.w  r8, r3, #32             @ encoding: [0x5f,0xea,0x13,0x08]
 @ CHECK: lsrs.w  r2, r3, #1              @ encoding: [0x5f,0xea,0x53,0x02]
 @ CHECK: lsr.w  r2, r3, #4              @ encoding: [0x4f,0xea,0x13,0x12]
 @ CHECK: lsrs.w  r2, r12, #15            @ encoding: [0x5f,0xea,0xdc,0x32]
 
 @ CHECK: lsr.w  r3, r3, #19             @ encoding: [0x4f,0xea,0xd3,0x43]
 @ CHECK: lsrs.w  r8, r8, #2              @ encoding: [0x5f,0xea,0x98,0x08]
 @ CHECK: lsrs.w  r7, r7, #5              @ encoding: [0x5f,0xea,0x57,0x17]
 @ CHECK: lsr.w  r12, r12, #21           @ encoding: [0x4f,0xea,0x5c,0x5c]
 
+@ CHECK: lsrs   r1, r2, #1              @ encoding: [0x51,0x08]
+@ CHECK: itt    eq                      @ encoding: [0x04,0xbf]
+@ CHECK: lsrseq.w r1, r2, #1            @ encoding: [0x5f,0xea,0x52,0x01]
+@ CHECK: lsreq  r1, r2, #1              @ encoding: [0x51,0x08]
 
 @------------------------------------------------------------------------------
 @ LSR (register)
 @------------------------------------------------------------------------------
         lsr r3, r4, r2
         lsr.w r1, r2
         lsrs r3, r4, r8
 

llvm/trunk/test/MC/ARM/lsl-zero.s

+// RUN: not llvm-mc -triple=thumbv7 -show-encoding < %s 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=CHECK-NONARM --check-prefix=CHECK-THUMBV7 %s
+// RUN: not llvm-mc -triple=thumbv8 -show-encoding < %s 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=CHECK-NONARM --check-prefix=CHECK-THUMBV8 %s
+// RUN: llvm-mc -triple=armv7 -show-encoding < %s 2>&1 | FileCheck --check-prefix=CHECK --check-prefix=CHECK-ARM %s
+
+        // lsl #0 is actually mov, so here we check that it behaves the same as
+        // mov with regards to the permitted registers and how it behaves in an
+        // IT block.
+
+        // Using PC is invalid in thumb
+        lsl pc, r0, #0
+        lsl r0, pc, #0
+        lsl pc, pc, #0
+        lsls pc, r0, #0
+        lsls r0, pc, #0
+        lsls pc, pc, #0
+
+// CHECK-NONARM: error: instruction requires: arm-mode
+// CHECK-NONARM-NEXT: lsl pc, r0, #0
+// CHECK-NONARM: error: instruction requires: arm-mode
+// CHECK-NONARM-NEXT: lsl r0, pc, #0
+// CHECK-NONARM: error: instruction requires: arm-mode
+// CHECK-NONARM-NEXT: lsl pc, pc, #0
+// CHECK-NONARM: error: instruction requires: arm-mode
+// CHECK-NONARM-NEXT: lsls pc, r0, #0
+// CHECK-NONARM: error: instruction requires: arm-mode
+// CHECK-NONARM-NEXT: lsls r0, pc, #0
+// CHECK-NONARM: error: instruction requires: arm-mode
+// CHECK-NONARM-NEXT: lsls pc, pc, #0
+
+// CHECK-ARM: mov pc, r0                @ encoding: [0x00,0xf0,0xa0,0xe1]
+// CHECK-ARM: mov r0, pc                @ encoding: [0x0f,0x00,0xa0,0xe1]
+// CHECK-ARM: mov pc, pc                @ encoding: [0x0f,0xf0,0xa0,0xe1]
+// CHECK-ARM: movs pc, r0               @ encoding: [0x00,0xf0,0xb0,0xe1]
+// CHECK-ARM: movs r0, pc               @ encoding: [0x0f,0x00,0xb0,0xe1]
+// CHECK-ARM: movs pc, pc               @ encoding: [0x0f,0xf0,0xb0,0xe1]
+
+        mov pc, r0, lsl #0
+        mov r0, pc, lsl #0
+        mov pc, pc, lsl #0
+        movs pc, r0, lsl #0
+        movs r0, pc, lsl #0
+        movs pc, pc, lsl #0
+
+// FIXME: Really the error we should be giving is "requires: arm-mode"
+// CHECK-NONARM: error: invalid operand for instruction
+// CHECK-NONARM-NEXT: mov pc, r0, lsl #0
+// CHECK-NONARM: error: invalid operand for instruction
+// CHECK-NONARM-NEXT: mov r0, pc, lsl #0
+// CHECK-NONARM: error: invalid operand for instruction
+// CHECK-NONARM-NEXT: mov pc, pc, lsl #0
+// CHECK-NONARM: error: invalid operand for instruction
+// CHECK-NONARM-NEXT: movs pc, r0, lsl #0
+// CHECK-NONARM: error: invalid operand for instruction
+// CHECK-NONARM-NEXT: movs r0, pc, lsl #0
+// CHECK-NONARM: error: invalid operand for instruction
+// CHECK-NONARM-NEXT: movs pc, pc, lsl #0
+
+// CHECK-ARM: mov pc, r0                @ encoding: [0x00,0xf0,0xa0,0xe1]
+// CHECK-ARM: mov r0, pc                @ encoding: [0x0f,0x00,0xa0,0xe1]
+// CHECK-ARM: mov pc, pc                @ encoding: [0x0f,0xf0,0xa0,0xe1]
+// CHECK-ARM: movs pc, r0               @ encoding: [0x00,0xf0,0xb0,0xe1]
+// CHECK-ARM: movs r0, pc               @ encoding: [0x0f,0x00,0xb0,0xe1]
+// CHECK-ARM: movs pc, pc               @ encoding: [0x0f,0xf0,0xb0,0xe1]
+
+        // Using SP is invalid before ARMv8 in thumb unless non-flags-setting
+        // and one of the source and destination is not SP
+        lsl sp, sp, #0
+        lsls sp, sp, #0
+        lsls r0, sp, #0
+        lsls sp, r0, #0
+
+// CHECK-THUMBV7: error: instruction variant requires ARMv8 or later
+// CHECK-THUMBV7-NEXT: lsl sp, sp, #0
+// CHECK-THUMBV7: error: instruction variant requires ARMv8 or later
+// CHECK-THUMBV7-NEXT: lsls sp, sp, #0
+// CHECK-THUMBV7: error: instruction variant requires ARMv8 or later
+// CHECK-THUMBV7-NEXT: lsls r0, sp, #0
+// CHECK-THUMBV7: error: instruction variant requires ARMv8 or later
+// CHECK-THUMBV7-NEXT: lsls sp, r0, #0
+
+// CHECK-ARM: mov sp, sp                @ encoding: [0x0d,0xd0,0xa0,0xe1]
+// CHECK-ARM: movs sp, sp               @ encoding: [0x0d,0xd0,0xb0,0xe1]
+// CHECK-ARM: movs r0, sp               @ encoding: [0x0d,0x00,0xb0,0xe1]
+// CHECK-ARM: movs sp, r0               @ encoding: [0x00,0xd0,0xb0,0xe1]
+
+        mov sp, sp, lsl #0
+        movs sp, sp, lsl #0
+        movs r0, sp, lsl #0
+        movs sp, r0, lsl #0
+
+// FIXME: We should consistently have the "requires ARMv8" error here
+// CHECK-THUMBV7: error: invalid operand for instruction
+// CHECK-THUMBV7-NEXT: mov sp, sp, lsl #0
+// CHECK-THUMBV7: error: invalid operand for instruction
+// CHECK-THUMBV7-NEXT: movs sp, sp, lsl #0
+// CHECK-THUMBV7: error: instruction variant requires ARMv8 or later
+// CHECK-THUMBV7-NEXT: movs r0, sp, lsl #0
+// CHECK-THUMBV7: error: invalid operand for instruction
+// CHECK-THUMBV7-NEXT: movs sp, r0, lsl #0
+
+// CHECK-ARM: mov sp, sp                @ encoding: [0x0d,0xd0,0xa0,0xe1]
+// CHECK-ARM: movs sp, sp               @ encoding: [0x0d,0xd0,0xb0,0xe1]
+// CHECK-ARM: movs r0, sp               @ encoding: [0x0d,0x00,0xb0,0xe1]
+// CHECK-ARM: movs sp, r0               @ encoding: [0x00,0xd0,0xb0,0xe1]
+
+        // Non-flags-setting with only one of source and destination SP should
+        // be OK
+        lsl sp, r0, #0
+        lsl r0, sp, #0
+
+// CHECK-NONARM: mov.w sp, r0           @ encoding: [0x4f,0xea,0x00,0x0d]
+// CHECK-NONARM: mov.w r0, sp           @ encoding: [0x4f,0xea,0x0d,0x00]
+
+// CHECK-ARM: mov sp, r0                @ encoding: [0x00,0xd0,0xa0,0xe1]
+// CHECK-ARM: mov r0, sp                @ encoding: [0x0d,0x00,0xa0,0xe1]
+
+        //FIXME: pre-ARMv8 we give an error for these instructions
+        //mov sp, r0, lsl #0
+        //mov r0, sp, lsl #0
+
+        // LSL #0 in IT block should select the 32-bit encoding
+        itt eq
+        lsleq  r0, r1, #0
+        lslseq r0, r1, #0
+
+// CHECK-NONARM: moveq.w r0, r1         @ encoding: [0x4f,0xea,0x01,0x00]
+// CHECK-NONARM: movseq.w r0, r1        @ encoding: [0x5f,0xea,0x01,0x00]
+
+// CHECK-ARM: moveq r0, r1              @ encoding: [0x01,0x00,0xa0,0x01]
+// CHECK-ARM: movseq r0, r1             @ encoding: [0x01,0x00,0xb0,0x01]
+
+        itt eq
+        moveq  r0, r1, lsl #0
+        movseq r0, r1, lsl #0
+
+// CHECK-NONARM: moveq.w r0, r1         @ encoding: [0x4f,0xea,0x01,0x00]
+// CHECK-NONARM: movseq.w r0, r1        @ encoding: [0x5f,0xea,0x01,0x00]
+
+// CHECK-ARM: moveq r0, r1              @ encoding: [0x01,0x00,0xa0,0x01]
+// CHECK-ARM: movseq r0, r1             @ encoding: [0x01,0x00,0xb0,0x01]