[Analysis][Docs] Parents of loops documentation

[baziotis]

Summary

Recently I had to use it and although one assumes it returns null if there's no parent loop, I think it helps to doc it.

[fhahn]

llvm/include/llvm/Analysis/LoopInfo.h

106

Use ///. That's how doxygen comments are defined. It might also be worth clarifying the parent loop relationship in https://llvm.org/docs/LoopTerminology.html.

For the comment here I would suggest to start with a brief first sentence and maybe another one that explain the parent relationship, like

Return the parent loop if it exists or nullptr otherwise.

[fhahn]

llvm/include/llvm/Analysis/LoopInfo.h

106

Use ///. That's how doxygen comments are defined. It might also be worth clarifying the parent loop relationship in https://llvm.org/docs/LoopTerminology.html.

Reading this back I realized it might sound a bit harsh. That was not the intention, sorry!

[baziotis]

llvm/include/llvm/Analysis/LoopInfo.h

106

No problem, thanks for the review! To change the LoopTerminology.html I change the /llvm/docs/LoopTerminology.rst right?

[fhahn]

llvm/include/llvm/Analysis/LoopInfo.h

106

Yep, that would be great!

[baziotis]

• Address comment.

[Meinersbur]

llvm/docs/LoopTerminology.rst

123–124

[grammar] "that is it is"

I also don't understand this part of a definition. Does it clarify in what positions a loop can be (it makes it an unusually structured dictionary definition -- I'd expect the first sentence to explain what a "parent loop" is followed by further explanations)? IMHO the LoopInfo being a forest is enough information.

llvm/include/llvm/Analysis/LoopInfo.h

106

Another suggestion: "Return the parent loop or nullptr for topmost loops"

[baziotis]

llvm/docs/LoopTerminology.rst

123–124

Ok, thanks for the comment, indeed the definition structure is kind of bad. Regarding the last note, I think it's not stated in the terminology (although it's stated in some parts of LoopInfo.h). Do you think it would be good to add this clarification?
Or maybe a last point in the introduction: "A loop can have only a single parent loop (in which it is entirely contained)". Something like that. Basically just to make obvious that a loop can have only a single parent.

llvm/include/llvm/Analysis/LoopInfo.h

106

Ok. I'm going with "top level" for consistency with the terminology but please comment if I shouldn't.

[Meinersbur]

llvm/docs/LoopTerminology.rst

123–124

I'd expect that tree concepts (node, root, leaf, child, parent, sibling, ancestor, descendent, ...) are known by a majority of programmers, but you should go for what you think is needed for someone who does not know terminology already (i.e. not me). I will look for that it's correct and understandable.

[baziotis]

llvm/docs/LoopTerminology.rst

123–124

Ok great! I think just a small note that loops form a forest of trees (or smth like that) is good. It's mentioned in LoopInfo.h but from the terminology it's not obvious so I think it's beneficial to a newcomer.

[baziotis]

Address comments. The addition in the terminology might be a little too short.

[Meinersbur]

LGTM

[baziotis]

In D74890#1887399, @Meinersbur wrote:

LGTM

Thanks! Note: I don't have commit permission, you may commit it whenever you can.

llvm/docs/LoopTerminology.rst

 
 * Every loop must have a header block, and some set of predecessors
   outside the loop.  A loop is allowed to be statically infinite, so
   there need not be any exiting edges.
 
 * Any two loops are either fully disjoint (no intersecting blocks), or
   one must be a sub-loop of the other.
 
+* Loops in a function form a forest. One implication of this fact
+  is that a loop either has no parent or a single parent.
+
 A loop may have an arbitrary number of exits, both explicit (via
 control flow) and implicit (via throwing calls which transfer control
 out of the containing function).  There is no special requirement on
 the form or structure of exit blocks (the block outside the loop which
 is branched to).  They may have multiple predecessors, phis, etc...
 
 Key Terminology
 ===============
while (..) {
     do {
       do {
         // <-- block of interest
         if (exit) break;
       } while (..);
     } while (..)
   }
 
 LoopInfo
 ========
 
 LoopInfo is the core analysis for obtaining information about loops.
 There are few key implications of the definitions given above which
 are important for working successfully with this interface.
 
 * LoopInfo does not contain information about non-loop cycles.  As a
   result, it is not suitable for any algorithm which requires complete
   cycle detection for correctness.

llvm/include/llvm/Analysis/LoopInfo.h

unsigned getLoopDepth() const {
     assert(!isInvalid() && "Loop not in a valid state!");
     unsigned D = 1;
     for (const LoopT *CurLoop = ParentLoop; CurLoop;
          CurLoop = CurLoop->ParentLoop)
       ++D;
     return D;
   }
   BlockT *getHeader() const { return getBlocks().front(); }
+  /// Return the parent loop if it exists or nullptr for top
+  /// level loops.
+
+  /// A loop is either top-level in a function (that is, it is not
+  /// contained in any other loop) or it is entirely enclosed in
+  /// some other loop.
+  /// If a loop is top-level, it has no parent, otherwise its
+  /// parent is the innermost loop in which it is enclosed.
   LoopT *getParentLoop() const { return ParentLoop; }
 
   /// This is a raw interface for bypassing addChildLoop.
   void setParentLoop(LoopT *L) {
     assert(!isInvalid() && "Loop not in a valid state!");
     ParentLoop = L;
   }