[ARM] GlobalISel: Select add i32, i32

[rovka]

Summary

Add the minimal support necessary to select a function that returns the sum of
two i32 values.

This includes some very naive and hardcoded argument/return lowering (to be
generalized in the future), as well as the handling of copy and add instructions
throughout the GlobalISel pipeline.

[rovka]

lib/CodeGen/GlobalISel/InstructionSelector.cpp

46

I'm not 100% sure that this is true in the general case. Thoughts?

[t.p.northover]

lib/CodeGen/GlobalISel/InstructionSelector.cpp

46

Seems a bit iffy. What was triggering it in ARM?

lib/Target/ARM/ARMRegisterBankInfo.cpp

114

You'll probably want this to be "return false" if you intend to use the SDAG fallback in your testing (and similarly for some other assertions).

[qcolombet]

Adding Ahmed as reviewer.

[rovka]

lib/CodeGen/GlobalISel/InstructionSelector.cpp

46

For ARM, the predicate register (as added by AddDefaultPred) doesn't have a register class, and the CC register (which is an optional def) isn't found in MRI's VRegInfo (so we get an assert in MRI::getRegClassOrRegBank).

It's very possible that I'm missing something, but AFAICT the other isel frameworks don't call constrainRegClass on the predicate and optional defs either (at least for ARM). I don't know what is better - rolling a custom version of this function in the ARM backend, or skipping predicates and optional defs here and letting each target handle them. I'm open to other solutions too if you can suggest any :)

lib/Target/ARM/ARMRegisterBankInfo.cpp

114

Right, I should probably start doing that. Thanks.

[rovka]

Rebase + some microscopic cleanups.

[rovka]

lib/CodeGen/GlobalISel/InstructionSelector.cpp

46

@tstellarAMD : I see that some R600 instructions have predicate operands, do you have any thoughts on how it would be best to handle them?

[rovka]

Make use of ValueHandler for lowering function arguments / returns.

[ab]

This seems mostly fine; first round of nitpicks ;)

Apologies for the long break.

lib/CodeGen/GlobalISel/InstructionSelector.cpp

46

Shouldn't this simply be:

if (MO.getReg() == 0)
  continue;

?

That seems like the relevant characteristic here: both the predicate and CC optional def are either <x>PSR (a physreg, ignored earlier), or 0 (no-reg).

lib/Target/ARM/ARMInstructionSelector.cpp

75–77

One-line -> no braces (here and elsewhere)

lib/Target/ARM/ARMRegisterBankInfo.cpp

55

How about "GPRB"?

110

Nitpick:

..., /*Cost=*/1, ...

instead of a separate variable?

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

4

Nitpick: I'd call this add_s32; I initially parsed it as adds+32, not add+s32 ;)

[rovka]

lib/CodeGen/GlobalISel/InstructionSelector.cpp

46

Ok, that works too. I was trying to be specific because I'm not very familiar with all the subtleties here, e.g. if there are other cases when a MO.getReg() could be 0 and we wouldn't want to skip it.

lib/Target/ARM/ARMInstructionSelector.cpp

75–77

Derp. Thanks.

lib/Target/ARM/ARMRegisterBankInfo.cpp

55

Sure, why not :)

Speaking of this, for AArch64 "GPR" is a reg bank, and for ARM it's a reg class. Would it be a good idea to have a target-independent convention for naming register banks? It could be a prefix / suffix added by TableGen to the name of the reg bank. I think it would make it easier to spot if a MIR file has been reg-bank-selected or not (without having to look for the regbankselected attribute).

110

Makes sense.

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

4

Good point :D

[rovka]

Update based on Ahmed's feedback. Thanks!

[rovka]

Ping.

[ab]

Looks good on my side, modulo a couple extra nitpicks (please do check the constraining in the test though).

lib/Target/ARM/ARMCallLowering.cpp

81–82

Is this so that you can early-return in the simple case? FWIW I found it non-obvious; how about extracting the assignment code into some lowerReturnVal or something?

