<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Hi Geoff,</div><br class=""><div><blockquote type="cite" class=""><div class="">On Oct 7, 2016, at 10:39 AM, Geoff Berry <<a href="mailto:gberry@codeaurora.org" class="">gberry@codeaurora.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Hi Quentin,</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">I don't think there is much overlap between this code and MachineCopyPropagation, as the new code is driven primarily by the LiveIntervalAnalysis data and the two passes remove different kinds of COPYs. </span></div></blockquote><div><br class=""></div><div>That sounds sensible.</div><div><br class=""></div><div>I’ll have a look at the patch by the end of next week.</div><div><br class=""></div><div>Cheers,</div><div>-Quentin</div><br class=""><blockquote type="cite" class=""><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class=""> I think the pass name "MachineCopyPropagation" is perhaps causing some confusion, since that pass doesn't actually do any propagating. IMO, it should be called something like MachineRedundantCopyRemoval instead to more accurately describe what it does.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On 10/7/2016 1:24 PM, Quentin Colombet wrote:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">Hi Geoff,<br class=""><br class=""><blockquote type="cite" class="">On Oct 7, 2016, at 8:36 AM, Geoff Berry <<a href="mailto:gberry@codeaurora.org" class="">gberry@codeaurora.org</a>> wrote:<br class=""><br class="">gberry added a comment.<br class=""><br class="">In <a href="https://reviews.llvm.org/D25347#563996" class="">https://reviews.llvm.org/D25347#563996</a>, @qcolombet wrote:<br class=""><br class=""><blockquote type="cite" class="">Hi Geoff,<br class=""><br class="">Disclaimer: I haven't looked at the patch, just a comment.<br class="">I would rather fix the MachineCopyPropagation pass than adding this logic to this pass.<br class=""><br class="">Why aren't we catching those cases in the MachineCopyPropagation pass?<br class=""></blockquote><br class="">Hi Quentin,<br class=""><br class="">I believe this was discussed before here: <a href="https://reviews.llvm.org/D20531" class="">https://reviews.llvm.org/D20531</a>. In this change, the COPYs only become removable as the result of renaming/recoloring their destination registers.<br class=""></blockquote>I see, thanks for checking.<br class=""><br class=""><blockquote type="cite" class=""> I thought the consensus was that this recoloring was only safe to do with virtual registers to avoid violating ABI/other register constraints.<br class=""></blockquote>That is correct.<br class=""><br class=""><blockquote type="cite" class=""> The COPYs that this change catches are mostly only removable after register allocation since they are only removable once live range splitting/spilling has occurred.<br class=""></blockquote>In that case, we would refactor the code so that both the MachineCopyPropagation and this addition uses common code. Again I haven’t looked at the patch itself, but I am guessing a lot of the core logic to look similar.<br class=""><br class="">Let me know if I am mistaken.<br class=""><br class="">Cheers,<br class="">-Quentin<br class=""><br class=""><blockquote type="cite" class="">In <a href="https://reviews.llvm.org/D25347#563996" class="">https://reviews.llvm.org/D25347#563996</a>, @qcolombet wrote:<br class=""><br class=""><blockquote type="cite" class="">Hi Geoff,<br class=""><br class="">Disclaimer: I haven't looked at the patch, just a comment.<br class="">I would rather fix the MachineCopyPropagation pass than adding this logic to this pass.<br class=""><br class="">Why aren't we catching those cases in the MachineCopyPropagation pass?<br class=""><br class="">Cheers,<br class="">-Quentin<br class=""></blockquote><br class=""><br class=""><br class=""><br class=""><a href="https://reviews.llvm.org/D25347" class="">https://reviews.llvm.org/D25347</a><br class=""><br class=""><br class=""><br class=""></blockquote></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">--<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Geoff Berry</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Employee of Qualcomm Datacenter Technologies, Inc.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" 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.</span></div></blockquote></div><br class=""></body></html>