[llvm-dev] MCRegisterClass::getSize() - spill size vs reg size

[Krzysztof Parzyszek via llvm-dev]

Hi Björn,

This is a known issue.  I plan to address it in the implementation of
http://lists.llvm.org/pipermail/llvm-dev/2016-September/105027.html

-Krzysztof

On 10/17/2016 7:13 AM, Björn Pettersson A via llvm-dev wrote:
> In MCRegisterClass (and TargetRegisterClass) there is a method getSize.
>
> According to the comments it should return both the size of “the
> register” (I assume that means “any register in the class”)
>
> which is assumed to be the same as the size needed on the stack when
> spilling such a register.
>
>
>
> The definition looks like this:
>
>
>
>   /// getSize - Return the size of the register in bytes, which is also
> the size
>
>   /// of a stack slot allocated to hold a spilled copy of this register.
>
>   unsigned getSize() const { return RegSize; }
>
>
>
>
>
> To me it seems like the primary use of getSize has been to get the
> SpillSize, but some users
>
> (GlobalISel?) also use this to get the register size. Note that the
> variable actually is called
>
> SpillSize in tablegen (CodeGenRegisters/RegisterInfoEmitter). But for
> some reason the
>
> MCRegisterClass renames it to RegSize and claims that it also denotes
> the register size.
>
>
>
>
>
> Some problems I see:
>
> a)       Is always register size and spill size the same?
> (It is not for my downstream, out-of-tree, target.)
>
> b)      Is always the register size in bits a multiple of the byte size?
> No, lots of targets implement 1-bit registers. So what is the size in
> bytes for such a register?
>
> c)       When defining register classes in <Target>RegisterInfo.td the
> spill size can be given explicitly
> (by using “Size = <size in bits>”). If not given (or rather if Size=0),
> then the SpillSize is derived
>
> from the first value type that is mapped to the register class.
> In tablegen this size is denoted SpillSize and it is given in bits. But
> tablegen
> will convert this to bytes by simply dividing by 8 (remainder is truncated).
>
> So, for example, one bit registers (exists for several targets) will get
> a spill size of zero bytes,
> unless Size is explicitly set.
>
> This is not easy to understand. Neither when looking at the
> RegisterClass definitions, nor as
> a user of MCRegisterClass::getSize (which according to the comment
> should return both size
> of register and spill size). Well, a 1-bit register is not a full byte,
> so maybe it is obvious that
>
> the size in bytes is zero?
>
>
>
>
>
> Some examples:
>
>
>
> PPC has a register class CRBITRC that has one bit registers, but it sets
> Size=32.
> So for this register class MCRegisterClass::getSize() will return 4
> bytes. So that is the spill size,
> but if a user of getSize() expects to get the register size it is
> definitely not the same as the spill size!
>
>
>
> Hexagon has the UsrBits register class with one bit registers. It does
> not set Size, but it sets
>
> isAllocatable=0 so it will not be used for virtual register (and thus
> never spilled). So a derived spill
>
> size of 0 is probably expected. But still, is it expected that the size
> of the register is 0 bytes, or
>
> do we want to round up the size to 1 byte?
>
>
>
>
>
> One idea to get this more general, and less confusing, could be as follows:
>
> 1)      Let tablegen save both SpillSize and RegSize for each register
> class.
> We should preferably keep the size in bits here. At least for the
> RegSize. This makes it possible for a user
> of the information to determine by itself it size should be rounded
> up/down if a conversion to bytes is needed.
>
> 2)      Add new methods in MCRegisterClass (and TargetRegisterClass) to
> getSpillSize() and getRegSizeInBits().
>
> 3)      Tablegen could assert that spill size is a multiple of the byte
> size.
>
> 4)      Mark the old getSize() as deprecated (and let it return the
> spill size until it can be removed).
>
> 5)      Start replacing uses of getSize() by the new methods, depending
> on if the register size or the spill size is of interest.
>
>
>
> Another improvement would be to rename Size to SpillSize when defining
> register classes in <Target>RegisterInfo.td files.
>
> If SpillSize isn’t explicitly given it should be derived from the value
> type. If it is explicitly given it should be allowed to set it
>
> to zero wihout tablegen replacing it by deriving size from value type.
>
>
>
> Any comments on this are welcome!
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
hosted by The Linux Foundation