[PATCH] D75013: [LoopTerminology] Rotated Loops

Wed Mar 4 10:05:57 PST 2020

Meinersbur added a comment.

In D75013#1904564 <https://reviews.llvm.org/D75013#1904564>, @kbarton wrote:

> The comment about single successor I got from the original patch for loop rotation (https://reviews.llvm.org/D22630).

This is not the original implementation of LoopRotate and wasn't even committed. IIUC, current LoopRotate only clones the loop header (i.e. the loop exit condition must be its terminator), D22630 <https://reviews.llvm.org/D22630> would also rotate loops where the loop exit condition is somewhere else.

================
Comment at: llvm/docs/LoopTerminology.rst:195-196
+
+This transformation canonicalizes the loop latch to have
+a single successor, which implies that the loop latch
+is also an exiting block. It is done by the `loop-rotate`
----------------
baziotis wrote:
> Meinersbur wrote:
> > baziotis wrote:
> > > Meinersbur wrote:
> > > > baziotis wrote:
> > > > > Meinersbur wrote:
> > > > > > [serious] By definition, a latch has a backedge to the header. If the latch had just a single successor, there could not be another edge to outside the loop.
> > > > > > 
> > > > > > Using C code to describe the effect is higher level than the IR-level it is actually performed on. What a latch is is not obvious in the C code. LoopRotate also copies the header block, which might be interesting to mention.
> > > > > > [serious] By definition, a latch has a backedge to the header. If the latch had just a single successor, there could not be another edge to outside the loop.
> > > > > 
> > > > > Yes, given that the latch contains the condition (as in a do-while loop), that was not so smart on my part. :P
> > > > > But honestly, the 2 videos confused me in this part:
> > > > > - On the one video it says that the latch has a single successor. I assume was a typo.
> > > > > - On the other, it says that it has a single predecessor. Which, in the example given, I don't think is true. Besides that,
> > > > > I don't see why having a single predecessor is important. What do I miss?
> > > > > 
> > > > > > Using C code to describe the effect is higher level than the IR-level it is actually performed on. What a latch is is not obvious in the C code. LoopRotate also copies the header block, which might be interesting to mention.
> > > > > 
> > > > > Yes, I'll change it to IR. How about if I put view-cfg-produced image?
> > > > Could you point me to where in the videos this statement is made? It is totally possible that we make mistakes as well.
> > > > 
> > > > I also would not see why the latch would need to have a single predecessor.
> > > > 
> > > > > How about if I put view-cfg-produced image?
> > > > 
> > > > In image illustration would be nice.
> > > > Could you point me to where in the videos this statement is made? It is totally possible that we make mistakes as well.
> > > > I also would not see why the latch would need to have a single predecessor.
> > > Of course, no bad intention. I'm not the best to identify the mistake anyway. :)
> > > So, in the video [[ https://youtu.be/3pRhvQi7Z10?t=287 | Writing Loop Optimizations ]], at around 4:47, there's a slide mentioning the single predecessor.
> > > Also, in the video [[ https://youtu.be/-JQr9aNagQo?t=330 | Loop Fusion, Loop Distribution and their Place in the Loop Optimization Pipeline ]], at around 5:31, there's a slide mentioning the single successor.
> > > 
> > > > In image illustration would be nice.
> > > Great.
> > The first presentation talks about a single predecessor and the second about single successor. I think it might refer to [[ https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp#L525 | this ]]:
> > ```
> >       // Preserve canonical loop form, which means that 'Exit' should have only
> >       // one predecessor. Note that Exit could be an exit block for multiple
> >       // nested loops, causing both of the edges to now be critical and need to
> >       // be split.
> > ```
> > However, AFAIK it is not directly a requirement for either loop simplified or rotated form, at least it is not checked in `isLoopSimplfyForm` or `isRotatedForm`. That is, I think it is legal (but somewhat strange) in both canonical forms for have a switch in the latch with three cases, one going to the header and two going to (the same) exit block. [[ https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Utils/LoopSimplify.cpp#L604 | Looks like LoopSimplify even reverses this change ]].
> > 
> > Maybe @kbarton can elaborate what was meant.
> Thanks for the references! The loop rotation excerpt was not very clear to me. I understood the LoopSimplify one, but seeing the bigger picture I don't understand the reasoning. I'll check tomorrow. Hopefully @kbarton can help.
> Btw, reading through these, it seems we should add to the docs what is a deoptimizing edge and what is a critical edge.
> It then might be clearer to read the commits: https://github.com/llvm/llvm-project/commit/2f6987ba61cc31c16c64f511e5cbc76b52dc67b3
> and
> https://github.com/llvm/llvm-project/commit/c8ca49659ac479b5be0349528d42179f40fc553b#diff-7430b177373b789cad7153ebb942f708R11
> I understood the LoopSimplify one, but seeing the bigger picture I don't understand the reasoning.

```
for (int i = 0; i < n; ++i) {
  auto v = *p;
  use(v);
}
```
The value of `p` is loop-invariant, but the following transformation is illegal (to avoid repeatedly loading the same value from memory):
```
auto v = *p;
for (int i = 0; i < n; ++i) {
  use(v);
}
```
because if `n==0`, the address in `p` is dereferenced while it was not in the original loop. `p` might be null and the transform code would segfault while the original code did not. Solution is to insert a check whether the loop body is executed at least once:
```
if (0 < n) { // loop guard
  auto v = *p;
  for (int i = 0; i < n; ++i) {
    use(v);
  }
}
```
Now the loop condition before entering the first iteration is redundant, we already checked it in the guard. We can move it to after the first iteration:
```
if (0 < n) { // loop guard
  auto v = *p;
  for (int i = 0; ; ++i) {
    use(v);
    if (i+1>=n)
      break;
  }
}
```
We just invented loop rotation.

The name 'rotation' comes from [[ https://i.imgur.com/Ju9nVVl.gif | gradually peeling/unrolling (with an infinite spiral) ]]) statements out of the loop until everything in the loop is executed at least once:
```
  int i = 0;
  while (1) {
    foo(i);
    bar(i);
    if (i >= n) return;
    ++i;
    xyzzy(i);
  }
```
 🡆
```
  int i = 0;
  foo(i);
  while (1) {
    bar(i);
    if (i >= n) return;
    ++i;
    xyzzy(i);
    foo(i);
  }
```
 🡆
```
  int i = 0;
  foo(i);
  bar(i);
  while (1) {
    if (i >= n) return;
    ++i;
    xyzzy(i);
    foo(i);
    bar(j);
  }
```
 🡆
```
  int i = 0;
  foo(i);
  bar(i);
  if (i >= n) return;
  while (1) {
    ++i;
    xyzzy(i);
    foo(i);
    bar(i);
    if (i >= n) return;
  }
```

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75013/new/

https://reviews.llvm.org/D75013