Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Loop Vectorizer] Loop not vectorized when accessing the same pointer with different offsets. #87336

Closed
komalon1 opened this issue Apr 2, 2024 · 2 comments · Fixed by #88039

Comments

@komalon1
Copy link
Contributor

komalon1 commented Apr 2, 2024

See reproducer in https://godbolt.org/z/v7fqorb1h.
a2 = a1 + 256;
a2 has an interleaved store access, and a1 has a simple linear access.
There are no actual dependencies between them when VF is 4, and it looks safe for the loop to be vectorized.
BTW, changing a2[2*i] to a2[i] makes vectorization safe.

@vfdff
Copy link
Contributor

vfdff commented Apr 3, 2024

gcc support this: https://godbolt.org/z/zGjhha5qx

@fhahn
Copy link
Contributor

fhahn commented Apr 4, 2024

Looks like there are a couple of improvements needed, looking now

fhahn added a commit that referenced this issue Apr 8, 2024
Add a number of LAA test cases with both forward and backward
dependences with non-constant strides and dependence distances.

This includes test coverage for
#87336

Also includes a LoopLoadElimination test to make sure the pass does not
crash on non-constant dependence distances.
fhahn added a commit to fhahn/llvm-project that referenced this issue Apr 8, 2024
Extend LoopAccessAnalysis to support different strides and as a
consequence non-constant distances between dependences using SCEV to
reason about the direction of the dependence.

In multiple places, logic to rule out dependences using the stride has
been updated to only be used if StrideA == StrideB, i.e. there's a
common stride.

We now also may bail out at multiple places where we may have to set
FoundNonConstantDistanceDependence. This is done when we need to bail
out and the distance is not constant to preserve original behavior.

I'd like to call out the changes in global_alias.ll in particular.
In the modified mayAlias01, and mayAlias02 tests, there should be no
aliasing in the original versions of the test, as they are accessing 2
different 100 element fields in a loop with 100 iterations.

I moved the original tests to noAlias15 and noAlias16 respectively,
while updating the original tests to use a variable trip count.

In some cases, like different_non_constant_strides_known_backward_min_distance_3,
we now vectorize with runtime checks, even though the runtime checks
will always be false. I'll also share a follow-up patch, that also uses
SCEV to more accurately identify backwards dependences with non-constant
distances.

Fixes llvm#87336
fhahn added a commit that referenced this issue Apr 25, 2024
…EV. (#88039)

Extend LoopAccessAnalysis to support different strides and as a
consequence non-constant distances between dependences using SCEV to
reason about the direction of the dependence.

In multiple places, logic to rule out dependences using the stride has
been updated to only be used if StrideA == StrideB, i.e. there's a
common stride.

We now also may bail out at multiple places where we may have to set
FoundNonConstantDistanceDependence. This is done when we need to bail
out and the distance is not constant to preserve original behavior.

Fixes #87336

PR: #88039
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants