[PATCH] D65257: Describe resticted form of loops in the new loop terminogy documentation

[Philip Reames via Phabricator via llvm-commits]

reames planned changes to this revision.
reames marked 3 inline comments as done.
reames added inline comments.


================
Comment at: docs/LoopTerminology.rst:138-139
+core to the transforms we generally wish to perform.  Rather than a single
+catch all restriction, we have a number of fine grained predicates which
+can be uses as preconditions for loop transforms.  
+
----------------
Meinersbur wrote:
> I don't get this. Where can these predicates be used? More importantly, which pass ensures the restricted/normal/canonical forms?
> 
> I'd not describe a restricted loop forms in terms of an API call. That's what the merthod documentation/doxygen is for. If you are not a frequent user of the Loop class, it is not obvious what it means when e.g. `getLoopLatch()` return nullptr. The naive interpretation is "there is not latch". Rather it should be described as property of the CFG).
Paragraph A - Many passes bailout with these predicates.  Mostly ensured by LoopSimplify, but other passes help too.

Paragraph B - I agree that the doxygen should have some of this, but the implications seem like possible overkill.  I could be convinced that this needs to be rewritten as a comment patch though, what do others think?


================
Comment at: docs/LoopTerminology.rst:148
+  be distinguished by their header block.  There can not exist a nested loop
+  which shares the same header if the loop contains only one backedge.
+
----------------
jdoerfert wrote:
> Loops, assuming "natural loops identified by LoopInfo", can always be identified/distinguished by their header blocks regardless of the backedges. The situation is different for irreducible loops though.
> 
> Maybe I misunderstood what you wanted to say. Can you clarify, give an example?
Yep, this was a mistaken assumption on my part.  I'll rephrase along the lines of https://reviews.llvm.org/D65299, once we're happy with the wording there.


================
Comment at: docs/LoopTerminology.rst:160
+  
+It is generally reasonable to expect the above properties hold for loops of
+interest when a LoopPass runs inside of the LoopPassManager.  Additionally,
----------------
Meinersbur wrote:
> No; `LoopSimplifyPass` must have been added before or `simplifyLoop` run on it manually.
Correct, but isn't it generally reasonable to assume that has happened by the point a loop pass runs?  I'll admit, my wording was influenced by how things used to work in the old pass manager.  Any suggestions on reframing?


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

https://reviews.llvm.org/D65257