<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.EmailStyle18
{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>escha,<o:p></o:p></p><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>I’m confused by your comment about bundles. It was known that this patch doesn’t handle forwarding into bundles, but it sounds like to me you are seeing problems with the dead COPY removal part of this pass, which was not intentionally changed by this patch (other than adding a DEBUG statement when it happens).<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><b>From:</b> fglaser@apple.com [mailto:fglaser@apple.com] <b>On Behalf Of </b>escha@apple.com<br><b>Sent:</b> Friday, February 2, 2018 4:10 PM<br><b>To:</b> Quentin Colombet <qcolombet@apple.com><br><b>Cc:</b> Geoff Berry <gberry@codeaurora.org>; reviews+D41835+public+9c1dec7fb6e75ce0@reviews.llvm.org; Geoff Berry via Phabricator <reviews@reviews.llvm.org>; javed.absar@arm.com; Matthias Braun <matze@braunis.de>; Jonas Paulsson <paulsson@linux.vnet.ibm.com>; tstellar@redhat.com; Matt Arsenault <Matthew.Arsenault@amd.com>; junbuml@codeaurora.org; marina.yatsina@intel.com; wei.ding2@amd.com; kannan.narayanan@amd.com; nhaehnle@gmail.com; Nemanja Ivanovic <nemanja.i.ibm@gmail.com>; llvm-commits <llvm-commits@lists.llvm.org>; tpr.llvm@botech.co.uk<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><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 2, 2018, at 12:55 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p><div><div><p class=MsoNormal><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 2, 2018, at 12:21 PM, <a href="mailto:escha@apple.com">escha@apple.com</a> wrote:<o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p><div><div><p class=MsoNormal>I mean, in that case we are likely to have to mark every single opcode (all 12,000 or so) with this requirement. At that point we might as well just opt out of the pass, I think? At least, it feels like a gross hack that papers over the fact that LLVM has changed how register classes work such that our entire approach is no longer valid.<o:p></o:p></p><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Also, it seems very weird to make this constraint-violating behavior *opt-out*. Maybe it should be opt-in, i.e. put doesNotHaveExtraSrcRegAllocReq on all instructions it’s okay for?<o:p></o:p></p></div></div></div></blockquote><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>That sounds like a better approach to me.<o:p></o:p></p></div><div><p class=MsoNormal>After talking with escha, I agree that TableGen is not necessarily expressive enough to model all the constraints that need to be met and I would err on the safe side of being opt-in instead of opt-out.<o:p></o:p></p></div></div></div></div></blockquote><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>this is actually worse than i initially realized. one of our targets is VLIW and is completely broken by this patch, *even if we opt out as described*, because it doesn’t iterate over the operands of a bundle. so for example:<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>r3 = COPY foo<o:p></o:p></p></div><div><p class=MsoNormal>BUNDLE_TYPE_FOO<o:p></o:p></p></div><div><p class=MsoNormal><thing that uses r3><o:p></o:p></p></div><div><p class=MsoNormal>END<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>it believes that r3 is dead because it doesn’t iterate over the bundle operands, so it never sees the use of r3.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>—escha<o:p></o:p></p></div></div></body></html>