<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">The patch looks fine to me. Is there any measurable compile time impact?<div><br></div><div>Evan</div><div><br><div><div>On Sep 9, 2013, at 9:20 AM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Ping^2?<div><br><div apple-content-edited="true">
<div style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">-Quentin</div>

</div>
<br><div style=""><div>On Sep 3, 2013, at 9:41 AM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Ping?<br><div apple-content-edited="true">
<div style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">-Quentin</div>

</div>
<br><div><div>On Aug 28, 2013, at 5:29 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Evan,<div><br></div><div>Eric suggested that you may be the right person to review this patch.</div><div>Could you have a look, please?</div><div><br></div><div>This fixes <<a href="rdar://problem/14742333">rdar://problem/14742333</a>>.</div><div><br></div><div>Thanks,<br><div apple-content-edited="true">
<div style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">-Quentin</div><div style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"></div></div></div></div>
<span><PeepholeCopy.svndiff></span><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div apple-content-edited="true"><div style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"></div>

</div>
<br><div><div>On Aug 28, 2013, at 5:25 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;"><br>On Aug 28, 2013, at 4:43 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br><br><blockquote type="cite"><blockquote type="cite"><br>The patch is not triggered by any of the tests that are in the llvm test<br>suite (tested on both ARM and X86).<br><br></blockquote><br>*nod* I figured, otherwise you'd have said it. :)<br><br><blockquote type="cite">Like I said in my original email, I could rewrite/eliminate more<br>aggressively copies (and in that case it does kick in in the llvm test<br>suite) but then, the register allocator does in a few cases a poor<br>coalescing/live-range splitting job.<br><br></blockquote><br>*nod*<br><br><blockquote type="cite">That said, as it is, when the patch kicks in, we observed with our internal<br>tests: no regressions and up to 25% runtime speed-up.<br><br></blockquote><br>Pretty nice. Can you give a general idea of the type of code that this<br>is triggering on?<br></blockquote>Basically it was some code using a tone of intrinsics, in particular mixing vector, integer, and floating point code.<br>For isel, this means, tone of bitcast, vector insert, and vector extract, across different basic blocks.<br><br><blockquote type="cite"><br><blockquote type="cite">Perhaps an<br>example of what you're optimizing as a comment in the code?<br><br>I have added one in the file header. What do you think?<br><br></blockquote><br>Helps. I was hoping for more code details, but the comment provides<br>some helpful bit.<br></blockquote>Do you think I should put more comments?<br><br><blockquote type="cite"><br><blockquote type="cite">I have moved the assignment inside the loop.<br>Is it better?<br>(I do like predicated code!)<br><br></blockquote><br>Looks better to me from a readability perspective.<br><br><blockquote type="cite"><br>I'm not necessarily comfortable approving, but the code looks reasonable.<br><br>What do you miss to be comfortable?<br>Could you point me someone that may be comfortable to approve this, please?<br>(So that I can through him under the bus!)<br><br></blockquote><br>Ultimately Jakob or Evan. It looks pretty simple so unless you get an<br>objection by tomorrow morning I guess submit?<br></blockquote>I will do that.<br><br>Thanks,<br>Quentin<br><blockquote type="cite"><br>-eric<br><br><blockquote type="cite">Thanks again.<br><br>-Quentin<br><br><br>-eric<br><br><br>On Fri, Aug 23, 2013 at 1:04 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>><br>wrote:<br><br>Hi,<br><br>Here is a patch to rewrite copies to avoid cross register banks copies when<br>possible.<br>Thanks for your reviews.<br><br>** Context **<br><br>By definition copies across register bank are not coalescable. Still, it may<br>be possible to get rid of such a copy when the value is available in another<br>register of the same register file.<br>Consider the following example, where capital and lower letters denote<br>different register file:<br>b = copy A <-- cross-bank copy<br><br>C = copy b <-- cross-bank copy<br><br>This could have been optimized this way:<br>b = copy A  <-- cross-bank copy<br><br>C = copy A <-- same-bank copy<br><br>Note: b and C's definitions may be in different basic blocks.<br><br><br>** Proposed Solution **<br><br>Add a peephole optimization that looks through a chain of copies leading to<br>a cross-bank copy and reuses a source that is on the same register file if<br>available.<br><br>This solution could also be used to get rid of some copies (e.g., A could<br>have been used instead of C). However, we do not do so because:<br>- It may over constrain the coloring of the source register for coalescing.<br>- The register allocator may not be able to find a nice split point for the<br>longer live-range, leading to more spill.<br><br><br>** Testcase?! **<br><br>This patch does not include a test case, because it is very difficult to<br>reproduce this behavior with a reasonably small input.<br><br>Cheers,<br>-Quentin<br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></blockquote></blockquote></div></blockquote></div><br></div>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></blockquote></div><br></div>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></blockquote></div><br></div></div></blockquote></div><br></div></body></html>