[MIPS GlobalISel] Refactor MipsRegisterBankInfo [NFC]

[Petar.Avramovic]

Summary

Check if size of operand LLT matches sizes of available register banks
before inspecting the opcode in order to reduce number of checks.
Factor commonly used pieces of code into functions.

[atanasyan]

LGTM

lib/Target/Mips/MipsRegisterBankInfo.h

public:
   MipsRegisterBankInfo(const TargetRegisterInfo &TRI);
 
   const RegisterBank &
   getRegBankFromRegClass(const TargetRegisterClass &RC) const override;
 
   const InstructionMapping &
   getInstrMapping(const MachineInstr &MI) const override;
 
+  /// Here we have to narrowScalar s64 operands to s32, combine away G_MERGE or
+  /// G_UNMERGE and erase instructions that became dead in the process. We
+  /// manually assign bank to def operand of all new instructions that were
+  /// created in the process since they will not end up in RegBankSelect loop.
   void applyMappingImpl(const OperandsMapper &OpdMapper) const override;
 
+  /// RegBankSelect determined that s64 operand is better to be split into two
+  /// s32 operands in gprb. Here we manually set register banks of def operands
+  /// of newly created instructions since they will not get regbankselected.
+  void setRegBank(MachineInstr &MI, MachineRegisterInfo &MRI) const;
+
 private:
   /// Some instructions are used with both floating point and integer operands.
   /// We assign InstType to such instructions as it helps us to avoid cross bank
   /// copies. InstType deppends on context.
   enum InstType {
     /// Temporary type, when visit(..., nullptr) finishes will convert to one of
     /// the remaining types: Integer, FloatingPoint or Ambiguous.
     NotDetermined,

lib/Target/Mips/MipsRegisterBankInfo.cpp

 
 RegisterBankInfo::ValueMapping ValueMappings[] = {
     // invalid
     {nullptr, 0},
     // up to 3 operands in GPRs
     {&PartMappings[PMI_GPR - PMI_Min], 1},
     {&PartMappings[PMI_GPR - PMI_Min], 1},
     {&PartMappings[PMI_GPR - PMI_Min], 1},
-    // up to 3 ops operands FPRs - single precission
+    // up to 3 operands in FPRs - single precission
     {&PartMappings[PMI_SPR - PMI_Min], 1},
     {&PartMappings[PMI_SPR - PMI_Min], 1},
     {&PartMappings[PMI_SPR - PMI_Min], 1},
-    // up to 3 ops operands FPRs - double precission
+    // up to 3 operands in FPRs - double precission
     {&PartMappings[PMI_DPR - PMI_Min], 1},
     {&PartMappings[PMI_DPR - PMI_Min], 1},
     {&PartMappings[PMI_DPR - PMI_Min], 1}
 };
 
 } // end namespace Mips
 } // end namespace llvm
 
void MipsRegisterBankInfo::TypeInfoForMF::cleanupIfNewFunction(
     llvm::StringRef FunctionName) {
   if (MFName != FunctionName) {
     MFName = FunctionName;
     WaitingQueues.clear();
     Types.clear();
   }
 }
 
