<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On 30 Sep 2020, at 10:13, Mircea Trofin <<a href="mailto:mtrofin@google.com" class="">mtrofin@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta charset="UTF-8" class=""><br class="Apple-interchange-newline"><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><div class="gmail_quote" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><div dir="ltr" class="gmail_attr">On Tue, Sep 29, 2020 at 6:06 PM Daniel Sanders <<a href="mailto:daniel_l_sanders@apple.com" class="">daniel_l_sanders@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div style="overflow-wrap: break-word;" class=""><br class=""><div class=""><br class=""><blockquote type="cite" class=""><div class="">On 29 Sep 2020, at 11:13, Mircea Trofin <<a href="mailto:mtrofin@google.com" target="_blank" class="">mtrofin@google.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class=""><div dir="ltr" class=""><br class=""></div><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Sep 29, 2020 at 11:09 AM Daniel Sanders <<a href="mailto:daniel_l_sanders@apple.com" target="_blank" class="">daniel_l_sanders@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div class="">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() && ...)`.<br class=""></div></blockquote><div class=""><br class=""></div><div class="">Not sure I follow - asserts are elided in release builds - or is there a different cost?</div></div></div></div></blockquote><div class=""><br class=""></div><div class="">Even though the release builds elide them there's still a cost to the compiler engineers using asserts builds for their daily development.</div></div></div></blockquote><div class=""><br class=""></div><div class="">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). </div></div></div></blockquote><div><br class=""></div>That's right.<br class=""><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><div class="">Same for a 32 bit unsigned value.</div></div></div></blockquote><div><br class=""></div><div>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.</div><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><div class="">Does this fit in the design intent behind Register / MCRegister?</div></div></div></blockquote><div><br class=""></div><div>Yep</div><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><div class="">Thanks!</div><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div style="overflow-wrap: break-word;" class=""><div class=""><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div class=""><div class=""><blockquote type="cite" class=""><div class="">On 29 Sep 2020, at 11:02, Mircea Trofin <<a href="mailto:mtrofin@google.com" target="_blank" class="">mtrofin@google.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class="">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?</div><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Sep 29, 2020 at 10:49 AM Daniel Sanders <<a href="mailto:daniel_l_sanders@apple.com" target="_blank" class="">daniel_l_sanders@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div class=""><br class=""><div class=""><br class=""><blockquote type="cite" class=""><div class="">On 29 Sep 2020, at 09:28, Quentin Colombet <<a href="mailto:qcolombet@apple.com" target="_blank" class="">qcolombet@apple.com</a>> wrote:</div><br class=""><div class=""><div class=""><div class="">+ Daniel who added the MCRegister class.</div><div class=""><br class=""></div>Ah sorry, I replied too fast.<div class="">I mixed up MCPhysReg with MCRegister.</div><div class=""><br class=""></div><div class="">I was not aware we had such class.</div><div class=""><br class=""></div><div 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.</div><div class=""><br class=""></div><div class="">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.</div></div></div></blockquote><div class=""><br class=""></div>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)</div><div class=""><br class=""></div><div class="">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<br class=""><br class=""><blockquote type="cite" class=""><div class=""><div class=""><div class="">Let Daniel comment on that.</div><div class=""><br class=""></div><div class="">Cheers,</div><div class="">-Quentin</div><div class=""><div class=""><br class=""><blockquote type="cite" class=""><div class="">On Sep 29, 2020, at 9:12 AM, Mircea Trofin <<a href="mailto:mtrofin@google.com" target="_blank" class="">mtrofin@google.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class=""><div dir="ltr" class=""><br class=""></div><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Sep 29, 2020 at 9:08 AM Quentin Colombet <<a href="mailto:qcolombet@apple.com" target="_blank" class="">qcolombet@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">Hi,<br class=""><br class="">Register can represent virtual or physical registers.<br class="">MCRegister can only represent physical registers.<br class=""></blockquote><div class=""><br class=""></div><div class="">That's what I thought, but MCRegister has some stack slot APIs. </div></div></div></div></blockquote></div></div></div></div></blockquote><div class=""><br class=""></div><div class="">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:</div><div class=""><span style="white-space: pre-wrap;" class=""> </span>"StackSlot values do not exist in the MC layer, see Register::isStackSlot() for the more information on them."</div><div class=""><br class=""></div><div class="">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.</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><div class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">Eventually all Register instances are replaced by a MCRegister.<span class="Apple-converted-space"> </span><br class=""></blockquote><div class=""><br class=""></div><div class="">What happens in that case to the stack slot APIs?</div></div></div></div></blockquote></div></div></div></blockquote><div class=""><br class=""></div><div class="">Registers that are stack slots are never converted to MCRegister. That's not enforced in the constructor but it is enforced in MCRegister::isPhysicalRegister()</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><div class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">Cheers,<br class="">-Quentin<br class=""><br class="">> On Sep 28, 2020, at 5:46 PM, Mircea Trofin via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a>> wrote:<br class="">><span class="Apple-converted-space"> </span><br class="">> Hello,<br class="">><span class="Apple-converted-space"> </span><br class="">> 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.<br class=""></blockquote></div></div></div></blockquote></div></div></div></blockquote><div class=""><br class=""></div><div class="">Register provides everything related to vregs and stack slots.</div><div class="">Since I last looked at these files it seems a bit more of the number space definitions have been moved to MCRegister by</div><div class=""><span style="white-space: pre-wrap;" class=""> </span>16dae81edc2 [NFCI] Cleanup range checks in Register/MCRegister</div><div class="">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).</div><div class=""><br class=""></div><div class="">So to summarize the difference between MCRegister and Register:</div><div class=""><ul class=""><li class="">MCRegister defines the partitioning of the number space both for itself and for Register</li><li class="">MCRegister provides support for invalid and phys regs</li><li class="">Register is a superset of MCRegister and provides additional support for vreg and stack slots</li></ul></div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><div class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">><span class="Apple-converted-space"> </span><br class="">> Thanks!<br class="">> _______________________________________________<br class="">> LLVM Developers mailing list<br class="">><span class="Apple-converted-space"> </span><a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a><br class="">><span class="Apple-converted-space"> </span><a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank" class="">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a></blockquote></div></div></div></blockquote></div></div></div></blockquote></div></div></blockquote></div></div></blockquote></div></div></blockquote></div></div></div></blockquote></div></div></blockquote></div></div></blockquote></div><br class=""></body></html>