[PATCH] D22952: [LoopVectorize] Detect loops in the innermost loop before creating InnerLoopVectorizer

[Adam Nemet via llvm-commits]

anemet added a comment.

In https://reviews.llvm.org/D22952#503062, @chandlerc wrote:

> At a somewhat fundamental level, LLVM uses the term "loop" to mean "natural loop", and not any arbitrary cycle. Similarly, throughout LoopInfo and the loop pass manager we model "inner loop" as the innermost natural loop, regardless of whether there are cycles nested within it.
>
> So while I don't have a really strong opinion about how the vectorizer is factored (I don't really work on it) I would expect an LLVM API spelled "addInnerLoop" to mean "add inner natural loop" and not "add inner cycle which happens to be a natural loop" or "add inner loop that doesn't contain a cycle".
>
> It is possible we could work to change LLVM's terminology, but I think that'd be a pretty big shift. So, if you want to filter these things in "addInnerLoop" I'd suggest trying to find a more precise name.


Sure, I would be certainly OK changing the name if that is misleading.

I look at this more from the POV of which one is conservatively more correct?

I think that in addInnerLoop, adding a loop that actually has another non-natural loop nested is more error-prone than not.  I actually believe that we have other (inner-most) loop optimizations that have the same bug as the vectorizer that use a similar code to enumerate inner-most loops.  Thus it would probably be better to have a central utility that gives you natural loops that are guaranteed to have an acyclic body (whatever the name is), i.e. inner-most in the most conservative sense.


https://reviews.llvm.org/D22952