[Thumb] Emit Thumb move in both Thumb modes for struct_byval predicates

[prakhar]

Summary

The MOV/MOVT instructions being chosen for struct_byval predicates was
conditional only on Thumb2, resulting in an ARM MOV/MOVT instruction
being incorrectly emitted in Thumb1 mode. This is especially apparent
with v8-m.base targets. This patch ensures that Thumb instructions are
emitted in both Thumb modes.

[t.p.northover]

Added llvm-commits.

[t.p.northover]

Looks good with a couple of cleanups to the function now that IsThumb exists.

lib/Target/ARM/ARMISelLowering.cpp

7929

Just as well make this IsThumb now.

8031

Also better as isThumb?

[prakhar]

I'll clean up the other uses of IsThumb1/IsThumb2 while I'm at it.

[prakhar]

Clean up other uses of IsThumb1 and IsThumb2

[t.p.northover]

Thanks. Looks fine now.

lib/Target/ARM/ARMISelLowering.cpp

ARMTargetLowering::EmitStructByval(MachineInstr &MI,
   MachineFunction *MF = BB->getParent();
   MachineRegisterInfo &MRI = MF->getRegInfo();
   unsigned UnitSize = 0;
   const TargetRegisterClass *TRC = nullptr;
   const TargetRegisterClass *VecTRC = nullptr;
 
   bool IsThumb1 = Subtarget->isThumb1Only();
   bool IsThumb2 = Subtarget->isThumb2();
+  bool IsThumb = Subtarget->isThumb();
 
   if (Align & 1) {
     UnitSize = 1;
   } else if (Align & 2) {
     UnitSize = 2;
   } else {
     // Check whether we can use NEON instructions.
     if (!MF->getFunction()->hasFnAttribute(Attribute::NoImplicitFloat) &&
         Subtarget->hasNEON()) {
       if ((Align % 16 == 0) && SizeVal >= 16)
         UnitSize = 16;
       else if ((Align % 8 == 0) && SizeVal >= 8)
         UnitSize = 8;
     }
     // Can't use NEON instructions.
     if (UnitSize == 0)
       UnitSize = 4;
   }
 
   // Select the correct opcode and register class for unit size load/store
   bool IsNeon = UnitSize >= 8;
-  TRC = (IsThumb1 || IsThumb2) ? &ARM::tGPRRegClass : &ARM::GPRRegClass;
+  TRC = IsThumb ? &ARM::tGPRRegClass : &ARM::GPRRegClass;
   if (IsNeon)
     VecTRC = UnitSize == 16 ? &ARM::DPairRegClass
                             : UnitSize == 8 ? &ARM::DPRRegClass
                                             : nullptr;
 
   unsigned BytesLeft = SizeVal % UnitSize;
   unsigned LoopSize = SizeVal - BytesLeft;
 
ARMTargetLowering::EmitStructByval(MachineInstr &MI,
 
   // Load an immediate to varEnd.
   unsigned varEnd = MRI.createVirtualRegister(TRC);
   if (Subtarget->useMovt(*MF)) {
     unsigned Vtmp = varEnd;
     if ((LoopSize & 0xFFFF0000) != 0)
       Vtmp = MRI.createVirtualRegister(TRC);
     AddDefaultPred(BuildMI(BB, dl,
-                           TII->get(IsThumb2 ? ARM::t2MOVi16 : ARM::MOVi16),
+                           TII->get(IsThumb ? ARM::t2MOVi16 : ARM::MOVi16),
                            Vtmp).addImm(LoopSize & 0xFFFF));
 
     if ((LoopSize & 0xFFFF0000) != 0)
       AddDefaultPred(BuildMI(BB, dl,
-                             TII->get(IsThumb2 ? ARM::t2MOVTi16 : ARM::MOVTi16),
+                             TII->get(IsThumb ? ARM::t2MOVTi16 : ARM::MOVTi16),
                              varEnd)
                          .addReg(Vtmp)
                          .addImm(LoopSize >> 16));
   } else {
     MachineConstantPool *ConstantPool = MF->getConstantPool();
     Type *Int32Ty = Type::getInt32Ty(MF->getFunction()->getContext());
     const Constant *C = ConstantInt::get(Int32Ty, LoopSize);
 
     // MachineConstantPool wants an explicit alignment.
     unsigned Align = MF->getDataLayout().getPrefTypeAlignment(Int32Ty);
     if (Align == 0)
       Align = MF->getDataLayout().getTypeAllocSize(C->getType());
     unsigned Idx = ConstantPool->getConstantPoolIndex(C, Align);
 
-    if (IsThumb1)
+    if (IsThumb)
       AddDefaultPred(BuildMI(*BB, MI, dl, TII->get(ARM::tLDRpci)).addReg(
           varEnd, RegState::Define).addConstantPoolIndex(Idx));
     else
       AddDefaultPred(BuildMI(*BB, MI, dl, TII->get(ARM::LDRcp)).addReg(
           varEnd, RegState::Define).addConstantPoolIndex(Idx).addImm(0));
   }
   BB->addSuccessor(loopMBB);
 

test/CodeGen/ARM/struct_byval_arm_t1_t2.ll

 ;RUN: llc < %s -mtriple=armv7-none-linux-gnueabi   -mattr=+neon -verify-machineinstrs -filetype=obj | llvm-objdump -triple armv7-none-linux-gnueabi   -disassemble - | FileCheck %s --check-prefix=ARM
 ;RUN: llc < %s -mtriple=thumbv7-none-linux-gnueabi -mattr=+neon -verify-machineinstrs -filetype=obj | llvm-objdump -triple thumbv7-none-linux-gnueabi -disassemble - | FileCheck %s --check-prefix=THUMB2
 ;RUN: llc < %s -mtriple=armv7-none-linux-gnueabi   -mattr=-neon -verify-machineinstrs -filetype=obj | llvm-objdump -triple armv7-none-linux-gnueabi   -disassemble - | FileCheck %s --check-prefix=NO_NEON
 ;We want to have both positive and negative checks for thumb1. These checks
 ;are not easy to do in a single pass so we generate the output once to a
 ;temp file and run filecheck twice with different prefixes.
 ;RUN: llc < %s -mtriple=thumbv5-none-linux-gnueabi              -verify-machineinstrs -filetype=obj | llvm-objdump -triple thumbv5-none-linux-gnueabi -disassemble - > %t
 ;RUN: cat %t | FileCheck %s --check-prefix=THUMB1
 ;RUN: cat %t | FileCheck %s --check-prefix=T1POST
+;RUN: llc < %s -mtriple=thumbv8m.base-arm-none-eabi             -verify-machineinstrs -filetype=obj | llvm-objdump -triple thumbv8m.base-arm-none-eabi -disassemble - > %t
+;RUN: cat %t | FileCheck %s --check-prefix=THUMB1
+;RUN: cat %t | FileCheck %s --check-prefix=T1POST
+;RUN: cat %t | FileCheck %s --check-prefix=V8MBASE
 
 ;This file contains auto generated tests for the lowering of passing structs
 ;byval in the arm backend. We have tests for both packed and unpacked
 ;structs at varying alignments. Each test is run for arm, thumb2 and thumb1.
 ;We check for the strings in the generated object code using llvm-objdump
 ;because it provides better assurance that we are generating instructions
 ;for the correct architecture. Otherwise we could accidentally generate an
 ;ARM instruction for THUMB1 and wouldn't detect it because the assembly

 %struct.I = type  { [ 10 x i32 ], [ 3 x i8 ] }  ; 43 bytes
 declare void @use_I(%struct.I* byval)
 %struct.J = type  { [ 100 x i32 ] }  ; 400 bytes
 declare void @use_J(%struct.J* byval)
 %struct.K = type  { [ 100 x i32 ], i8 }  ; 401 bytes
 declare void @use_K(%struct.K* byval)
 %struct.L = type  { [ 100 x i32 ], [ 3 x i8 ] }  ; 403 bytes
 declare void @use_L(%struct.L* byval)
+%struct.M = type  { [  64 x i8 ] }   ; 64 bytes
+declare void @use_M(%struct.M* byval)
+%struct.N = type  { [ 128 x i8 ] }  ; 128 bytes
+declare void @use_N(%struct.N* byval)
 
 ;ARM-LABEL:    test_A_1:
 ;THUMB2-LABEL: test_A_1:
 ;NO_NEON-LABEL:test_A_1:
 ;THUMB1-LABEL: test_A_1:
 ;T1POST-LABEL: test_A_1:
   define void @test_A_1() {
 ;ARM:         ldrb    r{{[0-9]+}}, [{{.*}}], #1

 ;THUMB1:      bne
 
 ;T1POST-NOT:  vld1.32 {d{{[0-9]+}}, d{{[0-9]+}}}, [r{{.*}}]!
   entry:
     %a = alloca %struct.L, align 16
     call void @use_L(%struct.L* byval align 16 %a)
     ret void
   }
+;V8MBASE-LABEL: test_M:
+  define void @test_M() {
+
+;V8MBASE:      ldrb    r{{[0-9]+}}, {{\[}}[[BASE:r[0-9]+]]{{\]}}
+;V8MBASE:      adds    [[BASE]], #1
+;V8MBASE-NOT:  movw
+  entry:
+    %a = alloca %struct.M, align 1
+    call void @use_M(%struct.M* byval align 1 %a)
+    ret void
+  }
+;V8MBASE-LABEL: test_N:
+  define void @test_N() {
+
+;V8MBASE:      movw    r{{[0-9]+}}, #{{[0-9]+}}
+;V8MBASE-NOT:  b       #{{[0-9]+}}
+  entry:
+    %a = alloca %struct.N, align 1
+    call void @use_N(%struct.N* byval align 1 %a)
+    ret void
+  }