<html>
<head>
<base href="http://llvm.org/bugs/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:atrick@apple.com" title="Andrew Trick <atrick@apple.com>"> <span class="fn">Andrew Trick</span></a>
</span> changed
<a class="bz_bug_link
bz_status_RESOLVED bz_closed"
title="RESOLVED FIXED - Load not hoisted out of loop with multiple exits"
href="http://llvm.org/bugs/show_bug.cgi?id=10293">bug 10293</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Status</td>
<td>NEW
</td>
<td>RESOLVED
</td>
</tr>
<tr>
<td style="text-align:right;">Resolution</td>
<td>---
</td>
<td>FIXED
</td>
</tr>
<tr>
<td style="text-align:right;">Assignee</td>
<td>unassignedbugs@nondot.org
</td>
<td>atrick@apple.com
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_RESOLVED bz_closed"
title="RESOLVED FIXED - Load not hoisted out of loop with multiple exits"
href="http://llvm.org/bugs/show_bug.cgi?id=10293#c10">Comment # 10</a>
on <a class="bz_bug_link
bz_status_RESOLVED bz_closed"
title="RESOLVED FIXED - Load not hoisted out of loop with multiple exits"
href="http://llvm.org/bugs/show_bug.cgi?id=10293">bug 10293</a>
from <span class="vcard"><a class="email" href="mailto:atrick@apple.com" title="Andrew Trick <atrick@apple.com>"> <span class="fn">Andrew Trick</span></a>
</span></b>
<pre>Michele's observation is good. But whether loop rotation is beneficial is not
entirely dependent on whether the latch is simplified. For example, the loop
may have been written as:
unsigned foo(const char *CurPtr) {
unsigned i = 1;
while (i != 7) {
++i;
if (CurPtr[i-1] != CurPtr[0])
break;
}
return i;
}
I've attached this counter-test case as s.ll. You can see that with
loop-rotate.patch, we still don't rotate it.
The real problem is that loop rotate's heuristics don't handle multiple exits.
When this bug was filed, I attempted some fixes that teach loop rotate to
identify a useful loop guard (as opposed to some arbitrary loop exit), then
effectively move that test to the bottom of the loop. However, I was never
satisfied that I had something general and robust, and there were many
regressions to investigate. Really doing this right way is involved and proably
requires coordination among loop passes.
Since the bug has been around a long time and I still haven't fixed it the
right way, I'll went ahead with a quick fix.
The fix is in r181230.
As Ben pointed out, moving the call to simplifyLoopLatch somewhat defeats its
purpose, which is to prevent useless rotation. However, the same could be
argued of the current implementation: if the loop is rotated anyway, it defeats
the purpose of simplifying the latch.
I implemented an alternative that disabled the loop exit check if
simplification occured and benchmarked both. There were just too many
regressions for me to investigate with Michele's loop-rotate patch so
I went with my alternative instead. Attached is a spreadsheet
(r181103-latchcheck-vs-postsimplify.numers) comparing the two. The red
cells are regressions with loop-rotate.patch vs. r181230.
There are also some regressions with my patch, which show up green in the
spreadsheet. These are now tracked here:
<rdar://problem/13817079> LoopRotate needs better heuristics.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>