[PATCH] D68789: [LoopNest]: Analysis to discover properties of a loop nest.

[Ettore Tiotto via Phabricator via llvm-commits]

etiotto marked 2 inline comments as done.
etiotto added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LoopNestAnalysis.h:124
+  /// Return true if the loop nest is perfect and false otherwise.
+  bool isPerfect() const { return MaxPerfectDepth == getNestDepth(); }
+
----------------
Meinersbur wrote:
> etiotto wrote:
> > Meinersbur wrote:
> > > etiotto wrote:
> > > > Meinersbur wrote:
> > > > > [discussion] I understand "perfectly nested" as a property of a set of loops. That is, in
> > > > > ```
> > > > >   for (int i)
> > > > >     for (int j) {
> > > > >       preStmt();
> > > > >       for (int k) ...
> > > > >       postStmt();
> > > > >     }
> > > > > ```
> > > > > I'd understand the loop `i` and `j` to be a perfect loop nest. What code is in the body does not matter.
> > > > Correct. In your example loops i and j are perfectly nested with respect to each other. Loop j and k on the other hand are not perfectly nested because of the present of stmts between the loops. The entire loop nest is imperfect because of that.
> > > > 
> > > > So the isPerfect member function returns true if and only if any adjacent pair of loops in the nest are perfectly nested with respect to each other (not only the othermost 2 loops). That is the only loop containing user code is the innermost loop. 
> > > I am arguing that we should remove `isPerfect` entirely and replace by something that returns the maximum perfect loop nest depth. To avoid a difference whether in
> > > ```
> > > for (int i)
> > >   for (int j) {
> > >     OutlinedBody(i,j);
> > >   }
> > > ```
> > > the function `OutlinedBody` is inlined or not.
> > > 
> > > Also, since this is a loop analysis, there should be a function determining whether the currently analyzed loop is the outermost loop nest. A loop pass could use this to skip optimizing a loop of depth 2 when when including another outer loop it could optimize a loop nest depth of 3 (since the order of optimization is determined by the LoopPassManager and can be arbitrary).
> > Hi Michael, the isPerfect() member function is IMO convenient for consumers that want to know whether the entire loop nest is perfect (from a syntactic standpoint). However is not strictly necessary so I'll remove it and add this member function:  
> > 
> > /// Return true if the given loop \p L is the outermost loop in the nest.
> > bool isOutermost(const Loop &L) const;
> In my domain it is very common to have additional loops inside otherwise perfectly nested loops (e.g. stencils). Here is a simple example of a grayscale conversion:
> 
> ```
> for (int y = 0; y < Y; ++y)
>   for (int x = 0; x < X; ++x) {
>       double luminance = 0;
>       for (int c = 0; c < 3; ++c)
>         luminance += colored[x][y][c] / 3.0;
>       grayscale[x][y] = luminance;
>     }
> ```
> 
> For optimization, we would e.g. interchange the x and y loops. If the interchange uses `isPerfect` to determine whether the loop can be transformed, it will have to bail out (Let's ignore that the inner loop could be unrolled before loop interchange, many stencils in HPC have inner loops with dynamic trip count or are too large).
> 
> That is, using `isPerfect` would stop many HPC applications, maybe even a majority of our programs in production, to be optimized. For the syntactic property of two loops having no in-between code (with size-effects), what's inside the innermost body should be irrelevant and it would be surprising to me if it did. This is why I'd to not even include `isPerfect` in the API. 
> 
> If a pass really cannnot handle nested loops, it can still ask LoopInfo's innermost `Loop` whether it contains another loop. This would be an explicit restriction by the pass.
@meinersbur I have uploaded a patch that adds the a function which can be used to retrieve the list of loops that are perfect within each other in the nest. In your example it would return loop-y and loo-x into a list (and loop-c into another list by itself). Please take a look, it should address your concern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68789