[llvm-commits] [patch] Refactor getCalleeSavedRegClasses and getCalleeSavedRegs

[Jakob Stoklund Olesen]

On Jun 1, 2010, at 6:29 PM, Rafael Espindola wrote:

>> Any chance you could use the existing getPhysicalRegisterRegClass hook instead?
> 
> Not sure if that would work. The comments say
> 
>  // Pick the most super register class of the right type that contains
>  // this physreg.
> 
> So for ARM thumb1 this would return GPR and not tGPR for R4, right?

Yes, except ARM and Blackfin override this method to return the LEAST super register class...

They do this because getPhysicalRegisterRegClass is mostly used to produce an argument for copyRegToReg() when copying a physreg, and then it is important to get tGPR instead of GPR.

ARM and Blackfin are the only targets where it actually matters.

The function is evil because there is no right answer - a register can be in multiple register classes. It is double evil because the answer is usually used to pass back to the target anyway. The target already knows all it needs to know about R4.

It is the same nonsense with PEI - it just needs something to pass back into storeRegToStackSlot() / loadRegFromStackSlot().

The really dangerous bit here is PowerPC. It has callee-saved floating point registers, and it makes a difference if they are spilled as floats or doubles. Again, the LEAST super register class would be the right choice (F8RC is a subclass of F4RC, based on the substitution principle).


I think the definition of getPhysicalRegisterRegClass() is a mistake, it should return the LEAST super register class containing reg. Let's look at the uses:

AggressiveAntiDepBreaker uses it to find rename candidates. The largest register class is not guaranteed to satisfy all instructions using the register, so that could be a bug. The smallest register class containing reg would be fine.

LowerSubregs, SelectionDAG/InstrEmitter use it to set up arguments for copyRegToReg. The smaller class is more specific. This is why ARM overrides the function.

RegAllocPBQP uses it to determine if a physreg is allocatable. Either way is fine.

The schedulers use it for a getCrossCopyRegClass argument. More specific is better.

VirtRegRewriter uses it to allocate an emergency spill slot. More specific is better.


I think you should change getPhysicalRegisterRegClass to return the SMALLEST register class containing Reg, and use that. Then we can remove the overloads in ARM and Blackfin.

/jakob