[llvm-dev] Relation between Register and MCRegister

Wed Sep 30 11:01:15 PDT 2020

> On 30 Sep 2020, at 10:13, Mircea Trofin <mtrofin at google.com> wrote:
> 
> 
> 
> On Tue, Sep 29, 2020 at 6:06 PM Daniel Sanders <daniel_l_sanders at apple.com <mailto:daniel_l_sanders at apple.com>> wrote:
> 
> 
>> On 29 Sep 2020, at 11:13, Mircea Trofin <mtrofin at google.com <mailto:mtrofin at google.com>> wrote:
>> 
>> 
>> 
>> On Tue, Sep 29, 2020 at 11:09 AM Daniel Sanders <daniel_l_sanders at apple.com <mailto:daniel_l_sanders at apple.com>> wrote:
>> Yes so long as you're including the invalid space too (IIRC it matters for DBG_VALUE in particular) the reason I didn't do that is that there's a lot more ctors than consumers of MCRegister. It seemed cheaper to do the checks when they're consumed and pretty much every consumer I encountered started with `assert(Reg.isPhysicalRegister() && ...)`.
>> 
>> Not sure I follow - asserts are elided in release builds - or is there a different cost?
> 
> Even though the release builds elide them there's still a cost to the compiler engineers using asserts builds for their daily development.
> 
> Just to make sure I'm seeing things the same way - converting a MCRegister to Register is correct; converting a Register to a MCRegister is not always correct. It's only correct if the Register's underlying unsigned value maps to the physical namespace (or 0). 

That's right.

> Same for a 32 bit unsigned value.

When converting 32-bit unsigned to MCRegister/Register, yes it's the same. When converting MCRegister/Register to unsigned we can't know if it's correct or not as it depends on how it's used later. In general, converting to unsigned is an indication that the consumer needs to adopt MCRegister/Register as appropriate for their usage.

> Does this fit in the design intent behind Register / MCRegister?

Yep

> Thanks!
>  
> 
>>> On 29 Sep 2020, at 11:02, Mircea Trofin <mtrofin at google.com <mailto:mtrofin at google.com>> wrote:
>>> 
>>> Thanks! To test my understanding - we could add asserts in MCRegister ctor that the value of the unsigned is, indeed, only in the physical register namespace, is that correct?
>>> 
>>> On Tue, Sep 29, 2020 at 10:49 AM Daniel Sanders <daniel_l_sanders at apple.com <mailto:daniel_l_sanders at apple.com>> wrote:
>>> 
>>> 
>>>> On 29 Sep 2020, at 09:28, Quentin Colombet <qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
>>>> 
>>>> + Daniel who added the MCRegister class.
>>>> 
>>>> Ah sorry, I replied too fast.
>>>> I mixed up MCPhysReg with MCRegister.
>>>> 
>>>> I was not aware we had such class.
>>>> 
>>>> From a look at it, MCRegister are essentially the same thing as Register. I am guessing that the difference is Register is used in the CodeGen layer, while MCRegister are used in the MC layer.
>>>> 
>>>> Until recently Register were just plain unsigned types and we introduced it to have stronger type checking. I don’t know what’s the rationale for MCRegister. It looks redundant to be honest. Maybe it is just solving a layering violation issue.
>>> 
>>> The rationale is the same but it's not just layering. MCRegister does not permit vregs or stack slots. However, it still needs to know what they are in order to ensure it never sees them (see below)
>>> 
>>> As for why MCPhysReg and MCRegister both exist. This is mostly down to lots of API's using unsigned while MCPhysReg provides some code size savings. We don't want to blindly truncate to MCPhysReg because accidentally passing vregs or stack slots will collapse them into physregs making them much harder to debug, particularly if we then zero-extend them again to use them in certain API's. However, we also don't want to widen MCPhysReg to unsigned because that will make the backend tables bigger and most backends don't really need the full range allocated to physical registers
>>> 
>>>> Let Daniel comment on that.
>>>> 
>>>> Cheers,
>>>> -Quentin
>>>> 
>>>>> On Sep 29, 2020, at 9:12 AM, Mircea Trofin <mtrofin at google.com <mailto:mtrofin at google.com>> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>> On Tue, Sep 29, 2020 at 9:08 AM Quentin Colombet <qcolombet at apple.com <mailto:qcolombet at apple.com>> wrote:
>>>>> Hi,
>>>>> 
>>>>> Register can represent virtual or physical registers.
>>>>> MCRegister can only represent physical registers.
>>>>> 
>>>>> That's what I thought, but MCRegister has some stack slot APIs. 
>>> 
>>> That's almost correct. MCRegisters can only be invalid ($noreg) or physical registers. The lack of support for vregs is indicated by the lack of any vreg API's while the lack of support for stack slots is indicated by the lack of all (except one) stack slot API's and is mentioned in a comment:
>>> 	"StackSlot values do not exist in the MC layer, see Register::isStackSlot() for the more information on them."
>>> 
>>> The reason there's still an MCRegister::isStackSlot() function despite that is an implementation detail. The checks for invalid and phys reg historically tested for ==0 and >0 respectively. At the same time conversion from Register to unsigned/MCRegister was just a simple cast so if stack slots ever managed to escape from the Register object into unsigned/MCRegister they'd be incorrectly converted from stack slot to physreg (and one that's out of range for MCPhysReg). We needed to be able to detect stack slots being inappropriately converted to MCRegister and the partitioning of the number space is the same as for Register so it made sense to use the same API instead of inventing a new one.
>>> 
>>>>> Eventually all Register instances are replaced by a MCRegister. 
>>>>> 
>>>>> What happens in that case to the stack slot APIs?
>>> 
>>> Registers that are stack slots are never converted to MCRegister. That's not enforced in the constructor but it is enforced in MCRegister::isPhysicalRegister()
>>> 
>>>>> Cheers,
>>>>> -Quentin
>>>>> 
>>>>> > On Sep 28, 2020, at 5:46 PM, Mircea Trofin via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>>>>> > 
>>>>> > Hello,
>>>>> > 
>>>>> > I'm trying to understand what the relation between these two types is: do we need them both? Register seems to be delegating to MCRegister without owning any new additional responsibilities.
>>> 
>>> Register provides everything related to vregs and stack slots.
>>> Since I last looked at these files it seems a bit more of the number space definitions have been moved to MCRegister by
>>> 	16dae81edc2 [NFCI] Cleanup range checks in Register/MCRegister
>>> I'd intentionally left the vreg bit in Register because vregs weren't a thing to MCRegister and that range was already excluded by the existing code but it makes sense to define the whole number space there so long as MCRegister continues to not supply the API's for kinds of register it doesn't deal with (aside from the one it needs internally to prohibit stack slots).
>>> 
>>> So to summarize the difference between MCRegister and Register:
>>> MCRegister defines the partitioning of the number space both for itself and for Register
>>> MCRegister provides support for invalid and phys regs
>>> Register is a superset of MCRegister and provides additional support for vreg and stack slots
>>> 
>>>>> > 
>>>>> > Thanks!
>>>>> > _______________________________________________
>>>>> > LLVM Developers mailing list
>>>>> > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>>>>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200930/cee823dc/attachment-0001.html>