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

[Jakob Stoklund Olesen]

On Jun 1, 2010, at 9:11 PM, Rafael Espindola wrote:

>> 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).
> 
> Thanks a lot for the info! There is one more case that causes
> problems: win64. It defines some xmm registers to be callee saved. We
> have the classes FR32, FR64 and VR12. They all contain the same
> registers.

They should spill as VR128, and again that is the ultimate subclass:

  // FR32 Register Class sub-classes...
  static const TargetRegisterClass* const FR32Subclasses[] = {
    &X86::FR64RegClass, &X86::VR128RegClass, NULL
  };

  // FR64 Register Class sub-classes...
  static const TargetRegisterClass* const FR64Subclasses[] = {
    &X86::VR128RegClass, NULL
  };

TableGen is doing this on purpose. See RegisterInfoEmitter.cpp:

        // RC2 is a sub-class of RC if it is a valid replacement for any
        // instruction operand where an RC register is required. It must satisfy
        // these conditions:
        //
        // 1. All RC2 registers are also in RC.
        // 2. The RC2 spill size must not be smaller that the RC spill size.
        // 3. RC2 spill alignment must be compatible with RC.
        //
        // Sub-classes are used to determine if a virtual register can be used
        // as an instruction operand, or if it must be copied first.

        if (rc == rc2 || RC2.Elements.size() > RC.Elements.size() ||
            (RC.SpillAlignment && RC2.SpillAlignment % RC.SpillAlignment) ||
            RC.SpillSize > RC2.SpillSize || !isSubRegisterClass(RC2, RegSet))
          continue;


> getPhysicalRegisterRegClass is truly evil! The ARM backend partially
> overrides it. It still expects the old behaviour for floating point
> registers.
> 
> Thanks a lot for the pointers. I have some ideas on how to improve
> things, will give it a try tomorrow and post a new patch if it works.

I expect it to 'just work' if you change the definition of getPhysicalRegisterRegClass, but testing is in order, of course. Perhaps a new name is a good idea as well? getMinimalPhysRegClass?

The function has been around for a long time, and at some point a register was only allowed to exist in one register class.

/jakob