[PATCH] D17332: [LV] Fix PR26600: avoid out of bounds loads for interleaved access vectorization

[Hal Finkel via llvm-commits]

hfinkel added a comment.

In http://reviews.llvm.org/D17332#356879, @sbaranga wrote:

> In http://reviews.llvm.org/D17332#356693, @hfinkel wrote:
>
> > How complicated would it be, instead of bailing out when we have a group without the last member, to peel off the last vector iteration instead (i.e. jump to the scalar tail loop one vector-loop iteration "early")?
> >
> > It seems like that would be a better solution (although, if you agree, but it seems too complicated to implement for the release branch, I'm fine with taking this (and pulling it into the release branch), and then implementing the better solution in trunk only).
>
>
> That would work (and I agree it would be a better solution), but I don't have a good estimate of how much it that would be to implement (there are also probably a lot of edge cases there).
>
> I can also think of another way to fix this in some cases which wouldn't involve the scalar remainder. For example, in the following code (similar to the example from the PR):
>
>   struct A {
>     int a;
>     int b;
>   }
>   
>   for (int i = 0; i < 1000; ++i) {
>     a[i].b *= 2;
>   }
>   
>
> instead of doing the wide load from a[i].b we could do it from a[i].a and move the problem to knowing that the start is ok to load from (which we do know is safe), and it avoids the need for a scalar remainder. This might also work well if the loop was previously modified by LoopRotate (if it strips the first iteration, we can use the fact that we have dominating loads to the start of the memory location to deduce that the transformation would be safe).


I think that, when we can prove that's safe, that is also a good solution. I'm not sure we can always prove this, however. The underlying allocation in this case, for example, need not include a[0].a (although maybe with some higher-level TBAA-like information, we could know that it must).

> Would it be ok to commit this as is for now?


As previously stated, sure :-)

> Thanks,

> Silviu



Repository:
  rL LLVM

http://reviews.llvm.org/D17332