+static const MipsRegisterBankInfo::ValueMapping *getFprbMapping(unsigned Size) {
+  return Size == 32 ? &Mips::ValueMappings[Mips::SPRIdx]
+                    : &Mips::ValueMappings[Mips::DPRIdx];
+}
+
+static const unsigned CustomMappingID = 1;
+
+// Only 64 bit mapping is available in fprb and will be marked as custom, i.e.
+// will be split into two 32 bit registers in gprb.
+static const MipsRegisterBankInfo::ValueMapping *
+getGprbOrCustomMapping(unsigned Size, unsigned &MappingID) {
+  if (Size == 32)
+    return &Mips::ValueMappings[Mips::GPRIdx];
+
+  MappingID = CustomMappingID;
+  return &Mips::ValueMappings[Mips::DPRIdx];
+}
+
 const RegisterBankInfo::InstructionMapping &
 MipsRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
 
   static TypeInfoForMF TI;
 
   // Reset TI internal data when MF changes.
   TI.cleanupIfNewFunction(MI.getMF()->getName());
 
   unsigned Opc = MI.getOpcode();
   const MachineFunction &MF = *MI.getParent()->getParent();
   const MachineRegisterInfo &MRI = MF.getRegInfo();
 
   if (MI.getOpcode() != TargetOpcode::G_PHI) {
     const RegisterBankInfo::InstructionMapping &Mapping =
         getInstrMappingImpl(MI);
     if (Mapping.isValid())
       return Mapping;
   }
 
   using namespace TargetOpcode;
 
   unsigned NumOperands = MI.getNumOperands();
   const ValueMapping *OperandsMapping = &Mips::ValueMappings[Mips::GPRIdx];
   unsigned MappingID = DefaultMappingID;
