[PATCH] D65164: Define some basic terminology around loops in our documentation

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert reopened this revision.
jdoerfert added a comment.

> The first point is is incorrect. A single loop header can be the header of multiple nested loops.

I fear we disagree on the definition of "a loop".

I was thinking we describe the `llvm::Loop` here as it is discovered by LoopInfo. 
It seems you want to go for something different (or I am too tired to see my mistake).
[For the record, I think it often make sense to use a different definition than the one
 I want to see here. For now, I am worried about inconsistencies in our definitions and
 wrt. to the implementation.]

---

Let's try to unravel this (I think this is fun). I'm curious, what loops would you identify in the examples below.
I will discuss two "loop definitions" that seem natural to me but if you have something else in mind I would like to hear it.

  ah:  br ..., label %bl1, label %bl2
  al1: b %ah;
  al2: br %ah;



  bh:  br ..., label %bh, label %bl2
  bl2: br %bh;



  ch:  br ..., label %ch, label %ch

For me, all three examples have a single loop:
L_A: (header: `ah`, latches: `al1`, `al2`)
L_B: (header: `bh`, latches: `bh`, `bl2`)
L_C: (header: `ch`, latches: `ch`)
Basically, a loop is a header and a set of blocks dominated by the header such that you can reach the header from every one of them without leaving the set.

Now I could see why one would like to define it differently, e.g., via cycles in the CFG, so based on (back)edges.
That should result in two loops per example:
L_A1: (header: `ah`, latch: `al1`)
L_A2: (header: `ah`, latch: `al2`)
L_B1: (header: `bh`, latch: `bh`)
L_B2: (header: `bh`, latch:  `bl1`)
L_C1: (header: `ch`, latch: `ch`)
L_C2: (header: `ch`, latch: `ch`)

What I dislike about this definition, except that it is not what loop info reports, is:

- L_A1 and L_A2 are neither disjoint nor is one properly nested in the other.
- L_C1 and L_C2 are identical.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65164