[LLVMbugs] [Bug 10293] Load not hoisted out of loop with multiple exits

[bugzilla-daemon at llvm.org]

http://llvm.org/bugs/show_bug.cgi?id=10293

Andrew Trick <atrick at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
           Assignee|unassignedbugs at nondot.org   |atrick at apple.com

--- Comment #10 from Andrew Trick <atrick at apple.com> ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20130506/4b4f13a5/attachment.html>