[ARM] GlobalISel: Allow i16 and i8 adds

[rovka]

Summary

This allows i8 and i16 adds, as well as lowering of i8 and i16 function
arguments. Note however that the lowering is incomplete - ABI extensions are
handled in a subsequent patch.

[t.p.northover]

This one also looks like 2 patches really (select more COPY cases, support more args), though again both pretty sensible.

lib/Target/ARM/ARMCallLowering.cpp

36

Checking against vectors is probably good here, since you're changing it anyway. Somewhat surprisingly isInteger returns true for them.

lib/Target/ARM/ARMCallLowering.cpp

 #endif
 
 ARMCallLowering::ARMCallLowering(const ARMTargetLowering &TLI)
     : CallLowering(&TLI) {}
 
 static bool isSupportedType(const DataLayout DL, const ARMTargetLowering &TLI,
                             Type *T) {
   EVT VT = TLI.getValueType(DL, T);
-  return VT.isSimple() && VT.isInteger() &&
-         VT.getSimpleVT().getSizeInBits() == 32;
+  if (!VT.isSimple() || !VT.isInteger())
+    return false;
+
+  unsigned VTSize = VT.getSimpleVT().getSizeInBits();
+  return VTSize == 8 || VTSize == 16 || VTSize == 32;
 }
 
 namespace {
 struct FuncReturnHandler : public CallLowering::ValueHandler {
   FuncReturnHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI,
                     MachineInstrBuilder &MIB)
       : ValueHandler(MIRBuilder, MRI), MIB(MIB) {}
 
   unsigned getStackAddress(uint64_t Size, int64_t Offset,
                            MachinePointerInfo &MPO) override {
     llvm_unreachable("Don't know how to get a stack address yet");
   }
 
   void assignValueToReg(unsigned ValVReg, unsigned PhysReg,
                         CCValAssign &VA) override {
     assert(VA.isRegLoc() && "Value shouldn't be assigned to reg");
     assert(VA.getLocReg() == PhysReg && "Assigning to the wrong reg?");
 
-    assert(VA.getValVT().getSizeInBits() == 32 && "Unsupported value size");
+    assert(VA.getValVT().getSizeInBits() <= 32 && "Unsupported value size");
     assert(VA.getLocVT().getSizeInBits() == 32 && "Unsupported location size");
 
+    assert(VA.getLocInfo() != CCValAssign::SExt &&
+           VA.getLocInfo() != CCValAssign::ZExt &&
+           "ABI extensions not supported yet");
+
     MIRBuilder.buildCopy(PhysReg, ValVReg);
     MIB.addUse(PhysReg, RegState::Implicit);
   }
 
   void assignValueToAddress(unsigned ValVReg, unsigned Addr, uint64_t Size,
                             MachinePointerInfo &MPO, CCValAssign &VA) override {
     llvm_unreachable("Don't know how to assign a value to an address yet");
   }
void assignValueToAddress(unsigned ValVReg, unsigned Addr, uint64_t Size,
     MIRBuilder.buildLoad(ValVReg, Addr, *MMO);
   }
 
   void assignValueToReg(unsigned ValVReg, unsigned PhysReg,
                         CCValAssign &VA) override {
     assert(VA.isRegLoc() && "Value shouldn't be assigned to reg");
     assert(VA.getLocReg() == PhysReg && "Assigning to the wrong reg?");
 
-    assert(VA.getValVT().getSizeInBits() == 32 && "Unsupported value size");
+    assert(VA.getValVT().getSizeInBits() <= 32 && "Unsupported value size");
     assert(VA.getLocVT().getSizeInBits() == 32 && "Unsupported location size");
 
     MIRBuilder.getMBB().addLiveIn(PhysReg);
     MIRBuilder.buildCopy(ValVReg, PhysReg);
   }
 };
 } // End anonymous namespace
 
 bool ARMCallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
                                            const Function &F,
                                            ArrayRef<unsigned> VRegs) const {
   // Quick exit if there aren't any args
   if (F.arg_empty())
     return true;
 
   if (F.isVarArg())
     return false;
 
   auto DL = MIRBuilder.getMF().getDataLayout();
   auto &TLI = *getTLI<ARMTargetLowering>();
 
   auto &Args = F.getArgumentList();
-  for (auto &Arg : Args)
+  unsigned ArgIdx = 0;
+  for (auto &Arg : Args) {
+    ArgIdx++;
     if (!isSupportedType(DL, TLI, Arg.getType()))
       return false;
 
+    // FIXME: This check as well as ArgIdx are going away as soon as we support
+    // loading values < 32 bits.
+    if (ArgIdx > 4 && Arg.getType()->getIntegerBitWidth() != 32)
+      return false;
+  }
+
   CCAssignFn *AssignFn =
       TLI.CCAssignFnForCall(F.getCallingConv(), F.isVarArg());
 
   SmallVector<ArgInfo, 8> ArgInfos;
   unsigned Idx = 0;
   for (auto &Arg : Args) {
     ArgInfo AInfo(VRegs[Idx], Arg.getType());
     setArgFlags(AInfo, Idx + 1, DL, F);
     ArgInfos.push_back(AInfo);
     Idx++;
   }
 
   FormalArgHandler ArgHandler(MIRBuilder, MIRBuilder.getMF().getRegInfo());
   return handleAssignments(MIRBuilder, AssignFn, ArgInfos, ArgHandler);
 }