lib/Target/ARM/ARMISelLowering.h

493–494

Not a big fan of adding these in this patch; maybe commit this separately (and use it in ISelLowering.cpp) ?

lib/Target/ARM/ARMRegisterBankInfo.cpp

55

Speaking of this, for AArch64 "GPR" is a reg bank, and for ARM it's a reg class. Would it be a good idea to have a target-independent convention for naming register banks? It could be a prefix / suffix added by TableGen to the name of the reg bank.

It might; any ideas? We could also simply call the bank "GPR" on ARM. I don't think there's ever any ambiguity between bank name and class name (in MIR/C++, there might be ambiguity for humans). In fact, CCR is both the bank and the class on AArch64, but it's not really used, so problems might be hiding.

I think it would make it easier to spot if a MIR file has been reg-bank-selected or not (without having to look for the regbankselected attribute).

Eh, look for "class: _" ? ;)

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

12–15

Check the registers too?

[rovka]

Thanks for reviewing! I'll commit it with the requested changes.

Do you think you could have a look at the other patches in the series before going on vacation? https://reviews.llvm.org/D27803 + its dependencies.

Cheers,
Diana

lib/Target/ARM/ARMCallLowering.cpp

81–82

Ok, sorry about the confusion.

lib/Target/ARM/ARMISelLowering.h

493–494

Will do.

lib/Target/ARM/ARMRegisterBankInfo.cpp

55

Actually there is some ambiguity. Initially I named the bank GPR and got some really funny errors when trying to read in the output of a regbankselect run. It turns out MIRParserImpl::parseRegisterInfo calls getRegClass and only if that doesn't return anything it calls getRegBank (and then we get "unexpected size on non-generic virtual register" later on). It actually took me a while to figure out what the problem was, but then I just assumed it went without saying that reg classes and reg banks should have different names. We should probably add some asserts somewhere if it's not obvious to you either that the names should be different. I thought it was just me :)

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

12–15

Good point.

lib/CodeGen/GlobalISel/InstructionSelector.cpp

for (unsigned OpI = 0, OpE = I.getNumExplicitOperands(); OpI != OpE; ++OpI) {
 
     DEBUG(dbgs() << "Converting operand: " << MO << '\n');
     assert(MO.isReg() && "Unsupported non-reg operand");
 
     // Physical registers don't need to be constrained.
     if (TRI.isPhysicalRegister(MO.getReg()))
       continue;
 
+    // Register operands with a value of 0 (e.g. predicate operands) don't need
+    // to be constrained.
+    if (MO.getReg() == 0)
+      continue;
+
     const TargetRegisterClass *RC = TII.getRegClass(I.getDesc(), OpI, &TRI, MF);
     assert(RC && "Selected inst should have regclass operand");
 
     // If the operand is a vreg, we should constrain its regclass, and only
     // insert COPYs if that's impossible.
     // If the operand is a physreg, we only insert COPYs if the register class
     // doesn't contain the register.
     if (RBI.constrainGenericRegister(MO.getReg(), *RC, MRI))
       continue;
 
     DEBUG(dbgs() << "Constraining with COPYs isn't implemented yet");
     return false;
   }
   return true;
 }

lib/Target/ARM/ARMCallLowering.cpp

 
 #ifndef LLVM_BUILD_GLOBAL_ISEL
 #error "This shouldn't be built without GISel"
 #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;
