<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="">this is probably a reasonable solution. mostly, at this point, i’m worried about the fact that upstream now apparently has a different understanding of register classes than we do (in terms of what is “safe”), which could lead to other future issues for us.<div class=""><br class=""></div><div class="">—escha<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Feb 2, 2018, at 1:36 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; 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;"><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Ah, okay. So I assume you’re okay with disabling the post-RA run of this pass for this particular target to fix this particular issue?<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; 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>Friday, February 2, 2018 4:32 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>Quentin Colombet <qcolombet@apple.com>; 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 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; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">looking at it again, it’s making more sense: your patch enabled MachineCopyPropagation in the RA pipeline. the pass might have *already* been broken, but it wasn’t running for us, and now it is, so it broke things in that way too.<o:p class=""></o:p></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; 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; font-size: 11pt; font-family: Calibri, sans-serif;" class="">—escha<o:p class=""></o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; 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; font-size: 11pt; font-family: Calibri, sans-serif;" class="">On Feb 2, 2018, at 1:26 PM,<span class="Apple-converted-space"> </span><a href="mailto:gberry@codeaurora.org" style="color: purple; text-decoration: underline;" class="">gberry@codeaurora.org</a><span class="Apple-converted-space"> </span>wrote:<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 class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">escha,<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; 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; font-size: 11pt; font-family: Calibri, sans-serif;" class="">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 class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div><div class=""><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><div class=""><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><div class=""><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><div class=""><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><div class=""><div style="margin: 0in 0in 0.0001pt; 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 class=""><div style="margin: 0in 0in 0.0001pt; 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" style="color: purple; text-decoration: underline;" class="">fglaser@apple.com</a><span class="Apple-converted-space"> </span>[<a href="mailto:fglaser@apple.com" style="color: purple; text-decoration: underline;" 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" style="color: purple; text-decoration: underline;" class="">escha@apple.com</a><br class=""><b class="">Sent:</b><span class="apple-converted-space"> </span>Friday, February 2, 2018 4:10 PM<br class=""><b class="">To:</b><span class="apple-converted-space"> </span>Quentin Colombet <<a href="mailto:qcolombet@apple.com" style="color: purple; text-decoration: underline;" class="">qcolombet@apple.com</a>><br class=""><b class="">Cc:</b><span class="apple-converted-space"> </span>Geoff Berry <<a href="mailto:gberry@codeaurora.org" style="color: purple; text-decoration: underline;" class="">gberry@codeaurora.org</a>>;<span class="Apple-converted-space"> </span><a href="mailto:reviews+D41835+public+9c1dec7fb6e75ce0@reviews.llvm.org" style="color: purple; text-decoration: underline;" class="">reviews+D41835+public+9c1dec7fb6e75ce0@reviews.llvm.org</a>; Geoff Berry via Phabricator <<a href="mailto:reviews@reviews.llvm.org" style="color: purple; text-decoration: underline;" class="">reviews@reviews.llvm.org</a>>;<span class="Apple-converted-space"> </span><a href="mailto:javed.absar@arm.com" style="color: purple; text-decoration: underline;" class="">javed.absar@arm.com</a>; Matthias Braun <<a href="mailto:matze@braunis.de" style="color: purple; text-decoration: underline;" class="">matze@braunis.de</a>>; Jonas Paulsson <<a href="mailto:paulsson@linux.vnet.ibm.com" style="color: purple; text-decoration: underline;" class="">paulsson@linux.vnet.ibm.com</a>>;<span class="Apple-converted-space"> </span><a href="mailto:tstellar@redhat.com" style="color: purple; text-decoration: underline;" class="">tstellar@redhat.com</a>; Matt Arsenault <<a href="mailto:Matthew.Arsenault@amd.com" style="color: purple; text-decoration: underline;" class="">Matthew.Arsenault@amd.com</a>>;<span class="Apple-converted-space"> </span><a href="mailto:junbuml@codeaurora.org" style="color: purple; text-decoration: underline;" class="">junbuml@codeaurora.org</a>;<span class="Apple-converted-space"> </span><a href="mailto:marina.yatsina@intel.com" style="color: purple; text-decoration: underline;" class="">marina.yatsina@intel.com</a>;<span class="Apple-converted-space"> </span><a href="mailto:wei.ding2@amd.com" style="color: purple; text-decoration: underline;" class="">wei.ding2@amd.com</a>;<span class="Apple-converted-space"> </span><a href="mailto:kannan.narayanan@amd.com" style="color: purple; text-decoration: underline;" class="">kannan.narayanan@amd.com</a>;<span class="Apple-converted-space"> </span><a href="mailto:nhaehnle@gmail.com" style="color: purple; text-decoration: underline;" class="">nhaehnle@gmail.com</a>; Nemanja Ivanovic <<a href="mailto:nemanja.i.ibm@gmail.com" style="color: purple; text-decoration: underline;" class="">nemanja.i.ibm@gmail.com</a>>; llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" style="color: purple; text-decoration: underline;" class="">llvm-commits@lists.llvm.org</a>>;<span class="Apple-converted-space"> </span><a href="mailto:tpr.llvm@botech.co.uk" style="color: purple; text-decoration: underline;" class="">tpr.llvm@botech.co.uk</a><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><div class=""><div style="margin: 0in 0in 0.0001pt; 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; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><br class=""><br class=""><br class=""><o:p class=""></o:p></div></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class="" type="cite"><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">On Feb 2, 2018, at 12:55 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com" style="color: purple; text-decoration: underline;" class=""><span style="color: purple;" class="">qcolombet@apple.com</span></a>> wrote:<o:p class=""></o:p></div></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div><div class=""><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><br class=""><br class=""><br class=""><o:p class=""></o:p></div></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class="" type="cite"><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">On Feb 2, 2018, at 12:21 PM,<span class="apple-converted-space"> </span><a href="mailto:escha@apple.com" style="color: purple; text-decoration: underline;" class=""><span style="color: purple;" class="">escha@apple.com</span></a><span class="apple-converted-space"> </span>wrote:<o:p class=""></o:p></div></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div><div class=""><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">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 class=""></o:p></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">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 class=""></o:p></div></div></div></div></div></blockquote><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">That sounds like a better approach to me.<o:p class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">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 class=""></o:p></div></div></div></div></div></div></blockquote><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">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 class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">r3 = COPY foo<o:p class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">BUNDLE_TYPE_FOO<o:p class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><thing that uses r3><o:p class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">END<o:p class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">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 class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""> <o:p class=""></o:p></div></div></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">—escha</div></div></div></div></blockquote></div></div></div></div></div></blockquote></div><br class=""></div></body></html>