<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.EmailStyle19
{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>I’ve posted a patch that I believe addresses everyone’s concerns here: <a href="https://reviews.llvm.org/D43042">https://reviews.llvm.org/D43042</a><o:p></o:p></p><p class=MsoNormal>Please take a look.<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><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal style='margin-left:.5in'><b>From:</b> fglaser@apple.com [mailto:fglaser@apple.com] <b>On Behalf Of </b>escha@apple.com<br><b>Sent:</b> Wednesday, February 7, 2018 12:52 PM<br><b>To:</b> gberry@codeaurora.org<br><b>Cc:</b> 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><b>Subject:</b> Re: [PATCH] D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding<o:p></o:p></p></div></div><p class=MsoNormal style='margin-left:.5in'><o:p> </o:p></p><p class=MsoNormal style='margin-left:.5in'><o:p> </o:p></p><div><p class=MsoNormal style='margin-left:.5in'><br><br><o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal style='margin-left:.5in'>On Feb 5, 2018, at 11:27 AM, via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<o:p></o:p></p></div><p class=MsoNormal style='margin-left:.5in'><o:p> </o:p></p><div><div><p class=MsoNormal style='margin-left:.5in'> <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 style='margin-left:.5in'><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><o:p></o:p></p></div></div></div><div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div style='margin-left:.5in'><p class=MsoNormal style='margin-left:.5in'>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 style='margin-left:.5in'><p class=MsoNormal style='margin-left:.5in'> <o:p></o:p></p></div><div><div><div><div><div><div><div><div><div><div><div><div><div><div style='margin-left:.5in'><p class=MsoNormal style='margin-left:.5in'>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><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'><p class=MsoNormal style='margin-left:.5in'>- 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 style='margin-left:.5in'><p class=MsoNormal style='margin-left:.5in'>- The register allocator respects those constraints and selects a register from the correct class<o:p></o:p></p></div></div><div style='margin-left:.5in'><p class=MsoNormal style='margin-left:.5in'>- The selected physical register is marked as renamable<o:p></o:p></p></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'><p class=MsoNormal style='margin-left:.5in'>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 style='margin-left:.5in'><p class=MsoNormal style='margin-left:.5in'> <o:p></o:p></p></div></div><div style='margin-left:.5in'><p class=MsoNormal style='margin-left:.5in'>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 style='margin-left:.5in'><p class=MsoNormal style='margin-left:.5in'>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></blockquote><div><div style='margin-left:.5in'><p class=MsoNormal style='margin-left:.5in'> <o:p></o:p></p></div></div><div><div style='margin-left:.5in'><p class=MsoNormal style='margin-left:.5in'>Again, yes I think we need that.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='margin-left:.5in'> <o:p></o:p></p></div><div><p class=MsoNormal style='margin-left:.5in'> <o:p></o:p></p></div><div><p class=MsoNormal style='margin-left:.5in'>Ok, I will put together a strawman proposal for adding this target global allowRegisterRenaming flag.<o:p></o:p></p></div><div><p class=MsoNormal style='margin-left:.5in'> <o:p></o:p></p></div></div><div><div style='margin-left:.5in'><p class=MsoNormal style='margin-left:.5in'>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 style='margin-left:.5in'><p class=MsoNormal style='margin-left:.5in'> <o:p></o:p></p></div></div></div></blockquote><div><p class=MsoNormal style='margin-left:.5in'><o:p> </o:p></p></div><p class=MsoNormal style='margin-left:.5in'>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><p class=MsoNormal style='margin-left:.5in'><o:p> </o:p></p></div><div><p class=MsoNormal style='margin-left:.5in'>thanks for looking into this!<o:p></o:p></p></div><div><p class=MsoNormal style='margin-left:.5in'><o:p> </o:p></p></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><p class=MsoNormal style='margin-left:.5in'><o:p> </o:p></p></div></body></html>