+}
+
+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.getLocVT().getSizeInBits() == 32 && "Unsupported location size");
+
+    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");
+  }
+
+  MachineInstrBuilder &MIB;
+};
+} // End anonymous namespace.
+
 bool ARMCallLowering::lowerReturn(MachineIRBuilder &MIRBuilder,
                                   const Value *Val, unsigned VReg) const {
-  // We're currently only handling void returns
-  if (Val != nullptr)
+  assert(!Val == !VReg && "Return value without a vreg");
+
+  auto Ret = AddDefaultPred(MIRBuilder.buildInstr(ARM::BX_RET));
+  if (!Val) {
+    // Nothing more to do
+    return true;
+  }
+
+  // Move the insertion point before the return
+  MIRBuilder.setInstr(*Ret);
+
+  auto &MF = MIRBuilder.getMF();
+  const auto &F = *MF.getFunction();
+
+  auto DL = MF.getDataLayout();
+  auto &TLI = *getTLI<ARMTargetLowering>();
+  if (!isSupportedType(DL, TLI, Val->getType()))
     return false;
 
-  AddDefaultPred(MIRBuilder.buildInstr(ARM::BX_RET));
+  CCAssignFn *AssignFn =
+      TLI.CCAssignFnForReturn(F.getCallingConv(), F.isVarArg());
 
-  return true;
+  ArgInfo RetInfo(VReg, Val->getType());
+  setArgFlags(RetInfo, AttributeSet::ReturnIndex, DL, F);
+
+  FuncReturnHandler RetHandler(MIRBuilder, MF.getRegInfo(), Ret);
+  return handleAssignments(MIRBuilder, AssignFn, RetInfo, RetHandler);
 }
 
+namespace {
+struct FormalArgHandler : public CallLowering::ValueHandler {
+  FormalArgHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI)
+      : ValueHandler(MIRBuilder, MRI) {}
+
+  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.getLocVT().getSizeInBits() == 32 && "Unsupported location size");
+
+    MIRBuilder.getMBB().addLiveIn(PhysReg);
+    MIRBuilder.buildCopy(ValVReg, PhysReg);
+  }
+
+  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");
+  }
+};
+} // End anonymous namespace
+
 bool ARMCallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
                                            const Function &F,
                                            ArrayRef<unsigned> VRegs) const {
-  return F.arg_empty();
+  // Quick exit if there aren't any args
+  if (F.arg_empty())
+    return true;
+
+  // Stick to only 4 arguments for now
+  if (F.arg_size() > 4)
+    return false;
+
+  if (F.isVarArg())
+    return false;
+
+  auto DL = MIRBuilder.getMF().getDataLayout();
+  auto &TLI = *getTLI<ARMTargetLowering>();
+
+  auto &Args = F.getArgumentList();
+  for (auto &Arg : Args)
+    if (!isSupportedType(DL, TLI, Arg.getType()))
+      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/ARMISelLowering.h

public:
     bool supportSwiftError() const override {
       return true;
     }
 
     bool hasStandaloneRem(EVT VT) const override {
       return HasStandaloneRem;
     }
 
+    CCAssignFn *CCAssignFnForCall(CallingConv::ID CC, bool isVarArg) const;
+    CCAssignFn *CCAssignFnForReturn(CallingConv::ID CC, bool isVarArg) const;
+
   protected:
     std::pair<const TargetRegisterClass *, uint8_t>
     findRepresentativeClass(const TargetRegisterInfo *TRI,
                             MVT VT) const override;
 
   private:
     /// Subtarget - Keep a pointer to the ARMSubtarget around so that we can
     /// make the right decision when generating code for different targets.

lib/Target/ARM/ARMISelLowering.cpp

if (!Subtarget->isAAPCS_ABI()) {
       return CallingConv::ARM_APCS;
     } else if (Subtarget->hasVFP2() && !Subtarget->isThumb1Only() && !isVarArg)
       return CallingConv::ARM_AAPCS_VFP;
     else
       return CallingConv::ARM_AAPCS;
   }
 }
 