-  const unsigned CustomMappingID = 1;
+
+  // Check if LLT sizes match sizes of available register banks.
+  for (const MachineOperand &Op : MI.operands()) {
+    if (Op.isReg()) {
+      LLT RegTy = MRI.getType(Op.getReg());
+
+      if (RegTy.isScalar() &&
+          (RegTy.getSizeInBits() != 32 && RegTy.getSizeInBits() != 64))
+        return getInvalidInstructionMapping();
+    }
+  }
+
+  const LLT Op0Ty = MRI.getType(MI.getOperand(0).getReg());
+  unsigned Op0Size = Op0Ty.getSizeInBits();
+  InstType InstTy = InstType::Integer;
 
   switch (Opc) {
   case G_TRUNC:
   case G_ADD:
   case G_SUB:
   case G_MUL:
   case G_UMULH:
   case G_ZEXTLOAD:
MipsRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
   case G_SDIV:
   case G_UDIV:
   case G_SREM:
   case G_UREM:
   case G_BRINDIRECT:
   case G_VASTART:
     OperandsMapping = &Mips::ValueMappings[Mips::GPRIdx];
     break;
-  case G_LOAD: {
-    unsigned Size = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits();
-    InstType InstTy = InstType::Integer;
-    if (!MRI.getType(MI.getOperand(0).getReg()).isPointer()) {
+  case G_STORE:
+  case G_LOAD:
+    if (!Op0Ty.isPointer())
       InstTy = TI.determineInstType(&MI);
-    }
 
     if (InstTy == InstType::FloatingPoint ||
-        (Size == 64 && InstTy == InstType::Ambiguous)) { // fprb
-      OperandsMapping =
-          getOperandsMapping({Size == 32 ? &Mips::ValueMappings[Mips::SPRIdx]
-                                         : &Mips::ValueMappings[Mips::DPRIdx],
-                              &Mips::ValueMappings[Mips::GPRIdx]});
-      break;
-    } else { // gprb
+        (Op0Size == 64 && InstTy == InstType::Ambiguous))
+      OperandsMapping = getOperandsMapping(
+          {getFprbMapping(Op0Size), &Mips::ValueMappings[Mips::GPRIdx]});
+    else
       OperandsMapping =
-          getOperandsMapping({Size <= 32 ? &Mips::ValueMappings[Mips::GPRIdx]
-                                         : &Mips::ValueMappings[Mips::DPRIdx],
+          getOperandsMapping({getGprbOrCustomMapping(Op0Size, MappingID),
                               &Mips::ValueMappings[Mips::GPRIdx]});
-      if (Size == 64)
-        MappingID = CustomMappingID;
-    }
 
     break;
-  }
-  case G_STORE: {
-    unsigned Size = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits();
-    InstType InstTy = InstType::Integer;
-    if (!MRI.getType(MI.getOperand(0).getReg()).isPointer()) {
+  case G_PHI:
+    if (!Op0Ty.isPointer())
       InstTy = TI.determineInstType(&MI);
-    }
-
-    if (InstTy == InstType::FloatingPoint ||
-        (Size == 64 && InstTy == InstType::Ambiguous)) { // fprb
-      OperandsMapping =
-          getOperandsMapping({Size == 32 ? &Mips::ValueMappings[Mips::SPRIdx]
-                                         : &Mips::ValueMappings[Mips::DPRIdx],
-                              &Mips::ValueMappings[Mips::GPRIdx]});
-      break;
-    } else { // gprb
-      OperandsMapping =
-          getOperandsMapping({Size <= 32 ? &Mips::ValueMappings[Mips::GPRIdx]
-                                         : &Mips::ValueMappings[Mips::DPRIdx],
-                              &Mips::ValueMappings[Mips::GPRIdx]});
-      if (Size == 64)
-        MappingID = CustomMappingID;
-    }
-    break;
-  }
-  case G_PHI: {
-    unsigned Size = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits();
-    InstType InstTy = InstType::Integer;
-    if (!MRI.getType(MI.getOperand(0).getReg()).isPointer()) {
-      InstTy = TI.determineInstType(&MI);
-    }
 
     // PHI is copylike and should have one regbank in mapping for def register.
-    if (InstTy == InstType::Integer && Size == 64) { // fprb
+    if (InstTy == InstType::Integer && Op0Size == 64) {
       OperandsMapping =
           getOperandsMapping({&Mips::ValueMappings[Mips::DPRIdx]});
       return getInstructionMapping(CustomMappingID, /*Cost=*/1, OperandsMapping,
                                    /*NumOperands=*/1);
     }
     // Use default handling for PHI, i.e. set reg bank of def operand to match
     // register banks of use operands.
-    const RegisterBankInfo::InstructionMapping &Mapping =
-        getInstrMappingImpl(MI);
-    return Mapping;
-  }
+    return getInstrMappingImpl(MI);
   case G_SELECT: {
-    unsigned Size = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits();
-    InstType InstTy = InstType::Integer;
-    if (!MRI.getType(MI.getOperand(0).getReg()).isPointer()) {
+    if (!Op0Ty.isPointer())
       InstTy = TI.determineInstType(&MI);
-    }
 
     if (InstTy == InstType::FloatingPoint ||
-        (Size == 64 && InstTy == InstType::Ambiguous)) { // fprb
-      const RegisterBankInfo::ValueMapping *Bank =
-          Size == 32 ? &Mips::ValueMappings[Mips::SPRIdx]
-                     : &Mips::ValueMappings[Mips::DPRIdx];
+        (Op0Size == 64 && InstTy == InstType::Ambiguous)) {
+      const RegisterBankInfo::ValueMapping *Bank = getFprbMapping(Op0Size);
       OperandsMapping = getOperandsMapping(
           {Bank, &Mips::ValueMappings[Mips::GPRIdx], Bank, Bank});
       break;
-    } else { // gprb
+    } else {
       const RegisterBankInfo::ValueMapping *Bank =
-          Size <= 32 ? &Mips::ValueMappings[Mips::GPRIdx]
-                     : &Mips::ValueMappings[Mips::DPRIdx];
+          getGprbOrCustomMapping(Op0Size, MappingID);
       OperandsMapping = getOperandsMapping(
           {Bank, &Mips::ValueMappings[Mips::GPRIdx], Bank, Bank});
-      if (Size == 64)
-        MappingID = CustomMappingID;
     }
     break;
   }
-  case G_IMPLICIT_DEF: {
-    unsigned Size = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits();
-    InstType InstTy = InstType::Integer;
-    if (!MRI.getType(MI.getOperand(0).getReg()).isPointer()) {
+  case G_IMPLICIT_DEF:
+    if (!Op0Ty.isPointer())
       InstTy = TI.determineInstType(&MI);
-    }
 
-    if (InstTy == InstType::FloatingPoint) { // fprb
-      OperandsMapping = Size == 32 ? &Mips::ValueMappings[Mips::SPRIdx]
-                                   : &Mips::ValueMappings[Mips::DPRIdx];
-    } else { // gprb
-      OperandsMapping = Size == 32 ? &Mips::ValueMappings[Mips::GPRIdx]
-                                   : &Mips::ValueMappings[Mips::DPRIdx];
-      if (Size == 64)
-        MappingID = CustomMappingID;
-    }
+    if (InstTy == InstType::FloatingPoint)
+      OperandsMapping = getFprbMapping(Op0Size);
+    else
+      OperandsMapping = getGprbOrCustomMapping(Op0Size, MappingID);
+
     break;
-  }
-  case G_UNMERGE_VALUES: {
+  case G_UNMERGE_VALUES:
     OperandsMapping = getOperandsMapping({&Mips::ValueMappings[Mips::GPRIdx],
                                           &Mips::ValueMappings[Mips::GPRIdx],
                                           &Mips::ValueMappings[Mips::DPRIdx]});
     MappingID = CustomMappingID;
     break;
-  }
-  case G_MERGE_VALUES: {
+  case G_MERGE_VALUES:
     OperandsMapping = getOperandsMapping({&Mips::ValueMappings[Mips::DPRIdx],
                                           &Mips::ValueMappings[Mips::GPRIdx],
                                           &Mips::ValueMappings[Mips::GPRIdx]});
     MappingID = CustomMappingID;
     break;
-  }
   case G_FADD:
   case G_FSUB:
   case G_FMUL:
   case G_FDIV:
   case G_FABS:
-  case G_FSQRT:{
-    unsigned Size = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits();
-    assert((Size == 32 || Size == 64) && "Unsupported floating point size");
-    OperandsMapping = Size == 32 ? &Mips::ValueMappings[Mips::SPRIdx]
-                                 : &Mips::ValueMappings[Mips::DPRIdx];
+  case G_FSQRT:
+    OperandsMapping = getFprbMapping(Op0Size);
     break;
-  }
-  case G_FCONSTANT: {
-    unsigned Size = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits();
-    assert((Size == 32 || Size == 64) && "Unsupported floating point size");
-    const RegisterBankInfo::ValueMapping *FPRValueMapping =
-        Size == 32 ? &Mips::ValueMappings[Mips::SPRIdx]
-                   : &Mips::ValueMappings[Mips::DPRIdx];
-    OperandsMapping = getOperandsMapping({FPRValueMapping, nullptr});
+  case G_FCONSTANT:
+    OperandsMapping = getOperandsMapping({getFprbMapping(Op0Size), nullptr});
     break;
-  }
   case G_FCMP: {
-    unsigned Size = MRI.getType(MI.getOperand(2).getReg()).getSizeInBits();
-    assert((Size == 32 || Size == 64) && "Unsupported floating point size");
-    const RegisterBankInfo::ValueMapping *FPRValueMapping =
-        Size == 32 ? &Mips::ValueMappings[Mips::SPRIdx]
-                   : &Mips::ValueMappings[Mips::DPRIdx];
+    unsigned Op2Size = MRI.getType(MI.getOperand(2).getReg()).getSizeInBits();
     OperandsMapping =
         getOperandsMapping({&Mips::ValueMappings[Mips::GPRIdx], nullptr,
-                            FPRValueMapping, FPRValueMapping});
+                            getFprbMapping(Op2Size), getFprbMapping(Op2Size)});
     break;
   }
   case G_FPEXT:
     OperandsMapping = getOperandsMapping({&Mips::ValueMappings[Mips::DPRIdx],
                                           &Mips::ValueMappings[Mips::SPRIdx]});
     break;
   case G_FPTRUNC:
     OperandsMapping = getOperandsMapping({&Mips::ValueMappings[Mips::SPRIdx],
                                           &Mips::ValueMappings[Mips::DPRIdx]});
     break;
   case G_FPTOSI: {
+    assert((Op0Size == 32) && "Unsupported integer size");
     unsigned SizeFP = MRI.getType(MI.getOperand(1).getReg()).getSizeInBits();
-    assert((MRI.getType(MI.getOperand(0).getReg()).getSizeInBits() == 32) &&
-           "Unsupported integer size");
-    assert((SizeFP == 32 || SizeFP == 64) && "Unsupported floating point size");
-    OperandsMapping = getOperandsMapping({
-        &Mips::ValueMappings[Mips::GPRIdx],
-        SizeFP == 32 ? &Mips::ValueMappings[Mips::SPRIdx]
-                     : &Mips::ValueMappings[Mips::DPRIdx],
-    });
+    OperandsMapping = getOperandsMapping(
+        {&Mips::ValueMappings[Mips::GPRIdx], getFprbMapping(SizeFP)});
     break;
   }
-  case G_SITOFP: {
-    unsigned SizeInt = MRI.getType(MI.getOperand(1).getReg()).getSizeInBits();
-    unsigned SizeFP = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits();
-    (void)SizeInt;
-    assert((SizeInt == 32) && "Unsupported integer size");
-    assert((SizeFP == 32 || SizeFP == 64) && "Unsupported floating point size");
-    OperandsMapping =
-        getOperandsMapping({SizeFP == 32 ? &Mips::ValueMappings[Mips::SPRIdx]
-                                         : &Mips::ValueMappings[Mips::DPRIdx],
-                            &Mips::ValueMappings[Mips::GPRIdx]});
+  case G_SITOFP:
+    assert((MRI.getType(MI.getOperand(1).getReg()).getSizeInBits() == 32) &&
+           "Unsupported integer size");
+    OperandsMapping = getOperandsMapping(
+        {getFprbMapping(Op0Size), &Mips::ValueMappings[Mips::GPRIdx]});
     break;
-  }
   case G_CONSTANT:
   case G_FRAME_INDEX:
   case G_GLOBAL_VALUE:
   case G_JUMP_TABLE:
   case G_BRCOND:
     OperandsMapping =
         getOperandsMapping({&Mips::ValueMappings[Mips::GPRIdx], nullptr});
     break;
public:
 
   void createdInstr(MachineInstr &MI) override { InstList.insert(&MI); }
   void erasingInstr(MachineInstr &MI) override {}
   void changingInstr(MachineInstr &MI) override {}
   void changedInstr(MachineInstr &MI) override {}
 };
 } // end anonymous namespace
 
