[PATCH] D14477: Remove unnecessary call to getAllocatableRegClass

[Quentin Colombet via llvm-commits]

qcolombet accepted this revision.
qcolombet added a comment.
This revision is now accepted and ready to land.

Hi Matt,

Thanks for the clarifications.

> This is modeled with the VS_32/VS_64 register classes, which I want to make unallocatable because currently they influence pressure heuristics even though there should never be a virtual register with these classes.


That’s interesting. If all you virtual registers can go either to the V or S bank, that sounds to me like you would want the VS register classes to be actually allocatable. Indeed, you want to take the full advantage of all the space you have when you split a live-range. E.g., say you run out of register in S, after splitting you’ll have to spill whereas you could have use the VS class to fall back on a V register!

Anyway, this is orthogonal to the problem.

> getAllocatableRegClass on the VS_32 superset will just pick one or the other. Based on the ordering scheme, register classes with more registers are ordered first, so this will always select the VGPR allocatable subset.


Okay, I see how we can end up in this situation now. Please proceed with the change, but add the suggested assertion. I don’t see how this could be wrong but if it does, I want we know it!.

Cheers,
-Quentin


================
Comment at: lib/CodeGen/SelectionDAG/InstrEmitter.cpp:342
@@ -340,3 +341,3 @@
       VReg = NewVReg;
     }
   }
----------------
Add:
else assert(OpRC.isAllocatable() && “Constraining an allocatable VReg produced an unallocatable class?!");


http://reviews.llvm.org/D14477