+CCAssignFn *ARMTargetLowering::CCAssignFnForCall(CallingConv::ID CC,
+                                                 bool isVarArg) const {
+  return CCAssignFnForNode(CC, false, isVarArg);
+}
+
+CCAssignFn *ARMTargetLowering::CCAssignFnForReturn(CallingConv::ID CC,
+                                                   bool isVarArg) const {
+  return CCAssignFnForNode(CC, true, isVarArg);
+}
+
 /// CCAssignFnForNode - Selects the correct CCAssignFn for the given
 /// CallingConvention.
 CCAssignFn *ARMTargetLowering::CCAssignFnForNode(CallingConv::ID CC,
                                                  bool Return,
                                                  bool isVarArg) const {
   switch (getEffectiveCallingConv(CC, isVarArg)) {
   default:
     llvm_unreachable("Unsupported calling convention");

lib/Target/ARM/ARMInstructionSelector.h

public:
   ARMInstructionSelector(const ARMSubtarget &STI,
                          const ARMRegisterBankInfo &RBI);
 
   virtual bool select(MachineInstr &I) const override;
 
 private:
   const ARMBaseInstrInfo &TII;
   const ARMBaseRegisterInfo &TRI;
+  const ARMRegisterBankInfo &RBI;
 };
 
 } // End llvm namespace.
 #endif

lib/Target/ARM/ARMInstructionSelector.cpp

 /// This file implements the targeting of the InstructionSelector class for ARM.
 /// \todo This should be generated by TableGen.
 //===----------------------------------------------------------------------===//
 
 #include "ARMInstructionSelector.h"
 #include "ARMRegisterBankInfo.h"
 #include "ARMSubtarget.h"
 #include "ARMTargetMachine.h"
+#include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/Support/Debug.h"
 
 #define DEBUG_TYPE "arm-isel"
 
 using namespace llvm;
 
 #ifndef LLVM_BUILD_GLOBAL_ISEL
 #error "You shouldn't build this"
 #endif
 
 ARMInstructionSelector::ARMInstructionSelector(const ARMSubtarget &STI,
                                                const ARMRegisterBankInfo &RBI)
     : InstructionSelector(), TII(*STI.getInstrInfo()),
-      TRI(*STI.getRegisterInfo()) {}
+      TRI(*STI.getRegisterInfo()), RBI(RBI) {}
 
-bool ARMInstructionSelector::select(llvm::MachineInstr &I) const {
-  return !isPreISelGenericOpcode(I.getOpcode());
+static bool selectCopy(MachineInstr &I, const TargetInstrInfo &TII,
+                       MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
+                       const RegisterBankInfo &RBI) {
+  unsigned DstReg = I.getOperand(0).getReg();
+  if (TargetRegisterInfo::isPhysicalRegister(DstReg))
+    return true;
+
+  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(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)) {
+    DEBUG(dbgs() << "Failed to constrain " << TII.getName(I.getOpcode())
+                 << " operand\n");
+    return false;
+  }
+  return true;
+}
+
+bool ARMInstructionSelector::select(MachineInstr &I) const {
+  assert(I.getParent() && "Instruction should be in a basic block!");
+  assert(I.getParent()->getParent() && "Instruction should be in a function!");
+
+  auto &MBB = *I.getParent();
+  auto &MF = *MBB.getParent();
+  auto &MRI = MF.getRegInfo();
+
+  if (!isPreISelGenericOpcode(I.getOpcode())) {
+    if (I.isCopy())
+      return selectCopy(I, TII, MRI, TRI, RBI);
+
+    return true;
+  }
+
+  if (I.getOpcode() == TargetOpcode::G_ADD) {
+    I.setDesc(TII.get(ARM::ADDrr));
+    AddDefaultCC(AddDefaultPred(MachineInstrBuilder(MF, I)));
+    return constrainSelectedInstRegOperands(I, TII, TRI, RBI);
+  }
+
+  return false;
 }

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 s32 = LLT::scalar(32);
+
+  setAction({G_ADD, s32}, Legal);
+
   computeTables();
 }