lib/Target/ARM/ARMInstructionSelector.cpp

static bool selectCopy(MachineInstr &I, const TargetInstrInfo &TII,
 
   const RegisterBank *RegBank = RBI.getRegBank(DstReg, MRI, TRI);
   assert(RegBank && "Can't get reg bank for virtual register");
 
   const unsigned DstSize = MRI.getType(DstReg).getSizeInBits();
   unsigned SrcReg = I.getOperand(1).getReg();
   const unsigned SrcSize = RBI.getSizeInBits(SrcReg, MRI, TRI);
   (void)SrcSize;
-  assert(DstSize == SrcSize && "Copy with different width?!");
+  assert((DstSize == SrcSize ||
+          // Copies are a means to setup initial types, the number of
+          // bits may not exactly match.
+          (TargetRegisterInfo::isPhysicalRegister(SrcReg) &&
+           DstSize <= SrcSize)) &&
+         "Copy with different width?!");
 
   assert(RegBank->getID() == ARM::GPRRegBankID && "Unsupported reg bank");
   const TargetRegisterClass *RC = &ARM::GPRRegClass;
 
   // No need to constrain SrcReg. It will get constrained when
   // we hit another of its uses or its defs.
   // Copies do not have constraints.
   if (!RBI.constrainGenericRegister(DstReg, *RC, MRI)) {

lib/Target/ARM/ARMLegalizerInfo.cpp

 using namespace llvm;
 
 #ifndef LLVM_BUILD_GLOBAL_ISEL
 #error "You shouldn't build this"
 #endif
 
 ARMLegalizerInfo::ARMLegalizerInfo() {
   using namespace TargetOpcode;
+
   const LLT p0 = LLT::pointer(0, 32);
+
+  const LLT s8 = LLT::scalar(8);
+  const LLT s16 = LLT::scalar(16);
   const LLT s32 = LLT::scalar(32);
 
   setAction({G_FRAME_INDEX, p0}, Legal);
 
+  // TODO: smaller types
   setAction({G_LOAD, s32}, Legal);
   setAction({G_LOAD, 1, p0}, Legal);
 
-  setAction({G_ADD, s32}, Legal);
+  for (auto Ty : {s8, s16, s32})
+    setAction({G_ADD, Ty}, Legal);
 
   computeTables();
 }

test/CodeGen/ARM/GlobalISel/arm-instruction-select.mir

 # RUN: llc -O0 -mtriple arm-- -global-isel -run-pass=instruction-select -verify-machineinstrs %s -o - | FileCheck %s
 --- |
-  define void @test_adds32() { ret void }
+  define void @test_add_s8() { ret void }
+  define void @test_add_s16() { ret void }
+  define void @test_add_s32() { ret void }
+
   define void @test_load_from_stack() { ret void }
 ...
 ---
-name:            test_adds32
-# CHECK-LABEL: name: test_adds32
+name:            test_add_s8
+# CHECK-LABEL: name: test_add_s8
+legalized:       true
+regBankSelected: true
+selected:        false
+# CHECK: selected: true
+registers:
+  - { id: 0, class: gprb }
+  - { id: 1, class: gprb }
+  - { id: 2, class: gprb }
+body:             |
+  bb.0:
+    liveins: %r0, %r1
+
+    %0(s8) = COPY %r0
+    ; CHECK: [[VREGX:%[0-9]+]] = COPY %r0
+
+    %1(s8) = COPY %r1
+    ; CHECK: [[VREGY:%[0-9]+]] = COPY %r1
+
+    %2(s8) = G_ADD %0, %1
+    ; CHECK: [[VREGSUM:%[0-9]+]] = ADDrr [[VREGX]], [[VREGY]], 14, _, _
+
+    %r0 = COPY %2(s8)
+    ; CHECK: %r0 = COPY [[VREGSUM]]
+
+    BX_RET 14, _, implicit %r0
+    ; CHECK: BX_RET 14, _, implicit %r0
+...
+---
+name:            test_add_s16
+# CHECK-LABEL: name: test_add_s16
+legalized:       true
+regBankSelected: true
+selected:        false
+# CHECK: selected: true
+registers:
+  - { id: 0, class: gprb }
+  - { id: 1, class: gprb }
+  - { id: 2, class: gprb }
+body:             |
+  bb.0:
+    liveins: %r0, %r1
+
+    %0(s16) = COPY %r0
+    ; CHECK: [[VREGX:%[0-9]+]] = COPY %r0
+
+    %1(s16) = COPY %r1
+    ; CHECK: [[VREGY:%[0-9]+]] = COPY %r1
+
+    %2(s16) = G_ADD %0, %1
+    ; CHECK: [[VREGSUM:%[0-9]+]] = ADDrr [[VREGX]], [[VREGY]], 14, _, _
+
+    %r0 = COPY %2(s16)
+    ; CHECK: %r0 = COPY [[VREGSUM]]
+
+    BX_RET 14, _, implicit %r0
+    ; CHECK: BX_RET 14, _, implicit %r0
+...
+---
+name:            test_add_s32
+# CHECK-LABEL: name: test_add_s32
 legalized:       true
 regBankSelected: true
 selected:        false
 # CHECK: selected: true
 registers:
   - { id: 0, class: gprb }
   - { id: 1, class: gprb }
   - { id: 2, class: gprb }

test/CodeGen/ARM/GlobalISel/arm-irtranslator.ll

 ; RUN: llc -mtriple arm-unknown -global-isel -stop-after=irtranslator %s -o - | FileCheck %s
 
 define void @test_void_return() {
 ; CHECK-LABEL: name: test_void_return
 ; CHECK: BX_RET 14, _
 entry:
   ret void
 }
 
-define i32 @test_add(i32 %x, i32 %y) {
-; CHECK-LABEL: name: test_add
+define i8 @test_add_i8(i8 %x, i8 %y) {
+; CHECK-LABEL: name: test_add_i8
 ; CHECK: liveins: %r0, %r1
-; CHECK: [[VREGX:%[0-9]+]]{{.*}} = COPY %r0
-; CHECK: [[VREGY:%[0-9]+]]{{.*}} = COPY %r1
-; CHECK: [[SUM:%[0-9]+]]{{.*}} = G_ADD [[VREGX]], [[VREGY]]
-; CHECK: %r0 = COPY [[SUM]]
+; CHECK-DAG: [[VREGX:%[0-9]+]](s8) = COPY %r0
+; CHECK-DAG: [[VREGY:%[0-9]+]](s8) = COPY %r1
+; CHECK: [[SUM:%[0-9]+]](s8) = G_ADD [[VREGX]], [[VREGY]]
+; CHECK: %r0 = COPY [[SUM]](s8)
+; CHECK: BX_RET 14, _, implicit %r0
+entry:
+  %sum = add i8 %x, %y
+  ret i8 %sum
+}
+
+define i16 @test_add_i16(i16 %x, i16 %y) {
+; CHECK-LABEL: name: test_add_i16
+; CHECK: liveins: %r0, %r1
+; CHECK-DAG: [[VREGX:%[0-9]+]](s16) = COPY %r0
+; CHECK-DAG: [[VREGY:%[0-9]+]](s16) = COPY %r1
+; CHECK: [[SUM:%[0-9]+]](s16) = G_ADD [[VREGX]], [[VREGY]]
+; CHECK: %r0 = COPY [[SUM]](s16)
+; CHECK: BX_RET 14, _, implicit %r0
+entry:
+  %sum = add i16 %x, %y
+  ret i16 %sum
+}
+
+define i32 @test_add_i32(i32 %x, i32 %y) {
+; CHECK-LABEL: name: test_add_i32
+; CHECK: liveins: %r0, %r1
+; CHECK-DAG: [[VREGX:%[0-9]+]](s32) = COPY %r0
+; CHECK-DAG: [[VREGY:%[0-9]+]](s32) = COPY %r1
+; CHECK: [[SUM:%[0-9]+]](s32) = G_ADD [[VREGX]], [[VREGY]]
+; CHECK: %r0 = COPY [[SUM]](s32)
 ; CHECK: BX_RET 14, _, implicit %r0
 entry:
   %sum = add i32 %x, %y
   ret i32 %sum
 }
 
 define i32 @test_many_args(i32 %p0, i32 %p1, i32 %p2, i32 %p3, i32 %p4, i32 %p5) {
 ; CHECK-LABEL: name: test_many_args

test/CodeGen/ARM/GlobalISel/arm-isel.ll

 ; RUN: llc -mtriple arm-unknown -global-isel %s -o - | FileCheck %s
 
 define void @test_void_return() {
 ; CHECK-LABEL: test_void_return:
 ; CHECK: bx lr
 entry:
   ret void
 }
 
-define i32 @test_add(i32 %x, i32 %y) {
-; CHECK-LABEL: test_add:
+define i8 @test_add_i8(i8 %x, i8 %y) {
+; CHECK-LABEL: test_add_i8:
+; CHECK: add r0, r0, r1
+; CHECK: bx lr
+entry:
+  %sum = add i8 %x, %y
+  ret i8 %sum
+}
+
+define i16 @test_add_i16(i16 %x, i16 %y) {
+; CHECK-LABEL: test_add_i16:
+; CHECK: add r0, r0, r1
+; CHECK: bx lr
+entry:
+  %sum = add i16 %x, %y
+  ret i16 %sum
+}
+
+define i32 @test_add_i32(i32 %x, i32 %y) {
+; CHECK-LABEL: test_add_i32:
 ; CHECK: add r0, r0, r1
 ; CHECK: bx lr
 entry:
   %sum = add i32 %x, %y
   ret i32 %sum
 }
 
 define i32 @test_many_args(i32 %p0, i32 %p1, i32 %p2, i32 %p3, i32 %p4, i32 %p5) {

test/CodeGen/ARM/GlobalISel/arm-legalizer.mir

 # RUN: llc -mtriple arm-- -global-isel -run-pass=legalizer %s -o - | FileCheck %s
 --- |
+  define void @test_add_s8() { ret void }
+  define void @test_add_s16() { ret void }
   define void @test_add_s32() { ret void }
+
   define void @test_load_from_stack() { ret void }
 ...
 ---
+name:            test_add_s8
+# CHECK-LABEL: name: test_add_s8
+legalized:       false
+# CHECK: legalized: true
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+body:             |
+  bb.0:
+    liveins: %r0, %r1
+
+    %0(s8) = COPY %r0
+    %1(s8) = COPY %r1
+    %2(s8) = G_ADD %0, %1
+    ; G_ADD with s8 is legal, so we should find it unchanged in the output
+    ; CHECK: {{%[0-9]+}}(s8) = G_ADD {{%[0-9]+, %[0-9]+}}
+    %r0 = COPY %2(s8)
+    BX_RET 14, _, implicit %r0
+...
+---
+name:            test_add_s16
+# CHECK-LABEL: name: test_add_s16
+legalized:       false
+# CHECK: legalized: true
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+body:             |
+  bb.0:
+    liveins: %r0, %r1
+
+    %0(s16) = COPY %r0
+    %1(s16) = COPY %r1
+    %2(s16) = G_ADD %0, %1
+    ; G_ADD with s16 is legal, so we should find it unchanged in the output
+    ; CHECK: {{%[0-9]+}}(s16) = G_ADD {{%[0-9]+, %[0-9]+}}
+    %r0 = COPY %2(s16)
+    BX_RET 14, _, implicit %r0
+
+...
+---
 name:            test_add_s32
 # CHECK-LABEL: name: test_add_s32
 legalized:       false
 # CHECK: legalized: true
 regBankSelected: false
 selected:        false
 tracksRegLiveness: true
 registers:
bb.0:
     liveins: %r0, %r1, %r2, %r3
 
     ; This is legal, so we should find it unchanged in the output
     ; CHECK: [[FIVREG:%[0-9]+]](p0) = G_FRAME_INDEX %fixed-stack.[[FRAME_INDEX]]
     ; CHECK: {{%[0-9]+}}(s32) = G_LOAD [[FIVREG]](p0)
     %0(p0) = G_FRAME_INDEX %fixed-stack.2
     %1(s32) = G_LOAD %0(p0)
     BX_RET 14, _
-
 ...

test/CodeGen/ARM/GlobalISel/arm-regbankselect.mir

 # RUN: llc -mtriple arm-- -global-isel -run-pass=regbankselect %s -o - | FileCheck %s
 --- |
   define void @test_add_s32() { ret void }
+  define void @test_add_s16() { ret void }
+  define void @test_add_s8() { ret void }
 ...
 ---
 name:            test_add_s32
 # CHECK-LABEL: name: test_add_s32
 legalized:       true
 regBankSelected: false
 selected:        false
 # CHECK: registers:
bb.0:
 
     %0(s32) = COPY %r0
     %1(s32) = COPY %r1
     %2(s32) = G_ADD %0, %1
     %r0 = COPY %2(s32)
     BX_RET 14, _, implicit %r0
 
 ...
+---
+name:            test_add_s16
+# CHECK-LABEL: name: test_add_s16
+legalized:       true
+regBankSelected: false
+selected:        false
+# CHECK: registers:
+# CHECK: - { id: 0, class: gprb }
+# CHECK: - { id: 1, class: gprb }
+# CHECK: - { id: 2, class: gprb }
+
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+body:             |
+  bb.0:
+    liveins: %r0, %r1
+
+    %0(s16) = COPY %r0
+    %1(s16) = COPY %r1
+    %2(s16) = G_ADD %0, %1
+    %r0 = COPY %2(s16)
+    BX_RET 14, _, implicit %r0
+
+...
+---
+name:            test_add_s8
+# CHECK-LABEL: name: test_add_s8
+legalized:       true
+regBankSelected: false
+selected:        false
+# CHECK: registers:
+# CHECK: - { id: 0, class: gprb }
+# CHECK: - { id: 1, class: gprb }
+# CHECK: - { id: 2, class: gprb }
+
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+body:             |
+  bb.0:
+    liveins: %r0, %r1
+
+    %0(s8) = COPY %r0
+    %1(s8) = COPY %r1
+    %2(s8) = G_ADD %0, %1
+    %r0 = COPY %2(s8)
+    BX_RET 14, _, implicit %r0
+
+...