Skip to content

[clang][CodeGen] Fix metadata when vectorization is disabled by pragma #135163

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
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kasuga-fj
Copy link
Contributor

@kasuga-fj kasuga-fj commented 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 (under the assumption that scalable is not explicitly specified). 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), vectorize_width(1) or vectorize_width(1, fixed) are specified. Note that if scalable is explicitly enabled (like vectorize_width(1, scalable)), vectorization is still processed.

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 #75839

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
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Apr 10, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 10, 2025

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Ryotaro Kasuga (kasuga-fj)

Changes

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 #75839


Full diff: https://github.com/llvm/llvm-project/pull/135163.diff

5 Files Affected:

  • (modified) clang/lib/CodeGen/CGLoopInfo.cpp (+3-4)
  • (modified) clang/test/CodeGenCXX/pragma-loop-pr27643.cpp (+6-6)
  • (modified) clang/test/CodeGenCXX/pragma-loop-predicate.cpp (+11-11)
  • (modified) clang/test/CodeGenCXX/pragma-loop-safety.cpp (+2-2)
  • (modified) clang/test/CodeGenCXX/pragma-loop.cpp (+25-4)
diff --git a/clang/lib/CodeGen/CGLoopInfo.cpp b/clang/lib/CodeGen/CGLoopInfo.cpp
index 2b7d7881ab990..e9d622d78bf6d 100644
--- a/clang/lib/CodeGen/CGLoopInfo.cpp
+++ b/clang/lib/CodeGen/CGLoopInfo.cpp
@@ -200,7 +200,8 @@ LoopInfo::createLoopVectorizeMetadata(const LoopAttributes &Attrs,
   LLVMContext &Ctx = Header->getContext();
 
   std::optional<bool> Enabled;
-  if (Attrs.VectorizeEnable == LoopAttributes::Disable)
+  if (Attrs.VectorizeEnable == LoopAttributes::Disable ||
+      Attrs.VectorizeWidth == 1)
     Enabled = false;
   else if (Attrs.VectorizeEnable != LoopAttributes::Unspecified ||
            Attrs.VectorizePredicateEnable != LoopAttributes::Unspecified ||
@@ -643,9 +644,7 @@ void LoopInfoStack::push(BasicBlock *Header, clang::ASTContext &Ctx,
     case LoopHintAttr::Disable:
       switch (Option) {
       case LoopHintAttr::Vectorize:
-        // Disable vectorization by specifying a width of 1.
-        setVectorizeWidth(1);
-        setVectorizeScalable(LoopAttributes::Unspecified);
+        setVectorizeEnable(false);
         break;
       case LoopHintAttr::Interleave:
         // Disable interleaving by speciyfing a count of 1.
diff --git a/clang/test/CodeGenCXX/pragma-loop-pr27643.cpp b/clang/test/CodeGenCXX/pragma-loop-pr27643.cpp
index 37e29776264b6..b4614c4d39be4 100644
--- a/clang/test/CodeGenCXX/pragma-loop-pr27643.cpp
+++ b/clang/test/CodeGenCXX/pragma-loop-pr27643.cpp
@@ -40,14 +40,14 @@ void loop4(int *List, int Length) {
     List[i] = i * 2;
 }
 
-// CHECK: ![[LOOP1]] = distinct !{![[LOOP1]], [[MP:![0-9]+]], ![[VEC_WIDTH_1:.*]], ![[FIXED_WIDTH:.*]], ![[VEC_ENABLE:.*]]}
-// CHECK: ![[VEC_WIDTH_1]] = !{!"llvm.loop.vectorize.width", i32 1}
-// CHECK: ![[FIXED_WIDTH]] = !{!"llvm.loop.vectorize.scalable.enable", i1 false}
-// CHECK: ![[VEC_ENABLE]] = !{!"llvm.loop.vectorize.enable", i1 true}
+// CHECK: ![[LOOP1]] = distinct !{![[LOOP1]], [[MP:![0-9]+]], ![[VEC_DISABLE:[0-9]+]]}
+// CHECK: ![[VEC_DISABLE]] = !{!"llvm.loop.vectorize.enable", i1 false}
 
-// CHECK: ![[LOOP2]] = distinct !{![[LOOP2]], [[MP]], ![[VEC_WIDTH_2:.*]], ![[FIXED_WIDTH:.*]], ![[VEC_ENABLE]]}
+// CHECK: ![[LOOP2]] = distinct !{![[LOOP2]], [[MP]], ![[VEC_WIDTH_2:.*]], ![[FIXED_WIDTH:[0-9]+]], ![[VEC_ENABLE:[0-9]+]]}
 // CHECK: ![[VEC_WIDTH_2]] = !{!"llvm.loop.vectorize.width", i32 2}
+// CHECK: ![[FIXED_WIDTH]] = !{!"llvm.loop.vectorize.scalable.enable", i1 false}
+// CHECK: ![[VEC_ENABLE]] = !{!"llvm.loop.vectorize.enable", i1 true}
 
-// CHECK: ![[LOOP3]] = distinct !{![[LOOP3]], [[MP]], ![[VEC_WIDTH_1]], ![[FIXED_WIDTH]]}
+// CHECK: ![[LOOP3]] = distinct !{![[LOOP3]], [[MP]], ![[VEC_DISABLE]]}
 
 // CHECK: ![[LOOP4]] = distinct !{![[LOOP4]], [[MP]], ![[VEC_WIDTH_2]], ![[FIXED_WIDTH]], ![[VEC_ENABLE]]}
diff --git a/clang/test/CodeGenCXX/pragma-loop-predicate.cpp b/clang/test/CodeGenCXX/pragma-loop-predicate.cpp
index 8a25ed8de6239..4877733f6ea2a 100644
--- a/clang/test/CodeGenCXX/pragma-loop-predicate.cpp
+++ b/clang/test/CodeGenCXX/pragma-loop-predicate.cpp
@@ -37,8 +37,8 @@ void test3(int *List, int Length) {
     List[i] = i * 2;
 }
 
-// Check that disabling vectorization means a vectorization width of 1, and
-// also that vectorization_predicate isn't enabled.
+// Check that vectorize is disabled, and also that vectorization_predicate is
+// ignored.
 void test4(int *List, int Length) {
 // CHECK-LABEL: @{{.*}}test4{{.*}}(
 // CHECK: br label {{.*}}, !llvm.loop ![[LOOP4:.*]]
@@ -48,7 +48,7 @@ void test4(int *List, int Length) {
     List[i] = i * 2;
 }
 
-// Check that vectorize and vectorize_predicate are disabled.
+// Check that vectorize is disabled and vectorize_predicate is ignored.
 void test5(int *List, int Length) {
 // CHECK-LABEL: @{{.*}}test5{{.*}}(
 // CHECK: br label {{.*}}, !llvm.loop ![[LOOP5:.*]]
@@ -114,16 +114,16 @@ void test9(int *List, int Length) {
 // CHECK-NEXT: ![[LOOP3]] = distinct !{![[LOOP3]], [[MP]], [[GEN6]], [[GEN3]]}
 
 // CHECK-NEXT: ![[LOOP4]] = distinct !{![[LOOP4]], [[MP]], [[GEN10:![0-9]+]]}
-// CHECK-NEXT: [[GEN10]] = !{!"llvm.loop.vectorize.width", i32 1}
+// CHECK-NEXT: [[GEN10]] = !{!"llvm.loop.vectorize.enable", i1 false}
 
-// CHECK-NEXT: ![[LOOP5]] = distinct !{![[LOOP5]], [[MP]], [[GEN6]], [[GEN10]]}
+// CHECK-NEXT: ![[LOOP5]] = distinct !{![[LOOP5]], [[MP]], [[GEN10]]}
 
-// CHECK-NEXT: ![[LOOP6]] = distinct !{![[LOOP6]], [[MP]], [[GEN8]], [[GEN10]], [[GEN11:![0-9]+]]}
-// CHECK-NEXT: [[GEN11]] = !{!"llvm.loop.vectorize.scalable.enable", i1 false}
+// CHECK-NEXT: ![[LOOP6]] = distinct !{![[LOOP6]], [[MP]], [[GEN10]]}
 
-// CHECK-NEXT: ![[LOOP7]] = distinct !{![[LOOP7]], [[MP]], [[GEN8]], [[GEN12:![0-9]+]], [[GEN11]], [[GEN3]]}
-// CHECK-NEXT: [[GEN12]] = !{!"llvm.loop.vectorize.width", i32 4}
+// CHECK-NEXT: ![[LOOP7]] = distinct !{![[LOOP7]], [[MP]], [[GEN8]], [[GEN11:![0-9]+]], [[GEN12:![0-9]+]], [[GEN3]]}
+// CHECK-NEXT: [[GEN11]] = !{!"llvm.loop.vectorize.width", i32 4}
+// CHECK-NEXT: [[GEN12]] = !{!"llvm.loop.vectorize.scalable.enable", i1 false}
 
-// CHECK-NEXT: ![[LOOP8]] = distinct !{![[LOOP8]], [[MP]], [[GEN6]], [[GEN10]], [[GEN11]]}
+// CHECK-NEXT: ![[LOOP8]] = distinct !{![[LOOP8]], [[MP]], [[GEN10]]}
 
-// CHECK-NEXT: ![[LOOP9]] = distinct !{![[LOOP9]], [[MP]], [[GEN6]], [[GEN12]], [[GEN11]], [[GEN3]]}
+// CHECK-NEXT: ![[LOOP9]] = distinct !{![[LOOP9]], [[MP]], [[GEN6]], [[GEN11]], [[GEN12]], [[GEN3]]}
diff --git a/clang/test/CodeGenCXX/pragma-loop-safety.cpp b/clang/test/CodeGenCXX/pragma-loop-safety.cpp
index db1b6769e0514..771570f62057b 100644
--- a/clang/test/CodeGenCXX/pragma-loop-safety.cpp
+++ b/clang/test/CodeGenCXX/pragma-loop-safety.cpp
@@ -53,6 +53,6 @@ void interleave_test(int *List, int Length) {
 // CHECK: ![[INTERLEAVE_1]] = !{!"llvm.loop.interleave.count", i32 1}
 // CHECK: ![[INTENABLE_1]] = !{!"llvm.loop.vectorize.enable", i1 true}
 // CHECK: ![[ACCESS_GROUP_8]] = distinct !{}
-// CHECK: ![[LOOP2_HINTS]] = distinct !{![[LOOP2_HINTS]], [[MP]], ![[PARALLEL_ACCESSES_11:[0-9]+]], ![[UNROLL_DISABLE]], ![[WIDTH_1:[0-9]+]], ![[INTENABLE_1]]}
+// CHECK: ![[LOOP2_HINTS]] = distinct !{![[LOOP2_HINTS]], [[MP]], ![[PARALLEL_ACCESSES_11:[0-9]+]], ![[UNROLL_DISABLE]], ![[VECTORIZE_DISABLED:[0-9]+]]}
 // CHECK: ![[PARALLEL_ACCESSES_11]] = !{!"llvm.loop.parallel_accesses", ![[ACCESS_GROUP_8]]}
-// CHECK: ![[WIDTH_1]] = !{!"llvm.loop.vectorize.width", i32 1}
+// CHECK: ![[VECTORIZE_DISABLED]] = !{!"llvm.loop.vectorize.enable", i1 false}
diff --git a/clang/test/CodeGenCXX/pragma-loop.cpp b/clang/test/CodeGenCXX/pragma-loop.cpp
index 4857299f1c037..c08c42c384c04 100644
--- a/clang/test/CodeGenCXX/pragma-loop.cpp
+++ b/clang/test/CodeGenCXX/pragma-loop.cpp
@@ -194,7 +194,7 @@ void for_test_scalable(int *List, int Length) {
   }
 }
 
-// Verify for loop is performing scalable vectorization
+// Verify for loop is NOT performing vectorization because the width is 1
 void for_test_scalable_1(int *List, int Length) {
 #pragma clang loop vectorize_width(1, scalable) interleave_count(4) unroll(disable) distribute(disable)
   for (int i = 0; i < Length; i++) {
@@ -203,6 +203,24 @@ void for_test_scalable_1(int *List, int Length) {
   }
 }
 
+// Verify for loop is performing scalable vectorization
+void for_test_scalable_2(int *List, int Length) {
+#pragma clang loop vectorize_width(2, scalable) interleave_count(4) unroll(disable) distribute(disable)
+  for (int i = 0; i < Length; i++) {
+    // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_20:.*]]
+    List[i] = i * 2;
+  }
+}
+
+// Verify unroll attributes are directly attached to the loop metadata
+void for_test_vectorize_disable_unroll(int *List, int Length) {
+#pragma clang loop vectorize(disable) unroll_count(8)
+  for (int i = 0; i < Length; i++) {
+    // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_21:.*]]
+    List[i] = i * 2;
+  }
+}
+
 // CHECK-DAG: ![[MP:[0-9]+]] = !{!"llvm.loop.mustprogress"}
 
 // CHECK-DAG: ![[UNROLL_DISABLE:[0-9]+]] = !{!"llvm.loop.unroll.disable"}
@@ -220,9 +238,9 @@ void for_test_scalable_1(int *List, int Length) {
 // CHECK-DAG: ![[INTERLEAVE_16:[0-9]+]] = !{!"llvm.loop.interleave.count", i32 16}
 
 // CHECK-DAG: ![[VECTORIZE_ENABLE:[0-9]+]] = !{!"llvm.loop.vectorize.enable", i1 true}
+// CHECK-DAG: ![[VECTORIZE_DISABLE:[0-9]+]] = !{!"llvm.loop.vectorize.enable", i1 false}
 // CHECK-DAG: ![[FIXED_VEC:[0-9]+]] = !{!"llvm.loop.vectorize.scalable.enable", i1 false}
 // CHECK-DAG: ![[SCALABLE_VEC:[0-9]+]] = !{!"llvm.loop.vectorize.scalable.enable", i1 true}
-// CHECK-DAG: ![[WIDTH_1:[0-9]+]] = !{!"llvm.loop.vectorize.width", i32 1}
 // CHECK-DAG: ![[WIDTH_2:[0-9]+]] = !{!"llvm.loop.vectorize.width", i32 2}
 // CHECK-DAG: ![[WIDTH_5:[0-9]+]] = !{!"llvm.loop.vectorize.width", i32 5}
 // CHECK-DAG: ![[WIDTH_6:[0-9]+]] = !{!"llvm.loop.vectorize.width", i32 6}
@@ -241,7 +259,7 @@ void for_test_scalable_1(int *List, int Length) {
 
 // CHECK-DAG: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[WIDTH_2]], ![[FIXED_VEC]], ![[INTERLEAVE_2]], ![[VECTORIZE_ENABLE]]}
 
-// CHECK-DAG: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[MP]], ![[UNROLL_DISABLE]], ![[DISTRIBUTE_DISABLE]], ![[WIDTH_1]]}
+// CHECK-DAG: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[MP]], ![[UNROLL_DISABLE]], ![[DISTRIBUTE_DISABLE]], ![[VECTORIZE_DISABLE]]}
 
 // CHECK-DAG: ![[LOOP_6]] = distinct !{![[LOOP_6]], ![[MP]], ![[WIDTH_2]], ![[FIXED_VEC]], ![[INTERLEAVE_2]], ![[VECTORIZE_ENABLE]], ![[FOLLOWUP_VECTOR_3]]}
 
@@ -269,4 +287,7 @@ void for_test_scalable_1(int *List, int Length) {
 
 // CHECK-DAG: ![[LOOP_17]] = distinct !{![[LOOP_17]], ![[MP]], ![[UNROLL_DISABLE]], ![[DISTRIBUTE_DISABLE]], ![[FIXED_VEC]], ![[INTERLEAVE_4]], ![[VECTORIZE_ENABLE]]}
 // CHECK-DAG: ![[LOOP_18]] = distinct !{![[LOOP_18]], ![[MP]], ![[UNROLL_DISABLE]], ![[DISTRIBUTE_DISABLE]], ![[SCALABLE_VEC]], ![[INTERLEAVE_4]], ![[VECTORIZE_ENABLE]]}
-// CHECK-DAG: ![[LOOP_19]] = distinct !{![[LOOP_19]], ![[MP]], ![[UNROLL_DISABLE]], ![[DISTRIBUTE_DISABLE]], ![[WIDTH_1]], ![[SCALABLE_VEC]], ![[INTERLEAVE_4]], ![[VECTORIZE_ENABLE]]}
+// CHECK-DAG: ![[LOOP_19]] = distinct !{![[LOOP_19]], ![[MP]], ![[UNROLL_DISABLE]], ![[DISTRIBUTE_DISABLE]], ![[VECTORIZE_DISABLE]]}
+// CHECK-DAG: ![[LOOP_20]] = distinct !{![[LOOP_20]], ![[MP]], ![[UNROLL_DISABLE]], ![[DISTRIBUTE_DISABLE]], ![[WIDTH_2]], ![[SCALABLE_VEC]], ![[INTERLEAVE_4]], ![[VECTORIZE_ENABLE]]}
+
+// CHECK-DAG: ![[LOOP_21]] = distinct !{![[LOOP_21]], ![[MP]], ![[VECTORIZE_DISABLE]], ![[UNROLL_8]]}

@@ -200,7 +200,8 @@ LoopInfo::createLoopVectorizeMetadata(const LoopAttributes &Attrs,
LLVMContext &Ctx = Header->getContext();

std::optional<bool> Enabled;
if (Attrs.VectorizeEnable == LoopAttributes::Disable)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this condition has never been met.

@kasuga-fj
Copy link
Contributor Author

I think the llvm.loop.vectorize.enable metadata should be appended here unconditionally.

// vectorize.enable is set if:
// 1) loop hint vectorize.enable is set, or
// 2) it is implied when vectorize.predicate is set, or
// 3) it is implied when vectorize.width is set to a value > 1
// 4) it is implied when vectorize.scalable.enable is true
// 5) it is implied when vectorize.width is unset (0) and the user
// explicitly requested fixed-width vectorization, i.e.
// vectorize.scalable.enable is false.
if (Attrs.VectorizeEnable != LoopAttributes::Unspecified ||
(IsVectorPredicateEnabled && Attrs.VectorizeWidth != 1) ||
Attrs.VectorizeWidth > 1 ||
Attrs.VectorizeScalable == LoopAttributes::Enable ||
(Attrs.VectorizeScalable == LoopAttributes::Disable &&
Attrs.VectorizeWidth != 1)) {
bool AttrVal = Attrs.VectorizeEnable != LoopAttributes::Disable;
Args.push_back(
MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.enable"),
ConstantAsMetadata::get(ConstantInt::get(
llvm::Type::getInt1Ty(Ctx), AttrVal))}));
}

This conditional expression should be evaluated at the beginning of createVectorizeMetadata to determine the value of Enabled.

@@ -194,7 +194,7 @@ void for_test_scalable(int *List, int Length) {
}
}

// Verify for loop is performing scalable vectorization
// Verify for loop is NOT performing vectorization because the width is 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't true because VF=vscale x 1 vectorize_width(1, scalable) is a valid vectorisation factor and RISC-V targets in particular make use of this. The value of vscale at runtime could be > 1, and so you're still processing > 1 element.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, I misunderstood the condition, thanks.

So what if the width is 1 and fixed/scalable is not explicitly specified? Should vectorization run? If taking the intent of the original implementation, then it appears to not vectorize the loop.

// Disable vectorization by specifying a width of 1.
setVectorizeWidth(1);
setVectorizeScalable(LoopAttributes::Unspecified);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If taking the intent of the original implementation, then it appears to not vectorize the loop.

Yeah I think that makes sense, i.e. we should disable vectorization if (Attrs.VectorizeEnable == LoopAttributes::Disable || (Attrs.VectorizeWidth == 1 && Attrs.VectorizeScalable != LoopAttributes:: Enable)).

We should probably also update this bit in the docs to be explicit about it:

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed both code and docs. Thanks.

@@ -269,4 +287,7 @@ void for_test_scalable_1(int *List, int Length) {

// CHECK-DAG: ![[LOOP_17]] = distinct !{![[LOOP_17]], ![[MP]], ![[UNROLL_DISABLE]], ![[DISTRIBUTE_DISABLE]], ![[FIXED_VEC]], ![[INTERLEAVE_4]], ![[VECTORIZE_ENABLE]]}
// CHECK-DAG: ![[LOOP_18]] = distinct !{![[LOOP_18]], ![[MP]], ![[UNROLL_DISABLE]], ![[DISTRIBUTE_DISABLE]], ![[SCALABLE_VEC]], ![[INTERLEAVE_4]], ![[VECTORIZE_ENABLE]]}
// CHECK-DAG: ![[LOOP_19]] = distinct !{![[LOOP_19]], ![[MP]], ![[UNROLL_DISABLE]], ![[DISTRIBUTE_DISABLE]], ![[WIDTH_1]], ![[SCALABLE_VEC]], ![[INTERLEAVE_4]], ![[VECTORIZE_ENABLE]]}
// CHECK-DAG: ![[LOOP_19]] = distinct !{![[LOOP_19]], ![[MP]], ![[UNROLL_DISABLE]], ![[DISTRIBUTE_DISABLE]], ![[VECTORIZE_DISABLE]]}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I think this is broken. We should still be vectorising with VF=vscale x 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undid this, thanks.

@david-arm david-arm requested a review from lukel97 April 10, 2025 12:34
@llvmbot llvmbot added the clang:openmp OpenMP related changes to Clang label Apr 11, 2025
Copy link
Contributor

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM. Though this is a part of the code that I'm not familiar with so probably best to wait for another LGTM

@@ -37,8 +37,8 @@ void test3(int *List, int Length) {
List[i] = i * 2;
}

// Check that disabling vectorization means a vectorization width of 1, and
// also that vectorization_predicate isn't enabled.
// Check that vectorize is disabled, and also that vectorization_predicate is
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The loop doesn't have a vectorize_predicate pragma so I think this should just be // Check that vectorization is disabled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The unroll_count pragma does not work together with vectorize(disable)
4 participants