lib/Target/ARM/ARMRegisterBankInfo.h

 #define LLVM_LIB_TARGET_ARM_ARMREGISTERBANKINFO_H
 
 #include "llvm/CodeGen/GlobalISel/RegisterBankInfo.h"
 
 namespace llvm {
 
 class TargetRegisterInfo;
 
+namespace ARM {
+enum {
+  GPRRegBankID = 0, // General purpose registers
+  NumRegisterBanks,
+};
+} // end namespace ARM
+
 /// This class provides the information for the target register banks.
 class ARMRegisterBankInfo final : public RegisterBankInfo {
 public:
   ARMRegisterBankInfo(const TargetRegisterInfo &TRI);
+
+  const RegisterBank &
+  getRegBankFromRegClass(const TargetRegisterClass &RC) const override;
+
+  InstructionMapping getInstrMapping(const MachineInstr &MI) const override;
 };
 } // End llvm namespace.
 #endif

lib/Target/ARM/ARMRegisterBankInfo.cpp

 //===- ARMRegisterBankInfo.cpp -----------------------------------*- C++ -*-==//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 /// \file
 /// This file implements the targeting of the RegisterBankInfo class for ARM.
 /// \todo This should be generated by TableGen.
 //===----------------------------------------------------------------------===//
 
 #include "ARMRegisterBankInfo.h"
+#include "ARMInstrInfo.h" // For the register classes
 #include "llvm/CodeGen/GlobalISel/RegisterBank.h"
 #include "llvm/CodeGen/GlobalISel/RegisterBankInfo.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/Target/TargetRegisterInfo.h"
 
 using namespace llvm;
 
 #ifndef LLVM_BUILD_GLOBAL_ISEL
 #error "You shouldn't build this"
 #endif
 
+// FIXME: TableGen this.
+// If it grows too much and TableGen still isn't ready to do the job, extract it
+// into an ARMGenRegisterBankInfo.def (similar to AArch64).
+namespace llvm {
+namespace ARM {
+RegisterBank GPRRegBank;
+RegisterBank *RegBanks[] = {&GPRRegBank};
+
+RegisterBankInfo::PartialMapping GPRPartialMapping{0, 32, GPRRegBank};
+
+RegisterBankInfo::ValueMapping ValueMappings[] = {
+    {&GPRPartialMapping, 1}, {&GPRPartialMapping, 1}, {&GPRPartialMapping, 1}};
+} // end namespace arm
+} // end namespace llvm
+
 ARMRegisterBankInfo::ARMRegisterBankInfo(const TargetRegisterInfo &TRI)
