[PATCH] D150706: [LAA] Update MaxSafeDepDistBytes when non-unit stride

[Michael Maitland via Phabricator via llvm-commits]

michaelmaitland added a comment.

In D150706#4354633 <https://reviews.llvm.org/D150706#4354633>, @reames wrote:

> I'm not following why you think the existing computation of the MaxSafeDepDistBytes is wrong.  Reading through the code, this appears to be simply an early out.  We're doing an N^2 comparison, and if we've already found a distance of X, any later pair with distance > X can't be safe as part of the set.  We could instead return the fact this pair has no dependence - it may not - but there's no point to this because there must be some other pair with an unsafe dependence which will prevent vectorization of the whole set.
>
> That's honestly a weird optimization, but it does look to be only a compile time optimization.  It does seem to screw up the dependency reporting, but that appears to only drive debugging output.
>
> (Note, the code structure doesn't make this obvious, but MaxVF is computed using "Distance" given the previous return and min clause.)
>
> The legality influencing bit should be in the computation of MaxVF.  It does look to me like the computation of MaxVF hasn't been updated to match MinDistanceNeeded (i.e. doesn't special case last iteration), but that seems to be a missed optimization, not a miscompile.
>
> Can you clarify whether you think this is a miscompile or a missed optimization?  Your description seems to indicate miscompile, but from what I can tell, having an unnecessarily low MaxVF should just decrease the VF used, and thus maybe lead to missed optimization.  I don't see how you get from that to miscompile.

The test case I have added shows how the MaxSafeDepDistBytes was incorrectly calculated prior to accounting for loop stride. The comment in the code and in the commit message explain it in more detail. It is possible to vectorize that loop with a vector loop stride that abides by the MaxSafeDepDistBytes. But if this value is incorrect coming from LAA, then it will lead to a miscompile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150706