
<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On May 5, 2013, at 7:08 AM, Michele Scandale <<a href="mailto:michele.scandale@gmail.com">michele.scandale@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="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;">Hi Benjamin,<br><br><blockquote type="cite">I believe that this was intentionally chose because simplifyLoopLatch can improve the analyzability of a loop without introducing the duplication regular loop rotation does. Maybe Andy has an opinion here.<br></blockquote><br>I understand that the simplifyLoopLatch wants to put the loop so that the latch has a conditional branch, so that is similar to what loop rotate will produce, but without the duplication of the header.<br><br>In the case of loops with multiple exits there is no benefit respect to the number of branches executed between loop rotation and application of 'simplifyLoopLatch'.<br><br>The problem is that simplifyLoopLatch do not move the header of the while loop to the latch and this prevent the hoisting of loop invariant code that is guaranteed executed at each iteration.<br><br><blockquote type="cite">This also needs a test case.<br></blockquote><br>In the new patch there is also a test case that catch the hoisted 'load' instruction.<br><br><br>What I see is that simplifyLoopLatch as alternative to loop rotation prevents another optimization. I don't see a way to allow hoisting in these cases without a proper loop rotation.<br></div></blockquote></div><br><div>Thanks for making sure we get this fixed. I hope r181230 works for you. The short answer is that your patch is a good idea but does partly defeat the purpose of simplifyLoopLatch, and there were too many regressions for me to analyze. The fix I checked in simply has fewer regressions.</div><div><br></div><div>Please see the checkin comments and update to PR10293 for details.</div><div><br></div><div>I'm still not happy with LoopRotate's ability to expose downstream optimization, but doing better takes some real performance investigation, which is why I sat on this so long.</div><div><br></div><div>-Andy</div></body></html>