[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

120–121

[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

120–121

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

120–121

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

120–121

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

while (..) {
     do {
       do {
         // <-- block of interest
         if (exit) break;
       } while (..);
     } while (..)
   }
 
+Parent Loop - A loop is either a top level loop, that is it
+is not enclosed in any other loop or it is a sub-loop of another.
+A parent of a loop is the innermost loop in which it is enclosed in.
+A top level loop does not have a parent.
+
 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

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 otherwise.
+
+  /// 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 in.
   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;
   }