[llvm-dev] Relation between Register and MCRegister

Wed Sep 30 10:13:26 PDT 2020

On Tue, Sep 29, 2020 at 6:06 PM Daniel Sanders <daniel_l_sanders at apple.com>
wrote:

>
>
> On 29 Sep 2020, at 11:13, Mircea Trofin <mtrofin at google.com> wrote:
>
>
>
> On Tue, Sep 29, 2020 at 11:09 AM Daniel Sanders <
> 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).

Same for a 32 bit unsigned value.

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

Thanks!

>
> On 29 Sep 2020, at 11:02, Mircea Trofin <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> wrote:
>>
>>>
>>>
>>> On 29 Sep 2020, at 09:28, Quentin Colombet <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> wrote:
>>>
>>>
>>>
>>> On Tue, Sep 29, 2020 at 9:08 AM Quentin Colombet <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> 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
>>>> > 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/3081f783/attachment.html>