-    : RegisterBankInfo(nullptr, 0) {}
+    : RegisterBankInfo(ARM::RegBanks, ARM::NumRegisterBanks) {
+  static bool AlreadyInit = false;
+  // We have only one set of register banks, whatever the subtarget
+  // is. Therefore, the initialization of the RegBanks table should be
+  // done only once. Indeed the table of all register banks
+  // (ARM::RegBanks) is unique in the compiler. At some point, it
+  // will get tablegen'ed and the whole constructor becomes empty.
+  if (AlreadyInit)
+    return;
+  AlreadyInit = true;
+
+  // Initialize the GPR bank.
+  createRegisterBank(ARM::GPRRegBankID, "GPRB");
+
+  addRegBankCoverage(ARM::GPRRegBankID, ARM::GPRRegClassID, TRI);
+  addRegBankCoverage(ARM::GPRRegBankID, ARM::GPRwithAPSRRegClassID, TRI);
+  const RegisterBank &RBGPR = getRegBank(ARM::GPRRegBankID);
+  (void)RBGPR;
+  assert(&ARM::GPRRegBank == &RBGPR && "The order in RegBanks is messed up");
+  assert(RBGPR.covers(*TRI.getRegClass(ARM::GPRRegClassID)) &&
+         "Subclass not added?");
+  assert(RBGPR.covers(*TRI.getRegClass(ARM::GPRwithAPSRRegClassID)) &&
+         "Subclass not added?");
+  assert(RBGPR.covers(*TRI.getRegClass(ARM::GPRnopcRegClassID)) &&
+         "Subclass not added?");
+  assert(RBGPR.covers(*TRI.getRegClass(ARM::rGPRRegClassID)) &&
+         "Subclass not added?");
+  assert(RBGPR.covers(*TRI.getRegClass(ARM::tGPRRegClassID)) &&
+         "Subclass not added?");
+  assert(RBGPR.covers(*TRI.getRegClass(ARM::tcGPRRegClassID)) &&
+         "Subclass not added?");
+  assert(RBGPR.covers(*TRI.getRegClass(ARM::tGPR_and_tcGPRRegClassID)) &&
+         "Subclass not added?");
+  assert(RBGPR.getSize() == 32 && "GPRs should hold up to 32-bit");
+}
+
+const RegisterBank &ARMRegisterBankInfo::getRegBankFromRegClass(
+    const TargetRegisterClass &RC) const {
+  using namespace ARM;
+
+  switch (RC.getID()) {
+  case GPRRegClassID:
+  case tGPR_and_tcGPRRegClassID:
+    return getRegBank(ARM::GPRRegBankID);
+  default:
+    llvm_unreachable("Unsupported register kind");
+  }
+
+  llvm_unreachable("Switch should handle all register classes");
+}
+
+RegisterBankInfo::InstructionMapping
+ARMRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
+  auto Opc = MI.getOpcode();
+
+  // Try the default logic for non-generic instructions that are either copies
+  // or already have some operands assigned to banks.
+  if (!isPreISelGenericOpcode(Opc)) {
+    InstructionMapping Mapping = getInstrMappingImpl(MI);
+    if (Mapping.isValid())
+      return Mapping;
+  }
+
+  if (Opc == TargetOpcode::G_ADD) {
+    unsigned NumOperands = MI.getNumOperands();
+    ValueMapping *OperandsMapping = &ARM::ValueMappings[0];
+    return InstructionMapping{DefaultMappingID, /*Cost=*/1, OperandsMapping,
+                              NumOperands};
+  }
+
+  return InstructionMapping{};
+}

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 }
+...
+---
+name:            test_adds32
+# CHECK-LABEL: name: test_adds32
+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(s32) = COPY %r0
+    ; CHECK: [[VREGX:%[0-9]+]] = COPY %r0
+
+    %1(s32) = COPY %r1
+    ; CHECK: [[VREGY:%[0-9]+]] = COPY %r1
+
+    %2(s32) = G_ADD %0, %1
+    ; CHECK: [[VREGSUM:%[0-9]+]] = ADDrr [[VREGX]], [[VREGY]], 14, _, _
+
+    %r0 = COPY %2(s32)
+    ; CHECK: %r0 = COPY [[VREGSUM]]
+
+    BX_RET 14, _, implicit %r0
+    ; CHECK: BX_RET 14, _, implicit %r0
+...

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
+; 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: BX_RET 14, _, implicit %r0
+entry:
+  %sum = add i32 %x, %y
+  ret i32 %sum
+}

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:
+; CHECK: add r0, r0, r1
+; CHECK: bx lr
+entry:
+  %sum = add i32 %x, %y
+  ret i32 %sum
+}

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

+# RUN: llc -mtriple arm-- -global-isel -run-pass=legalizer %s -o - | FileCheck %s
+--- |
+  define void @test_add_s32() { ret void }
+...
+---
+name:            test_add_s32
+# CHECK-LABEL: name: test_add_s32
+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(s32) = COPY %r0
+    %1(s32) = COPY %r1
+    %2(s32) = G_ADD %0, %1
+    ; G_ADD with s32 is legal, so we should find it unchanged in the output
+    ; CHECK: {{%[0-9]+}}(s32) = G_ADD {{%[0-9]+, %[0-9]+}}
+    %r0 = COPY %2(s32)
+    BX_RET 14, _, implicit %r0
+
+...

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 }
+...
+---
+name:            test_add_s32
+# CHECK-LABEL: name: test_add_s32
+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(s32) = COPY %r0
+    %1(s32) = COPY %r1
+    %2(s32) = G_ADD %0, %1
+    %r0 = COPY %2(s32)
+    BX_RET 14, _, implicit %r0
+
+...