<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 22, 2016, at 1:52 PM, Matthias Braun <<a href="mailto:mbraun@apple.com" class="">mbraun@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><blockquote type="cite" class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""><br class="Apple-interchange-newline">On Jan 22, 2016, at 1:44 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com" class="">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><blockquote type="cite" class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""><br class="Apple-interchange-newline">On Jan 22, 2016, at 1:23 PM, Matthias Braun <<a href="mailto:mbraun@apple.com" class="">mbraun@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><blockquote type="cite" class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""><br class="Apple-interchange-newline">On Jan 22, 2016, at 12:29 PM, Derek Schuff <<a href="mailto:dschuff@google.com" class="">dschuff@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Here are 2 patches, which are independent of each other.<div class=""><br class=""></div><div class="">The first splits PrologEpilogInserter into 2 parts : <a href="http://reviews.llvm.org/D16481" class="">http://reviews.llvm.org/D16481</a></div><div class="">After looking at the code I thought it made more sense for the major split to include whether callee-saved register spills are supported. So for non-virtual targets, virtual registers are not supported and scavenging is optionally supported, and vice versa for virtual targets. The base case includes just frame finalization, prolog/epilog code insertion and FrameIndex elimination. The CSR-supporting case includes CSR spilling and optionally scavenging.</div><div class="">It's a little bit ugly because e.g. most of the target hooks (even those used in the core pieces like FI elimination) take a pointer to a RegScavenger object which could be null, and there's still a little bit of code in the FI elimination code that takes CSR spilling into account. But I do think it is an improvement, and I'd be interested in feedback. This patch could be ready to land if people are happy with the design.</div></div></div></blockquote><blockquote type="cite" class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">The other (<a href="http://reviews.llvm.org/D16483" class="">http://reviews.llvm.org/D16483</a><span class="Apple-converted-space"> </span>)is the aforementioned prototype for requiring MachineFunctionPasses to opt-in to supporting virtual registers. It's only done for WebAssembly and X86 so far as a proof of concept. The interesting bit to look at is the way it's implemented (right now it's just a virtual method that's called inside an assert which can be compiled away) and the passes that will opt-in. It's mostly a concrete version of the info I put in my previous post. Before it lands I'll have to update the rest of the targets, and will probably manually expand the SUPPORTS_VIRTUAL_REGISTERS macro which I used for my experimental convenience (unless you think it's actually better that way).</div></div></div></blockquote><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Thanks for working on this.</div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">In my opinion the important thing with the (not) supporting vregs in a pass was that I would have liked to make it explicit if a target wants to use vregs after regalloc.</div></div></blockquote><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote type="cite" class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">This way we can actually differentiate the cases where vregs appear after register allocation because of a bug and the case where they are intentional. The important thing would be to add something like MachineRegisterInfo::setVirtRegsAfterRegalloc() and MachineRegisterInfo::getVirtRegsAfterRegalloc(). Because I would assume that we will find more examples like the following (from MachineBasicBlock):</div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""> void addLiveIn(MCPhysReg PhysReg, LaneBitmask LaneMask = ~0u) {</div><div class=""> LiveIns.push_back(RegisterMaskPair(PhysReg, LaneMask));</div><div class=""> }</div><div class=""><br class=""></div><div class="">this needs to be changed to support VRegs. But if we do that change, I'd like to change it to something like this:</div><div class=""><br class=""></div><div class=""><div class=""> void addLiveIn(unsigned Reg, LaneBitmask LaneMask = ~0u) {</div><div class=""> assert(TargetRegisterInfo::isPhysReg(Reg) || getParent()->getRegInfo().getVirtRegsAfterRegalloc());</div><div class=""> LiveIns.push_back(RegisterMaskPair(Reg, LaneMask));</div><div class=""> }</div></div></div></div></blockquote><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">I haven’t looked at the patch, just commenting on that example. My understanding was that supporting both virtual and physical registers should not have any impact on anything else, but the pass itself. I.e., if the pass updates the liveIns, in that example, it does that only for physical registers. I.e., we do not push for changes like this.</div></div></blockquote><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">Without changing addLiveIn you will not be able to use something like the RegisterScavenger or LivePhysRegs (or rather an equivalent to those two that supports virtual registers).</div></div></blockquote><div><br class=""></div><div>That is the thing, if we need it, that probably means this pass does not support virtual registers.</div><br class=""><blockquote type="cite" class=""><div class=""><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""> This is probably not needed today but I'm pretty sure we will find more minor things in common infrastructure that need to be updated to support virtregs along physregs (a different example would be <a href="http://reviews.llvm.org/D14750" class="">http://reviews.llvm.org/D14750</a>). I don't expect any huge changes in the common infrastructure but enough that I would like to be able to write an assert like in my example above.</div></div></blockquote><div><br class=""></div>Fair enough, but I’d rather decide when we see an actual use for that.<br class=""><br class="">Q.<br class=""><blockquote type="cite" class=""><div class=""><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">- Matthias</div></div></blockquote></div><br class=""></body></html>