[llvm-commits] [patch] Mostly mechanical removal of getPhysicalRegisterRegClass.patch

Jakob Stoklund Olesen stoklund at 2pi.dk
Wed Jun 9 23:03:44 PDT 2010


On Jun 9, 2010, at 9:27 PM, Rafael Espindola wrote:

> The attached patch is a bit ugly and evil, but is mostly a test that
> we can remove getPhysicalRegisterRegClass. It might actually be the
> lesser of two evils :-)

Thanks for working on this, Rafael.

The patch is a bit big, and I think you can safely break it up, switching gradually to getMinimalPhysRegClass(). Then remove getPhysicalRegisterRegClass when there are no more users.

The first bit is in isel, creating virtregs for copying to and from a physreg. These all use the VT argument, and you should preserve that. It makes a difference if XMM1 is spilled as FR32, FR64, or VR128. In these cases, the physreg doesn't actually need to belong to the chosen class, and an argument could be made for using TLI.getRegClassFor(VT) instead. We want a compatible regclass to allow coalescing, but if coalescing is not possible, we want the largest possible regclass. In any case, you want a regclass compatible with VT.

Second there are some changes to various copyRegToReg() methods. I don't think all of your changes are appropriate as these functions already deal with subclasses. I think is is better to just plug the holes in the current coverage, like ARM not handling SPR_8.

A function like the X86 getLoadRegOpcode looks like it needs a switch(RC->getID())

> The part that is ugly in it is that in some cases we compare register
> classes, but the register itself is not a hard register. In such cases
> we cannot use RC.contains(foo). That is why I added hasSubClassOrIs.

This is a good idea in general, but I don't think you can apply it mechanically to the *InstrInfo.cpp files.

/jakob





More information about the llvm-commits mailing list