[PATCH] D88408: [docs] Revise loop terminology reference.

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis accepted this revision.
baziotis added a comment.
This revision is now accepted and ready to land.

> I think the document is more useful as a reference than a text-book reading.

Yes, that's why the item list was very useful IMO and I think this still is as useful.



================
Comment at: llvm/docs/LoopTerminology.rst:144
+
+ * The number of executions of the loop header before leaving the loop is the **loop trip count**. If the loop should not be executed at all, a **loop guard** must skip the entire loop:
+
----------------
Meinersbur wrote:
> baziotis wrote:
> > Meinersbur wrote:
> > > baziotis wrote:
> > > > I think that the reference to the loop guard here is out of the blue it raises questions. It may be better to just leave the loop rotate part reference it.
> > > > 
> > > > Also, we might want to add "loop trip count (also called iteration count)".
> > > The existence of a loop guard is independent of the loop-rotated normal form, the same way a preheader may exist before LoopSimplify.
> > Yes, but does LoopInfo recognize it if it's not from LoopRotate ? e.g. if I write `if (cond) { do { ... } while (cond); }` in the source, will it recognize that it has a guard ? I thought that for the compiler, a guard is something only the compiler inserts while with the above wording, a guard can also exist already.
> > 
> > I think I explained badly my confusion when I read this: I think it's not clear what is a loop guard, who inserts it (if anyone) and when (partly this resulted from the use of "should not" and "must" above). Maybe something like that?
> > A guard is any block that dominates the header (it can be the header) and guards the body of the loop from not being entered if the loop condition is false before the first iteration. The compiler may insert such a guard after some transformation to ensure correctness (e.g. LoopRotate).
> > 
> > The wording could be better but please tell me what you think.
> LoopRotation does not recognize guards. Guards being created is just the typical sideeffect of moving the first execution of the loop header (which in a typical for-loop contains the loop condition) before the loop, thus creating a guard. There could also be multiple "guards" skipping the loop checking for different conditions.
> 
> I used introduced the loop guard here to illustrate that a loop trip count of 1 does not mean the the loop body (in the sense of the input language) has been executed, and how that is avoided (possibly also by frontend IR generators), and to show and name a pattern one often finds in the wild.
> 
> A loop guard cannot be the header, the point of the guard is to skip the loop. The loop is not skipped if one first has to enter the loop to check the guard condition.
> For the point of optimization, the advantage of not entering the loop at all is stronger assumptions for the preheader (e.g. more things can be hoisted into the preheader), but I would prefer to not go into optimization details here; there is a link to the LoopRotation section. I did not even explain what the preheader use useful for.
Thanks for the explanation, let's leave "why insert a loop guard ?" out. How about making a little bit more explicit what is a loop guard (btw I realized that I too don't make it explicit in LoopRotation) ? With the current wording, the reader may wonder "what is a loop guard?". For example:
"... must skip the entire loop. A loop guard is a block that dominates the header, it is not part of the loop, and tests the loop condition before the first iteration in order to potentially skip the loop entirely".

> LoopRotation does not recognize guards.

Yes, I asked if LoopInfo recognizes them. Mainly my question is: Does the compiler realize if a loop guard is already there
or only because it inserts one ? (e.g. as part of LoopRotation) But anyway, feel free to ignore this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88408