llvm dev - Aug 2016 - RFC: Disambiguate RegClass->getSize()

*** Problem

The documentation for function "getSize" in both MCRegisterClass and 
TargetRegisterClass states:
   /// 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.

The problem is that these two values are not always the same. For 
example, Hexagon has predicate registers that are 8 bits long and vector 
predicate registers that are 64 or 128 bits long, but their spill slots 
are of different sizes. None of these registers can be spilled directly, 
and they have to be transferred into a spillable register first. These 
spillable registers are larger: 32 bits and 512/1024 bits respectively, 
and so getSize must return the larger value in order to handle spills 
correctly.

Different users of getSize may be interested in different aspects of the 
returned value: register allocator may care about the spill size while 
optimizations tracking the predicate values will be interested in the 
actual size of the register.

Moreover, if a register for which these two values differ had 
subregisters, the SubRegCoveredBits information could not be assumed to 
match the return value of getSize.


*** Proposal

Replace the getSize function in MCRegisterClass and TargetRegisterClass 
with two functions: getRegSize and getSpillSize.


TableGen would need to be modified to generate both of them, and unless 
the user specifies otherwise in a .td file, these values would be equal.

All users of getSize would need to be updated to call one of the new 
replacement functions. This can be done one step at the time: at first 
the old getSize would be present together with the new functions, it 
would be removed once all calls to it have been updated.



What does everyone think about it?


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

I had the impression that MCRegisterClass::getSize() is only ever used to
determine the size of the spillslots and it would indeed be a good idea to
rename it to something like getSpillSlotSize() (additionally this cannot be
confused with the number of register in a class as well).

Are you aware of any user that actually wants the number of bits in a register?
I am not sure we actually need that information at the generic codegen level. I
must admit I hear about SubRegCoveredBits for the first time now, its main use
at the moment is to output dwarf information and stack maps information. It also
seems aarch64 and an internal target get away with not setting this information
correctly. So far I assumed the lanemasks are the only tool to determine what
parts of a register is covered. What uses do you have in mind?

- Matthias
> On Aug 17, 2016, at 8:20 AM, Krzysztof Parzyszek via llvm-dev <llvm-dev
at lists.llvm.org> wrote:
> 
> *** Problem
> 
> The documentation for function "getSize" in both MCRegisterClass
and TargetRegisterClass states:
>  /// 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.
> 
> The problem is that these two values are not always the same. For example,
Hexagon has predicate registers that are 8 bits long and vector predicate
registers that are 64 or 128 bits long, but their spill slots are of different
sizes. None of these registers can be spilled directly, and they have to be
transferred into a spillable register first. These spillable registers are
larger: 32 bits and 512/1024 bits respectively, and so getSize must return the
larger value in order to handle spills correctly.
> 
> Different users of getSize may be interested in different aspects of the
returned value: register allocator may care about the spill size while
optimizations tracking the predicate values will be interested in the actual
size of the register.
> 
> Moreover, if a register for which these two values differ had subregisters,
the SubRegCoveredBits information could not be assumed to match the return value
of getSize.
> 
> 
> *** Proposal
> 
> Replace the getSize function in MCRegisterClass and TargetRegisterClass
with two functions: getRegSize and getSpillSize.
> 
> 
> TableGen would need to be modified to generate both of them, and unless the
user specifies otherwise in a .td file, these values would be equal.
> 
> All users of getSize would need to be updated to call one of the new
replacement functions. This can be done one step at the time: at first the old
getSize would be present together with the new functions, it would be removed
once all calls to it have been updated.
> 
> 
> 
> What does everyone think about it?
> 
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
by The Linux Foundation
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

On 08/17/2016 11:32 AM, Matthias Braun via llvm-dev
wrote:
> Are you aware of any user that actually wants the number of bits in a
register?
This is what we use RC->getSize() for in most places, but they are also 
the same.

-Matt

On 8/17/2016 1:32 PM, Matthias Braun wrote:
> I had the impression that MCRegisterClass::getSize() is only ever used to
determine the size of the spillslots and it would indeed be a good idea to
rename it to something like getSpillSlotSize() (additionally this cannot be
confused with the number of register in a class as well).
>
> Are you aware of any user that actually wants the number of bits in a
register? I am not sure we actually need that information at the generic codegen
level. I must admit I hear about SubRegCoveredBits for the first time now, its
main use at the moment is to output dwarf information and stack maps
information. It also seems aarch64 and an internal target get away with not
setting this information correctly. So far I assumed the lanemasks are the only
tool to determine what parts of a register is covered. What uses do you have in
mind?
Hexagon has a bit-tracker utility, whose core mechanism is 
target-independent. It keeps track of the values of each bit in all 
virtual registers. We then use it to simplify various instructions, like 
testbit, bitwise-and/or/xor, eliminate redundant instructions (like 
zero- or sign-extension of already extended values), etc.

Predicate registers on Hexagon can be used as a simple "true/false" 
value (where only the LSB is checked), or as a mask in vector 
operations, where each bit is significant. Right now, in the 
Hexagon-specific code we simply override the register size (calculated 
from RC::getSize) with what we know the real size is.
We don't yet handle vector predicate registers, but in the end it would 
be useful to do it, and to have a target-independent facility to check 
the register size instead of using getSize with hacks for known special 
cases.

If the bit-tracker was to be utilized by other targets, this would be 
something needed for it to work.


Do you think there are any downsides to making the register size known?


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