[llvm] r374870 - [MIPS GlobalISel] Refactor MipsRegisterBankInfo [NFC]

Petar Avramovic via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 15 02:18:42 PDT 2019


Author: petar.avramovic
Date: Tue Oct 15 02:18:42 2019
New Revision: 374870

URL: http://llvm.org/viewvc/llvm-project?rev=374870&view=rev
Log:
[MIPS GlobalISel] Refactor MipsRegisterBankInfo [NFC]

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.

Differential Revision: https://reviews.llvm.org/D68866

Modified:
    llvm/trunk/lib/Target/Mips/MipsRegisterBankInfo.cpp
    llvm/trunk/lib/Target/Mips/MipsRegisterBankInfo.h

Modified: llvm/trunk/lib/Target/Mips/MipsRegisterBankInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsRegisterBankInfo.cpp?rev=374870&r1=374869&r2=374870&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Mips/MipsRegisterBankInfo.cpp (original)
+++ llvm/trunk/lib/Target/Mips/MipsRegisterBankInfo.cpp Tue Oct 15 02:18:42 2019
@@ -50,11 +50,11 @@ RegisterBankInfo::ValueMapping ValueMapp
     {&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}
@@ -355,6 +355,24 @@ void MipsRegisterBankInfo::TypeInfoForMF
   }
 }
 
+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 {
 
@@ -379,7 +397,21 @@ MipsRegisterBankInfo::getInstrMapping(co
   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:
@@ -406,64 +438,27 @@ MipsRegisterBankInfo::getInstrMapping(co
   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()) {
-      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()) {
+  case G_PHI:
+    if (!Op0Ty.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,
@@ -471,98 +466,63 @@ MipsRegisterBankInfo::getInstrMapping(co
     }
     // 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:
@@ -574,29 +534,18 @@ MipsRegisterBankInfo::getInstrMapping(co
                                           &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:
@@ -639,11 +588,41 @@ public:
 };
 } // 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();
@@ -651,12 +630,12 @@ void MipsRegisterBankInfo::applyMappingI
   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:
@@ -671,35 +650,21 @@ void MipsRegisterBankInfo::applyMappingI
       // 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;
   }

Modified: llvm/trunk/lib/Target/Mips/MipsRegisterBankInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsRegisterBankInfo.h?rev=374870&r1=374869&r2=374870&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Mips/MipsRegisterBankInfo.h (original)
+++ llvm/trunk/lib/Target/Mips/MipsRegisterBankInfo.h Tue Oct 15 02:18:42 2019
@@ -38,8 +38,17 @@ public:
   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




More information about the llvm-commits mailing list