[PATCH] D24557: [Loop Vectorizer] Simplified GEP cloning. NFC.

[Michael Kuperstein via llvm-commits]

mkuper added a comment.

Ugh. Ignore my last comment, this is wrong.

We have *almost* fixed what "uniform" means in the vectorizer (thanks again, Matt), but not quite.

(a) LVL::isUniform() and LAI::isUniform() are basically equivalent to isLoopInvariant(). They imply the value is equal on all lanes.
(b) LVL::isUniformAfterVectorization() refers to what @anemet is talking about - it's a property of how the value is used, specifically that we only use the lane 0 value.

So:
@anemet: this is already phrased in terms of loop invariance.
@delena: I no longer agree with your example.

If we have:

  %foo = getelementptr inbounds i8, i8* %ptr, i64 %ind

And %foo is consecutive, then one of the following holds:

1. %ptr is loop-invariant (uniform), and %ind is consecutive.
2. %ptr itself is consecutive, and %ind is loop-invariant (uniform).

So, I think I understand why the assert fails now. 
For case 1, where %ind is consecutive, the assert will pass, because %ind is the "induction operand" (that is, it's in the "right place" in the GEP). 
For case 2, where %ptr is consecutive, the assert may fail, because %ptr has to be consecutive, but doesn't really have to be the IV.

This also possibly explains why the assert started failing after you improved consecutivity detection. If we have something like this:

  %p = getelementptr inbounds i8, i8* %iv, i64 0
  %q = getelementptr inbounds i8, i8* %p, i64 1

Then as long as only %p is considered consecutive, everything is fine, but if %q is also considered consecutive, the assert should fail.

Does this make sense?


Repository:
  rL LLVM

https://reviews.llvm.org/D24557