GlobalISel: Improve crash on invalid mapping

[arsenm]

Summary

If NumBreakDowns is 0, BreakDown is null.
This trades a null dereference with an assert somewhere
else.

[bogner]

I don’t think I understand what this is trying to accomplish. ISTM that this will now hit a null dereference on line 123 when we read BreadDown in the case that NumBreakDowns is zero, but this is to avoid hitting an assert somewhere else? If this is to make it clearer as to what’s failing, shouldn’t we assert NumBreakDowns != 0 here instead of just hoping to crash on a null dereference?

[arsenm]

In D55694#1332254, @bogner wrote:

I don’t think I understand what this is trying to accomplish. ISTM that this will now hit a null dereference on line 123 when we read BreadDown in the case that NumBreakDowns is zero, but this is to avoid hitting an assert somewhere else? If this is to make it clearer as to what’s failing, shouldn’t we assert NumBreakDowns != 0 here instead of just hoping to crash on a null dereference?

No, that is what this avoids. Right now there is a null dereference on line 123. Now this function exits, so this continues until it hits
Assertion failed: (getOperandMapping(Idx).isValid() && "We must have a mapping for reg operands"), function verify, file ../lib/CodeGen/GlobalISel/RegisterBankInfo.cpp, line 572.

[bogner]

In D55694#1332646, @arsenm wrote:

In D55694#1332254, @bogner wrote:

I don’t think I understand what this is trying to accomplish. ISTM that this will now hit a null dereference on line 123 when we read BreadDown in the case that NumBreakDowns is zero, but this is to avoid hitting an assert somewhere else? If this is to make it clearer as to what’s failing, shouldn’t we assert NumBreakDowns != 0 here instead of just hoping to crash on a null dereference?

No, that is what this avoids. Right now there is a null dereference on line 123. Now this function exits, so this continues until it hits
Assertion failed: (getOperandMapping(Idx).isValid() && "We must have a mapping for reg operands"), function verify, file ../lib/CodeGen/GlobalISel/RegisterBankInfo.cpp, line 572.

Sorry, I must've read this diff backwards or something. LGTM.

[arsenm]

r349464

lib/CodeGen/GlobalISel/RegBankSelect.cpp

 
 bool RegBankSelect::assignmentMatch(
     unsigned Reg, const RegisterBankInfo::ValueMapping &ValMapping,
     bool &OnlyAssign) const {
   // By default we assume we will have to repair something.
   OnlyAssign = false;
   // Each part of a break down needs to end up in a different register.
   // In other word, Reg assignement does not match.
-  if (ValMapping.NumBreakDowns > 1)
+  if (ValMapping.NumBreakDowns != 1)
     return false;
 
   const RegisterBank *CurRegBank = RBI->getRegBank(Reg, *MRI, *TRI);
   const RegisterBank *DesiredRegBrank = ValMapping.BreakDown[0].RegBank;
   // Reg is free of assignment, a simple assignment will make the
   // register bank to match.
   OnlyAssign = CurRegBank == nullptr;
   LLVM_DEBUG(dbgs() << "Does assignment already match: ";