Add "llvm_unreachable" for function RegisterBankInfo::getRegBank

[skan]

Summary

RegClassOrBank is an object of RegClassOrRegBank, which is defined as

using llvm::RegClassOrRegBank = typedef PointerUnion<const TargetRegisterClass *, const RegisterBank *>
}

so control flow can not get here. Use ""llvm_unreachable" here to avoid "null pointer" confusion.

[skan]

add "const" to pointer type description.

[dsanders]

lib/CodeGen/GlobalISel/RegisterBankInfo.cpp

96 ↗

(On Diff #200203)

There's a typo here ('Reigister')

[skan]

fix the typo ("Register")

[skan]

lib/CodeGen/GlobalISel/RegisterBankInfo.cpp

96 ↗

(On Diff #200203)

fixed in the updated patch

[qcolombet]

llvm/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp

96

Sorry for the late reply but that doesn't seem right.

Yes, RegClassOrRegBank is of one of the two classes, but it can still be nullptr, e.g., before regbankselect. Therefore this case seems reachable to me.

What am I missing?

[skan]

RegClassOrRegBank is of one of the two classes, but it can still be nullptr, e.g., before regbankselect.
This patch has been reverted in llvm trunk.

llvm/lib/CodeGen/GlobalISel/RegisterBankInfo.cpp

if (TargetRegisterInfo::isPhysicalRegister(Reg))
     return &getRegBankFromRegClass(getMinimalPhysRegClass(Reg, TRI));
 
   assert(Reg && "NoRegister does not have a register bank");
   const RegClassOrRegBank &RegClassOrBank = MRI.getRegClassOrRegBank(Reg);
   if (auto *RB = RegClassOrBank.dyn_cast<const RegisterBank *>())
     return RB;
   if (auto *RC = RegClassOrBank.dyn_cast<const TargetRegisterClass *>())
     return &getRegBankFromRegClass(*RC);
-  return nullptr;
+
+  llvm_unreachable("RegClassOrBank is either a const RegisterBank* or "
+                   "a const TargetRegisterClass*");
 }
 
 const TargetRegisterClass &
 RegisterBankInfo::getMinimalPhysRegClass(unsigned Reg,
                                          const TargetRegisterInfo &TRI) const {
   assert(TargetRegisterInfo::isPhysicalRegister(Reg) &&
          "Reg must be a physreg");
   const auto &RegRCIt = PhysRegMinimalRCs.find(Reg);

llvm/lib/Target/X86/X86InstructionSelector.cpp

bool X86InstructionSelector::selectDivRem(MachineInstr &I,
   const unsigned DstReg = I.getOperand(0).getReg();
   const unsigned Op1Reg = I.getOperand(1).getReg();
   const unsigned Op2Reg = I.getOperand(2).getReg();
 
   const LLT RegTy = MRI.getType(DstReg);
   assert(RegTy == MRI.getType(Op1Reg) && RegTy == MRI.getType(Op2Reg) &&
          "Arguments and return value types must match");
 
-  const RegisterBank *RegRB = RBI.getRegBank(DstReg, MRI, TRI);
-  if (!RegRB || RegRB->getID() != X86::GPRRegBankID)
+  const RegisterBank &RegRB = *RBI.getRegBank(DstReg, MRI, TRI);
+  if (RegRB.getID() != X86::GPRRegBankID)
     return false;
 
   const static unsigned NumTypes = 4; // i8, i16, i32, i64
   const static unsigned NumOps = 4;   // SDiv, SRem, UDiv, URem
   const static bool S = true;         // IsSigned
   const static bool U = false;        // !IsSigned
   const static unsigned Copy = TargetOpcode::COPY;
   // For the X86 IDIV instruction, in most cases the dividend
bool X86InstructionSelector::selectDivRem(MachineInstr &I,
   case TargetOpcode::G_UREM:
     OpIndex = 3;
     break;
   }
 
   const DivRemEntry &TypeEntry = *OpEntryIt;
   const DivRemEntry::DivRemResult &OpEntry = TypeEntry.ResultTable[OpIndex];
 
-  const TargetRegisterClass *RegRC = getRegClass(RegTy, *RegRB);
+  const TargetRegisterClass *RegRC = getRegClass(RegTy, RegRB);
   if (!RBI.constrainGenericRegister(Op1Reg, *RegRC, MRI) ||
       !RBI.constrainGenericRegister(Op2Reg, *RegRC, MRI) ||
       !RBI.constrainGenericRegister(DstReg, *RegRC, MRI)) {
     LLVM_DEBUG(dbgs() << "Failed to constrain " << TII.getName(I.getOpcode())
                       << " operand\n");
     return false;
   }