<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 15 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.apple-converted-space
{mso-style-name:apple-converted-space;}
span.EmailStyle20
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body lang=EN-US link=blue vlink=purple><div class=WordSection1><p class=MsoNormal>This change has been re-enabled in r326208, now that the renamable flag setting has been made more conservative by default and is opt-in on a per-target basis so as not to effect out-of-tree targets. Please let me know if there any more issues.<o:p></o:p></p><p class=MsoNormal><o:p> </o:p></p><div><p class=MsoNormal>-- <o:p></o:p></p><p class=MsoNormal>Geoff Berry<o:p></o:p></p><p class=MsoNormal>Employee of Qualcomm Datacenter Technologies, Inc.<o:p></o:p></p><p class=MsoNormal> 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></o:p></p></div><p class=MsoNormal><o:p> </o:p></p><div style='border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt'><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b>From:</b> qcolombet@apple.com [mailto:qcolombet@apple.com] <br><b>Sent:</b> Friday, February 16, 2018 10:10 PM<br><b>To:</b> gberry@codeaurora.org<br><b>Cc:</b> escha@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><b>Subject:</b> Re: [PATCH] D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding<o:p></o:p></p></div></div><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>FYI, I've revert the commit in r325421. It was getting messing :)<o:p></o:p></p><div><p class=MsoNormal><br><br><o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal>On Feb 7, 2018, at 1:38 PM, <a href="mailto:gberry@codeaurora.org">gberry@codeaurora.org</a> wrote:<o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p><div><div><p class=MsoNormal>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"><span style='color:purple'>https://reviews.llvm.org/D43042</span></a><o:p></o:p></p></div><div><p class=MsoNormal>Please take a look.<o:p></o:p></p></div><div><p class=MsoNormal> <o:p></o:p></p></div><div><div><p class=MsoNormal>--<span class=apple-converted-space> </span><o:p></o:p></p></div><div><p class=MsoNormal>Geoff Berry<o:p></o:p></p></div><div><p class=MsoNormal>Employee of Qualcomm Datacenter Technologies, Inc.<o:p></o:p></p></div><div><p class=MsoNormal>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></o:p></p></div></div><div><p class=MsoNormal> <o:p></o:p></p></div><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><div style='margin-left:.5in'><p class=MsoNormal><b>From:</b><span class=apple-converted-space> </span><a href="mailto:fglaser@apple.com">fglaser@apple.com</a> [<a href="mailto:fglaser@apple.com">mailto:fglaser@apple.com</a>]<span class=apple-converted-space> </span><b>On Behalf Of<span class=apple-converted-space> </span></b><a href="mailto:escha@apple.com">escha@apple.com</a><br><b>Sent:</b><span class=apple-converted-space> </span>Wednesday, February 7, 2018 12:52 PM<br><b>To:</b><span class=apple-converted-space> </span><a href="mailto:gberry@codeaurora.org">gberry@codeaurora.org</a><br><b>Cc:</b><span class=apple-converted-space> </span><a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>; Nemanja Ivanovic <<a href="mailto:nemanja.i.ibm@gmail.com">nemanja.i.ibm@gmail.com</a>>; <a href="mailto:junbuml@codeaurora.org">junbuml@codeaurora.org</a>; <a href="mailto:marina.yatsina@intel.com">marina.yatsina@intel.com</a>; <a href="mailto:reviews+D41835+public+9c1dec7fb6e75ce0@reviews.llvm.org">reviews+D41835+public+9c1dec7fb6e75ce0@reviews.llvm.org</a>; Nicolai Hähnle-Montoro <<a href="mailto:nhaehnle@gmail.com">nhaehnle@gmail.com</a>>; Matthias Braun <<a href="mailto:matze@braunis.de">matze@braunis.de</a>>; llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>>; <a href="mailto:tpr.llvm@botech.co.uk">tpr.llvm@botech.co.uk</a>; <a href="mailto:javed.absar@arm.com">javed.absar@arm.com</a><br><b>Subject:</b><span class=apple-converted-space> </span>Re: [PATCH] D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding<o:p></o:p></p></div></div></div><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div><div><div style='margin-left:.5in'><p class=MsoNormal><br><br><br><o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div style='margin-left:.5in'><p class=MsoNormal>On Feb 5, 2018, at 11:27 AM, via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org"><span style='color:purple'>llvm-commits@lists.llvm.org</span></a>> wrote:<o:p></o:p></p></div></div><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div><div><div><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div></div><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal><b>From:</b><span class=apple-converted-space> </span><a href="mailto:qcolombet@apple.com"><span style='color:purple'>qcolombet@apple.com</span></a><span class=apple-converted-space> </span>[<a href="mailto:qcolombet@apple.com"><span style='color:purple'>mailto:qcolombet@apple.com</span></a>]<span class=apple-converted-space> </span><br><b>Sent:</b><span class=apple-converted-space> </span>Friday, February 2, 2018 8:18 PM<br><br><br><br><o:p></o:p></p></div></div></div></div><div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal>On Feb 2, 2018, at 3:53 PM, Nemanja Ivanovic via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org"><span style='color:purple'>llvm-commits@lists.llvm.org</span></a>> wrote:<o:p></o:p></p></div></div></div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div></div><div><div><div><div><div><div><div><div><div><div><div><div><div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal>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></o:p></p></div></div></div><p class=MsoNormal style='mso-margin-top-alt:0in;margin-right:0in;margin-bottom:12.0pt;margin-left:1.0in'>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></o:p></p></div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal>- There are constraints added to the virtual registers that restrict which register class the assigned physical register must come from<o:p></o:p></p></div></div></div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal>- The register allocator respects those constraints and selects a register from the correct class<o:p></o:p></p></div></div></div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal>- The selected physical register is marked as renamable<o:p></o:p></p></div></div></div><p class=MsoNormal style='mso-margin-top-alt:0in;margin-right:0in;margin-bottom:12.0pt;margin-left:1.0in'>- Calling MachineInstr::getRegClassConstraint() on the machine instruction returns a register class that does not take into account the additional constraints<o:p></o:p></p></div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal>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></o:p></p></div></div></div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div></div></div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal>Wouldn't this mean that a target that can be in this state has at least these two reasonable options:<o:p></o:p></p></div></div></div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal>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></o:p></p></div></div></div></div></div></div></blockquote><div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div></div></div><div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal>Again, yes I think we need that.<o:p></o:p></p></div></div></div><div><div><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div></div><div><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div></div><div><div style='margin-left:.5in'><p class=MsoNormal>Ok, I will put together a strawman proposal for adding this target global allowRegisterRenaming flag.<o:p></o:p></p></div></div><div><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div></div></div><div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal>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></o:p></p></div></div></div><div style='margin-left:.5in'><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div></div></div></div></blockquote><div><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div></div><div style='margin-left:.5in'><p class=MsoNormal>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></o:p></p></div></div><div><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div></div><div><div style='margin-left:.5in'><p class=MsoNormal>thanks for looking into this!<o:p></o:p></p></div></div><div><div style='margin-left:.5in'><p class=MsoNormal> <o:p></o:p></p></div></div><div><p class=MsoNormal style='mso-margin-top-alt:0in;margin-right:0in;margin-bottom:12.0pt;margin-left:.5in'>—escha<o:p></o:p></p></div></div></blockquote></div><p class=MsoNormal><o:p> </o:p></p></div></div></body></html>