GlobalISel: Split main function of RegBankSelect up

[arsenm]

Summary

This will allow for easier overriding of the pass.

[rovka]

Smaller functions FTW, regardless of whether or not we ever override them :)

[qcolombet]

LGTM

llvm/include/llvm/CodeGen/GlobalISel/RegBankSelect.h

645

+1 :D

llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp

726

Nit: I would remove the NDEBUG guards here.

In this patch the caller is already doing that and longer term if other users appear, I would expect that they would want to actually check something (or wrap that in their own guards, e.g., assert).

[arsenm]

c1fc5219cb23d1e14c0115381321f6d8d6a4fc17

llvm/include/llvm/CodeGen/GlobalISel/RegBankSelect.h

void switchTo(RepairingKind NewKind) {
       InsertPoints.clear();
       CanMaterialize = NewKind != RepairingKind::Impossible;
       HasSplit = false;
       assert(NewKind != RepairingKind::Insert &&
              "We would need more MI to switch to Insert");
     }
   };
 
-private:
+protected:
   /// Helper class used to represent the cost for mapping an instruction.
   /// When mapping an instruction, we may introduce some repairing code.
   /// In most cases, the repairing code is local to the instruction,
   /// thus, we can omit the basic block frequency from the cost.
   /// However, some alternatives may produce non-local cost, e.g., when
   /// repairing a phi, and thus we then need to scale the local cost
   /// to the non-local cost. This class does this for us.
   /// \note: We could simply always scale the cost. The problem is that
return MachineFunctionProperties().set(
         MachineFunctionProperties::Property::RegBankSelected);
   }
 
   MachineFunctionProperties getClearedProperties() const override {
     return MachineFunctionProperties()
       .set(MachineFunctionProperties::Property::NoPHIs);
   }
 
+  /// Check that our input is fully legal: we require the function to have the
+  /// Legalized property, so it should be.
+  ///
+  /// FIXME: This should be in the MachineVerifier.
+  bool checkFunctionIsLegal(MachineFunction &MF) const;
+
   /// Walk through \p MF and assign a register bank to every virtual register
   /// that are still mapped to nothing.
   /// The target needs to provide a RegisterBankInfo and in particular
   /// override RegisterBankInfo::getInstrMapping.
   ///
   /// Simplified algo:
   /// \code
   ///   RBI = MF.subtarget.getRegBankInfo()
   ///   MIRBuilder.setMF(MF)
   ///   for each bb in MF
   ///     for each inst in bb
   ///       MIRBuilder.setInstr(inst)
   ///       MappingCosts = RBI.getMapping(inst);
   ///       Idx = findIdxOfMinCost(MappingCosts)
   ///       CurRegBank = MappingCosts[Idx].RegBank
   ///       MRI.setRegBank(inst.getOperand(0).getReg(), CurRegBank)
   ///       for each argument in inst
   ///         if (CurRegBank != argument.RegBank)
   ///           ArgReg = argument.getReg()
   ///           Tmp = MRI.createNewVirtual(MRI.getSize(ArgReg), CurRegBank)
   ///           MIRBuilder.buildInstr(COPY, Tmp, ArgReg)
   ///           inst.getOperand(argument.getOperandNo()).setReg(Tmp)
   /// \endcode
+  bool assignRegisterBanks(MachineFunction &MF);
+
   bool runOnMachineFunction(MachineFunction &MF) override;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_CODEGEN_GLOBALISEL_REGBANKSELECT_H

llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp

bool RegBankSelect::assignInstr(MachineInstr &MI) {
 
   LLVM_DEBUG(dbgs() << "Best Mapping: " << *BestMapping << '\n');
 
   // After this call, MI may not be valid anymore.
   // Do not use it.
   return applyMapping(MI, *BestMapping, RepairPts);
 }
 
-bool RegBankSelect::runOnMachineFunction(MachineFunction &MF) {
-  // If the ISel pipeline failed, do not bother running that pass.
-  if (MF.getProperties().hasProperty(
-          MachineFunctionProperties::Property::FailedISel))
-    return false;
-
-  LLVM_DEBUG(dbgs() << "Assign register banks for: " << MF.getName() << '\n');
-  const Function &F = MF.getFunction();
-  Mode SaveOptMode = OptMode;
-  if (F.hasOptNone())
-    OptMode = Mode::Fast;
-  init(MF);
-
-#ifndef NDEBUG
-  // Check that our input is fully legal: we require the function to have the
-  // Legalized property, so it should be.
-  // FIXME: This should be in the MachineVerifier.
-  if (!DisableGISelLegalityCheck)
-    if (const MachineInstr *MI = machineFunctionIsIllegal(MF)) {
-      reportGISelFailure(MF, *TPC, *MORE, "gisel-regbankselect",
-                         "instruction is not legal", *MI);
-      return false;
-    }
-#endif
-
+bool RegBankSelect::assignRegisterBanks(MachineFunction &MF) {
   // Walk the function and assign register banks to all operands.
   // Use a RPOT to make sure all registers are assigned before we choose
   // the best mapping of the current instruction.
   ReversePostOrderTraversal<MachineFunction*> RPOT(&MF);
   for (MachineBasicBlock *MBB : RPOT) {
     // Set a sensible insertion point so that subsequent calls to
     // MIRBuilder.
     MIRBuilder.setMBB(*MBB);
while (!WorkList.empty()) {
       if (!assignInstr(MI)) {
         reportGISelFailure(MF, *TPC, *MORE, "gisel-regbankselect",
                            "unable to map instruction", MI);
         return false;
       }
     }
   }
 
+  return true;
+}
+
+bool RegBankSelect::checkFunctionIsLegal(MachineFunction &MF) const {
+#ifndef NDEBUG
+  if (!DisableGISelLegalityCheck) {
+    if (const MachineInstr *MI = machineFunctionIsIllegal(MF)) {
+      reportGISelFailure(MF, *TPC, *MORE, "gisel-regbankselect",
+                         "instruction is not legal", *MI);
+      return false;
+    }
+  }
+#endif
+
+  return true;
+}
+
+bool RegBankSelect::runOnMachineFunction(MachineFunction &MF) {
+  // If the ISel pipeline failed, do not bother running that pass.
+  if (MF.getProperties().hasProperty(
+          MachineFunctionProperties::Property::FailedISel))
+    return false;
+
+  LLVM_DEBUG(dbgs() << "Assign register banks for: " << MF.getName() << '\n');
+  const Function &F = MF.getFunction();
+  Mode SaveOptMode = OptMode;
+  if (F.hasOptNone())
+    OptMode = Mode::Fast;
+  init(MF);
+
+#ifndef NDEBUG
+  if (!checkFunctionIsLegal(MF))
+    return false;
+#endif
+
+  assignRegisterBanks(MF);
+
   OptMode = SaveOptMode;
   return false;
 }
 
 //------------------------------------------------------------------------------
 //                  Helper Classes Implementation
 //------------------------------------------------------------------------------
 RegBankSelect::RepairingPlacement::RepairingPlacement(