<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="">FYI, I've revert the commit in r325421. It was getting messing :)<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Feb 7, 2018, at 1:38 PM, <a href="mailto:gberry@codeaurora.org" class="">gberry@codeaurora.org</a> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="WordSection1" style="page: WordSection1; 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 style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">I’ve posted a patch that I believe addresses everyone’s concerns here:<span class="Apple-converted-space"> </span><a href="https://reviews.llvm.org/D43042" style="color: purple; text-decoration: underline;" class="">https://reviews.llvm.org/D43042</a><o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Please take a look.<o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">--<span class="Apple-converted-space"> </span><o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Geoff Berry<o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Employee of Qualcomm Datacenter Technologies, Inc.<o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.<o:p class=""></o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div class=""><div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(225, 225, 225); padding: 3pt 0in 0in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><b class="">From:</b><span class="Apple-converted-space"> </span><a href="mailto:fglaser@apple.com" class="">fglaser@apple.com</a> [<a href="mailto:fglaser@apple.com" class="">mailto:fglaser@apple.com</a>]<span class="Apple-converted-space"> </span><b class="">On Behalf Of<span class="Apple-converted-space"> </span></b><a href="mailto:escha@apple.com" class="">escha@apple.com</a><br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>Wednesday, February 7, 2018 12:52 PM<br class=""><b class="">To:</b><span class="Apple-converted-space"> </span>gberry@codeaurora.org<br class=""><b class="">Cc:</b><span class="Apple-converted-space"> </span>qcolombet@apple.com; Nemanja Ivanovic <nemanja.i.ibm@gmail.com>; junbuml@codeaurora.org; marina.yatsina@intel.com; reviews+D41835+public+9c1dec7fb6e75ce0@reviews.llvm.org; Nicolai Hähnle-Montoro <nhaehnle@gmail.com>; Matthias Braun <matze@braunis.de>; llvm-commits <llvm-commits@lists.llvm.org>; tpr.llvm@botech.co.uk; javed.absar@arm.com<br class=""><b class="">Subject:</b><span class="Apple-converted-space"> </span>Re: [PATCH] D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding<o:p class=""></o:p></div></div></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><br class=""><br class=""><o:p class=""></o:p></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class="" type="cite"><div class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">On Feb 5, 2018, at 11:27 AM, via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" style="color: purple; text-decoration: underline;" class="">llvm-commits@lists.llvm.org</a>> wrote:<o:p class=""></o:p></div></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div><div class=""><div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(225, 225, 225); padding: 3pt 0in 0in;" class=""><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><b class="">From:</b><span class="apple-converted-space"> </span><a href="mailto:qcolombet@apple.com" style="color: purple; text-decoration: underline;" class=""><span style="color: purple;" class="">qcolombet@apple.com</span></a><span class="apple-converted-space"> </span>[<a href="mailto:qcolombet@apple.com" style="color: purple; text-decoration: underline;" class=""><span style="color: purple;" class="">mailto:qcolombet@apple.com</span></a>]<span class="apple-converted-space"> </span><br class=""><b class="">Sent:</b><span class="apple-converted-space"> </span>Friday, February 2, 2018 8:18 PM<br class=""><br class=""><br class=""><o:p class=""></o:p></div></div></div></div><div class=""><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class="" type="cite"><div class=""><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">On Feb 2, 2018, at 3:53 PM, Nemanja Ivanovic via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" style="color: purple; text-decoration: underline;" class=""><span style="color: purple;" class="">llvm-commits@lists.llvm.org</span></a>> wrote:<o:p class=""></o:p></div></div></div><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div><div class=""><div class=""><div class=""><div class=""><div class=""><div class=""><div class=""><div class=""><div class=""><div class=""><div class=""><div class=""><div class=""><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Sorry, but it seems to me that we are working around the problem with the hasExtraRegAllocReq flag and this workaround is introducing new problems that then need to be solved.<o:p class=""></o:p></div></div></div><p class="MsoNormal" style="margin: 0in 0in 12pt 1in; font-size: 11pt; font-family: Calibri, sans-serif;">Going back to the original problem that Escha reported, it seems to be that the back end ends up in the following state which arguably is probably not a reasonable state to be in (if register renaming is to be allowed):<o:p class=""></o:p></p></div><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">- There are constraints added to the virtual registers that restrict which register class the assigned physical register must come from<o:p class=""></o:p></div></div></div><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">- The register allocator respects those constraints and selects a register from the correct class<o:p class=""></o:p></div></div></div><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">- The selected physical register is marked as renamable<o:p class=""></o:p></div></div></div><p class="MsoNormal" style="margin: 0in 0in 12pt 1in; font-size: 11pt; font-family: Calibri, sans-serif;">- Calling MachineInstr::getRegClassConstraint() on the machine instruction returns a register class that does not take into account the additional constraints<o:p class=""></o:p></p></div><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Ultimately, anything that does register renaming with the back end in that state can potentially break the code since it doesn't know what actual class the register rename is allowed to come from.<o:p class=""></o:p></div></div></div><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div></div><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Wouldn't this mean that a target that can be in this state has at least these two reasonable options:<o:p class=""></o:p></div></div></div><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">1. Set some property that will ensure that the renamable flag is never set on any register - essentially what Geoff is suggesting in the latter part of his last email. I would just make it a property of TargetRegisterInfo (perhaps `virtual bool TargetRegisterInfo::allowRegisterRenaming()`).<o:p class=""></o:p></div></div></div></div></div></div></blockquote><div class=""><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div></div><div class=""><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Again, yes I think we need that.<o:p class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Ok, I will put together a strawman proposal for adding this target global allowRegisterRenaming flag.<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div></div><div class=""><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">More generally, I believe we want to step back with the renamable flag, think about its semantic and what in the infrastructure we would need to fix, if any, to make sure that semantic is preserved/preservable along the way.<o:p class=""></o:p></div></div></div><div style="margin-left: 0.5in;" class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div></div></div></blockquote><div class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">what’s the status of this; are we planning on reverting this in llvm for now? i ask because currently i’m handling the problem in our tree by keeping this patch reverted, but we really try to avoid long-term divergence from upstream llvm both due to the hassles it causes us and the difficulties it causes in trying to contribute patches back to upstream llvm.<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class="">thanks for looking into this!<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div><div class=""><p class="MsoNormal" style="margin: 0in 0in 12pt 0.5in; font-size: 11pt; font-family: Calibri, sans-serif;">—escha</p></div></div></div></blockquote></div><br class=""></body></html>