On 05/14/2012 02:17 PM, Jakob Stoklund Olesen wrote:> On May 14, 2012, at 1:02 PM, reed kotler wrote: > >> Does anyone understand the purpose of : >> >> TargetRegisterInfo::getMinimalPhysRegClass ??? > Barely. > >> Why is there the presumption to use the minimal subclass? > The function can be traced back to a time when men were men and registers belonged to ONE register class. That concept doesn't make sense any longer, as LLVM supports and aggressively uses overlapping register classes. > > I changed the meaning of the function to be 'the most specific register class containing Reg' which at least attempts to assign some meaning to a unique answer. > > In general, there isn't a good answer to "What is the register class of R?" > >> I want to introduct a different register class for MIPS 16 but don't >> want it to chose MIPS 16 when >> I'm compiling for MIPS 32. > What exactly are you trying to do? The getMinimalPhysRegClass hook shouldn't be used for much these days. The most prominent client is PEI spilling callee-saved registers. > > The register allocator is generally assuming that sub-classes of legal register classes are usable. That is a pervasive assumption. > > /jakob >I'm not using getMinimalPhysRegClass. Some target independent code is using it. It makes trouble for us and I would like to submit a patch to make it a virtual function so that I can override it and make it meaningful for Mips, as long as this method still exists. I want to add another register class for Mips16 and don't want to define a Mips16 set of registers because in reality there is no such thing; MIPS16 is an application extension that can exist for either Mips32 or Mips64 which uses a different instruction encoding. When I'm compiling for -mips23 -nomips16 I don't want the mips16 register class being passed to any functions which take such a register class parameter. As it is right now, it sees mips16 as the minimal size class and passes it when I'm compiling for -mips32 -nomips16
On May 14, 2012, at 2:28 PM, reed kotler wrote:> I'm not using getMinimalPhysRegClass. Some target independent code is using it.Probably PEI.> It makes trouble for us and I would like to submit a patch to make it a virtual function so that I can override it and make it meaningful for Mips, as long as this method still exists. > > I want to add another register class for Mips16 and don't want to define a Mips16 set of registers because in reality there is no such thing; MIPS16 is an application extension that can exist for either Mips32 or Mips64 which uses a different instruction encoding. > > When I'm compiling for -mips23 -nomips16 I don't want the mips16 register class being passed to any functions which take such a register class parameter. > > As it is right now, it sees mips16 as the minimal size class and passes it when I'm compiling for -mips32 -nomips16The ARM tGPR register class is the same. It has no business showing up in non-Thumb code, but it is completely harmless when it does. My best advice to you is don't try to swim upstream. The Liskov substitution principle for register classes is deeply ingrained in the LLVM register allocators. /jakob
On 05/14/2012 02:42 PM, Jakob Stoklund Olesen wrote:> On May 14, 2012, at 2:28 PM, reed kotler wrote: > >> I'm not using getMinimalPhysRegClass. Some target independent code is using it. > Probably PEI. > >> It makes trouble for us and I would like to submit a patch to make it a virtual function so that I can override it and make it meaningful for Mips, as long as this method still exists. >> >> I want to add another register class for Mips16 and don't want to define a Mips16 set of registers because in reality there is no such thing; MIPS16 is an application extension that can exist for either Mips32 or Mips64 which uses a different instruction encoding. >> >> When I'm compiling for -mips23 -nomips16 I don't want the mips16 register class being passed to any functions which take such a register class parameter. >> >> As it is right now, it sees mips16 as the minimal size class and passes it when I'm compiling for -mips32 -nomips16 > The ARM tGPR register class is the same. It has no business showing up in non-Thumb code, but it is completely harmless when it does. > > My best advice to you is don't try to swim upstream. The Liskov substitution principle for register classes is deeply ingrained in the LLVM register allocators. > > /jakob >What is the danger of overriding the getMinimalPhysRegClass ? Well, now if it's being changed I have to change my version too. But with MIPS, we have no register subclasses so I can just return the one I want with no danger of it interfering. The other approach is to just ignore the register class parameter which is not much different than overriding the virtual function, if getMinimalPhysRegClass where a virtual function.
On 05/14/2012 02:42 PM, Jakob Stoklund Olesen wrote:> On May 14, 2012, at 2:28 PM, reed kotler wrote: > >> I'm not using getMinimalPhysRegClass. Some target independent code is using it. > Probably PEI. > >> It makes trouble for us and I would like to submit a patch to make it a virtual function so that I can override it and make it meaningful for Mips, as long as this method still exists. >> >> I want to add another register class for Mips16 and don't want to define a Mips16 set of registers because in reality there is no such thing; MIPS16 is an application extension that can exist for either Mips32 or Mips64 which uses a different instruction encoding. >> >> When I'm compiling for -mips23 -nomips16 I don't want the mips16 register class being passed to any functions which take such a register class parameter. >> >> As it is right now, it sees mips16 as the minimal size class and passes it when I'm compiling for -mips32 -nomips16 > The ARM tGPR register class is the same. It has no business showing up in non-Thumb code, but it is completely harmless when it does. > > My best advice to you is don't try to swim upstream. The Liskov substitution principle for register classes is deeply ingrained in the LLVM register allocators. > > /jakob >If I submit this patch, is it going to be rejected: iff --git a/include/llvm/Target/TargetRegisterInfo.h b/include/llvm/Target/Targ index 7e73db3..2262715 100644 --- a/include/llvm/Target/TargetRegisterInfo.h +++ b/include/llvm/Target/TargetRegisterInfo.h @@ -298,7 +298,7 @@ public: /// getMinimalPhysRegClass - Returns the Register Class of a physical /// register of the given type, picking the most sub register class of /// the right type that contains this physreg. - const TargetRegisterClass * + virtual const TargetRegisterClass * getMinimalPhysRegClass(unsigned Reg, EVT VT = MVT::Other) const; /// getAllocatableClass - Return the maximal subclass of the given register ( ???
On 05/14/2012 02:42 PM, Jakob Stoklund Olesen wrote:> On May 14, 2012, at 2:28 PM, reed kotler wrote: > >> I'm not using getMinimalPhysRegClass. Some target independent code is using it. > Probably PEI. > >> It makes trouble for us and I would like to submit a patch to make it a virtual function so that I can override it and make it meaningful for Mips, as long as this method still exists. >> >> I want to add another register class for Mips16 and don't want to define a Mips16 set of registers because in reality there is no such thing; MIPS16 is an application extension that can exist for either Mips32 or Mips64 which uses a different instruction encoding. >> >> When I'm compiling for -mips23 -nomips16 I don't want the mips16 register class being passed to any functions which take such a register class parameter. >> >> As it is right now, it sees mips16 as the minimal size class and passes it when I'm compiling for -mips32 -nomips16 > The ARM tGPR register class is the same. It has no business showing up in non-Thumb code, but it is completely harmless when it does. > > My best advice to you is don't try to swim upstream. The Liskov substitution principle for register classes is deeply ingrained in the LLVM register allocators. > > /jakob >I guess I can just fix the problem with: if ((RC == &Mips::CPU16RegsRegClass) && !TM.getSubtargetImpl()->inMips16Mode()) RC = &Mips::CPURegsRegClass;