-/// Here we have to narrowScalar s64 operands to s32, combine away
-/// G_MERGE/G_UNMERGE and erase instructions that became dead in the process.
-/// We manually assign 32 bit gprb to register operands of all new instructions
-/// that got created in the process since they will not end up in RegBankSelect
-/// loop. Careful not to delete instruction after MI i.e. MI.getIterator()++.
+void MipsRegisterBankInfo::setRegBank(MachineInstr &MI,
+                                      MachineRegisterInfo &MRI) const {
+  Register Dest = MI.getOperand(0).getReg();
+  switch (MI.getOpcode()) {
+  case TargetOpcode::G_STORE:
+    // No def operands, skip this instruction.
+    break;
+  case TargetOpcode::G_CONSTANT:
+  case TargetOpcode::G_LOAD:
+  case TargetOpcode::G_SELECT:
+  case TargetOpcode::G_PHI:
+  case TargetOpcode::G_IMPLICIT_DEF: {
+    assert(MRI.getType(Dest) == LLT::scalar(32) && "Unexpected operand type.");
+    MRI.setRegBank(Dest, getRegBank(Mips::GPRBRegBankID));
+    break;
+  }
+  case TargetOpcode::G_GEP: {
+    assert(MRI.getType(Dest).isPointer() && "Unexpected operand type.");
+    MRI.setRegBank(Dest, getRegBank(Mips::GPRBRegBankID));
+    break;
+  }
+  default:
+    llvm_unreachable("Unexpected opcode.");
+  }
+}
+
+static void
+combineAwayG_UNMERGE_VALUES(LegalizationArtifactCombiner &ArtCombiner,
+                            MachineInstr &MI) {
+  SmallVector<MachineInstr *, 2> DeadInstrs;
+  ArtCombiner.tryCombineMerges(MI, DeadInstrs);
+  for (MachineInstr *DeadMI : DeadInstrs)
+    DeadMI->eraseFromParent();
+}
+
 void MipsRegisterBankInfo::applyMappingImpl(
     const OperandsMapper &OpdMapper) const {
   MachineInstr &MI = OpdMapper.getMI();
   InstListTy NewInstrs;
   MachineIRBuilder B(MI);
   MachineFunction *MF = MI.getMF();
   MachineRegisterInfo &MRI = OpdMapper.getMRI();
+  const LegalizerInfo &LegInfo = *MF->getSubtarget().getLegalizerInfo();
 
   InstManager NewInstrObserver(NewInstrs);
   GISelObserverWrapper WrapperObserver(&NewInstrObserver);
   LegalizerHelper Helper(*MF, WrapperObserver, B);
-  LegalizationArtifactCombiner ArtCombiner(
-      B, MF->getRegInfo(), *MF->getSubtarget().getLegalizerInfo());
+  LegalizationArtifactCombiner ArtCombiner(B, MF->getRegInfo(), LegInfo);
 
   switch (MI.getOpcode()) {
   case TargetOpcode::G_LOAD:
   case TargetOpcode::G_STORE:
   case TargetOpcode::G_PHI:
   case TargetOpcode::G_SELECT:
   case TargetOpcode::G_IMPLICIT_DEF: {
     Helper.narrowScalar(MI, 0, LLT::scalar(32));
     // Handle new instructions.
     while (!NewInstrs.empty()) {
       MachineInstr *NewMI = NewInstrs.pop_back_val();
       // This is new G_UNMERGE that was created during narrowScalar and will
       // not be considered for regbank selection. RegBankSelect for mips
       // visits/makes corresponding G_MERGE first. Combine them here.
-      if (NewMI->getOpcode() == TargetOpcode::G_UNMERGE_VALUES) {
-        SmallVector<MachineInstr *, 2> DeadInstrs;
-        ArtCombiner.tryCombineMerges(*NewMI, DeadInstrs);
-        for (MachineInstr *DeadMI : DeadInstrs)
-          DeadMI->eraseFromParent();
-      }
+      if (NewMI->getOpcode() == TargetOpcode::G_UNMERGE_VALUES)
+        combineAwayG_UNMERGE_VALUES(ArtCombiner, *NewMI);
       // This G_MERGE will be combined away when its corresponding G_UNMERGE
       // gets regBankSelected.
       else if (NewMI->getOpcode() == TargetOpcode::G_MERGE_VALUES)
         continue;
       else
-        // Manually set register banks for all register operands to 32 bit gprb.
-        for (auto Op : NewMI->operands()) {
-          if (Op.isReg()) {
-            assert(MRI.getType(Op.getReg()).getSizeInBits() == 32 &&
-                   "Only 32 bit gprb is handled here.\n");
-            MRI.setRegBank(Op.getReg(), getRegBank(Mips::GPRBRegBankID));
-          }
-        }
+        // Manually set register banks for def operands to 32 bit gprb.
+        setRegBank(*NewMI, MRI);
     }
     return;
   }
-  case TargetOpcode::G_UNMERGE_VALUES: {
-    SmallVector<MachineInstr *, 2> DeadInstrs;
-    ArtCombiner.tryCombineMerges(MI, DeadInstrs);
-    for (MachineInstr *DeadMI : DeadInstrs)
-      DeadMI->eraseFromParent();
+  case TargetOpcode::G_UNMERGE_VALUES:
+    combineAwayG_UNMERGE_VALUES(ArtCombiner, MI);
     return;
-  }
   default:
     break;
   }
 
   return applyDefaultMapping(OpdMapper);
 }