Skip to content

The unroll_count pragma does not work together with vectorize(disable) #75839

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

Open
mveriksson opened this issue Dec 18, 2023 · 0 comments · May be fixed by #135163
Open

The unroll_count pragma does not work together with vectorize(disable) #75839

mveriksson opened this issue Dec 18, 2023 · 0 comments · May be fixed by #135163

Comments

@mveriksson
Copy link
Contributor

When I try to use both unroll_count(8) and vectorize(disable) on a loop, I get unexpected results.

Example, compiled with -O3:

typedef struct { int i, j; } Pair;
void j(Pair *p);
void f(void) {
    Pair p[640];
#pragma clang loop unroll_count(8) vectorize(disable)
    for (int j = 0; j < 640; ++j)
        p[j] = (Pair){0, 1000};
    j(p);
}

The loop is unrolled with factor 5 instead of 8 as specified in the unroll_count: https://godbolt.org/z/Keoxqob57

If I change the trip count to 64, it's completely unrolled which is also unexpected.

If I remove "vectorize(disable)", the loop is unrolled with factor 8 as I expect.

I think there are some issues with followup metadata. The unroll_count is placed in a "llvm.loop.vectorize.followup_all" metadata node, and vectorization is disabled, meaning the followup metadata isn't promoted to become normal loop metadata. So the UnrollPass doesn't see it.

For the unexpected full unroll with a lower trip count: this happens in the FullUnrollPass, which is before LoopVectorizePass, so the followup metadata hasn't even had a chance to become normal loop metadata. This seems like a flaw in the followup metadata design.

kasuga-fj added a commit to kasuga-fj/llvm-project that referenced this issue Apr 10, 2025
There are two ways to disable vectorization with pragma,
`vectorize(disable)` and `vectorize_width(1)`. The document
(https://clang.llvm.org/docs/LanguageExtensions.html#vectorization-interleaving-and-predication)
states:

> Specifying a width/count of 1 disables the optimization, and is
equivalent to `vectorize(disable)` or `interleave(disable)`.

So the behavior should be the same when using either is used. However,
the current implementation doesn't satisfy this. When
`vectorize(disable)` is specified, it is converted internally to the
same as `vectorize_width(1)`, and the metadata is generated as if
vectorization is not explicitly specified as enabled or disabled. This
can cause a problem when other transformations are also specified by
pragma. For example, if `unroll_count(8)` is specified, the loop should
have a metadata that contains the information directly, like:

```
!loop0 = !{!"loop0", !vectorize_disable, !unroll}
!vectorize_disable = {...}
!unroll = !{!"llvm.loop.unroll_count", i32 8}
```

But now it's attached into followup metadata for vectorization.

```
!loop0 = !{!"loop0", !vectorize_width, !followup}
!vectorize_width = !{!"llvm.loop.vectorize.width", i32 1}
!followup = !{!"llvm.loop.vectorize.followup_all", !unroll}
!unroll = !{!"llvm.loop.unroll_count", i32 8}
```

As a result, the unroll attributes are ignored because the vectorization
is not applied.

This patch fixes this issue by generating metadata that disables
vectorization when `vectorize(disable)` or `vectorize_width(1)` are
specified.

The document also says:

> If the transformation is disabled (e.g. vectorize(disable)), that
takes precedence over transformations option pragmas implying that
transformation.

Therefore, if vectorization is disabled, all other vectorize options
like `vectorize_predicate` should be ignored. This patch also omits to
generate attributes for vectorization when it is disabled.

Fix llvm#75839
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.

2 participants