[PATCH] D41098: [InlineSpiller] Fix a crash due to lack of forward progress from remat

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet requested changes to this revision.
qcolombet added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/CodeGen/InlineSpiller.cpp:530
+  // but if find we need to, we should seriously consider separating remat as
+  // it's own phase in RegAllocGreedy.
+
----------------
Just to comment on the TODO, this code is actually not conservative as it assumes all the registers of the largest legal super class are a suitable assignment for the current register.
This is generally not true, since this won't take into account the constraints of the encoding of MI.

Anyhow, sketching a potential solution in the comment below.


================
Comment at: lib/CodeGen/InlineSpiller.cpp:543
+  const ArrayRef<MCPhysReg> AllocationOrder = RC->getRawAllocationOrder(MF);
+  if (AllocationOrder.size() > MI.getNumOperands())
+    // fastpath for common case
----------------
This is kind of a random check because:
1. Not all operands in MI are registers
2. Not all operands in MI have the same constraints and thus AllocationOrder may not be relevant for all of them


================
Comment at: lib/CodeGen/InlineSpiller.cpp:575
+      return false;
+  }
+  return true;
----------------
I believe the solution should look something like this:
const TargetRegisterClass *RelaxedRC = getLargestLegalSuperClassConstraintedOnMIUsage(MI, VReg) // look at getNumAllocatableRegsForConstraints in RegAllocGreedy

if (!RelaxedRC)
 return false; // Could be a fatal error because if we end up in this situation that means the allocation problem is not feasible

LiveRegUnits Used(TRI)
Used.accumulate(MI)

// Walk the registers in RelaxedRC and check if it exists one which has all its regunits not available
for (MCPhysReg PossibleReg : RelaxedRC) {
  if (MRI.isReserved(PossibleReg))
    continue;
  bool IsAvailable = true;
  for (MCRegUnitIterator Units(PossibleReg, TRI); Units.isValid(); ++Units)
     if (!Used.available(Units)) {
       IsAvailable = false;
      break;
    }
  if (IsAvailable)
    return true;
}
return false;


https://reviews.llvm.org/D41098