[clang] Loop pragma vectorize(disable)

[SjoerdMeijer]

Summary

This changes the behaviour of vectorize(disable). I.e., disabling vectorization
now means disabling vectorization, and not setting the vectorization width to 1.

This is a follow up of the discussion on the behaviour of different loop
pragmas, and that setting transformation options should imply enabling the
transformation, see also
http://lists.llvm.org/pipermail/cfe-dev/2019-August/063054.html.

Not only is this change in the behaviour of vectorize(disable) probably more sensible,
it also helps in implementing options implying transformations.

[Meinersbur]

Therefore, vectorize(disable) would also disable interleaving?

[SjoerdMeijer]

Therefore, vectorize(disable) would also disable interleaving?

I don't have strong opinions on this. I think there's something to say for both options that we have (i.e. vectorize(disable) disables interleaving or enables it).
But I think it was @fhahn who mentioned that it would be possible to disable vectorisation, but still do interleaving. Perhaps bit of a edge use use, but again, it's possible currently. That would mean we don't need to disable interleaving here I think.

[SjoerdMeijer]

friendly ping.

Shall we get this in, so that I can commit this and D66290? Then, we can perhaps continue the interleave discussion separately?
I will then return to the doc patch first in D66199, so that we can start wrapping up the loop pragma business, and hopefully it is a lot more consistent!

[fhahn]

IMO it is fine to say pragma vectorize(disable) disables the vectorizer completely, including interleaving. @Meinersbur, what do you think? I think it would be good to make that clear in the commit message.

[hsaito]

There are two ways to think.

1. vectorize(disable) as in disable the LoopVectorize pass itself.
2. vectorize(disable) as in disabling the loop vectorization transformation

It all depends on to whom we are providing these pragmas.

If we are providing pragmas for programmers, they don't care LoopVectorize or something else. They care about vectorization of loop. So, 2) makes more sense.

If we are providing pragmas for compiler writers, we should be making this more like

#pragma clang loop LoopVectorize(disable)

so that what is controlling what crisply known to everyone.

For anything that is programmer visible, I think it's time for LLVM vectorizer to focus on how the programmers use rather than how vectorizer writers think about things.

[SjoerdMeijer]

Hi Hideki,

I think your comments are spot on:

It all depends on to whom we are providing these pragmas.

Pragma's are user-facing "options" to override or force compiler decision making. I don't think there's another way to look at it, but please correct me if I'm wrong.

That's exactly the reason why I think vectorize(disable) should disable vectorisation for that loop. I just don't see what else a user would expect.

[SjoerdMeijer]

And thanks Florian for getting this discussion going again! Will definitely make this clear(er) in this description and commit message once we agree on it.

[hsaito]

In D66796#1663868, @SjoerdMeijer wrote:

That's exactly the reason why I think vectorize(disable) should disable vectorisation for that loop. I just don't see what else a user would expect.

I agree with you.

Now on the practical side. If whatever we decide here changes how the pragma behaves from today, we need to have a wider discussion and agreement at llvm-dev.

[SjoerdMeijer]

Now on the practical side. If whatever we decide here changes how the pragma behaves from today, we need to have a wider discussion and agreement at llvm-dev.

Yep, forgot about that, thanks for the suggestion. I've just posted this message to the list:

http://lists.llvm.org/pipermail/llvm-dev/2019-September/135016.html

[SjoerdMeijer]

Every day is a school day, and Hal taught me something ;-)

As I wrote in the thread on the llvm dev list, with Hal's explanations, I don't think I have a case for this anymore, and so will abandon it (please let me know if you disagree).

Many thanks for reviewing and the discussions!

clang/lib/CodeGen/CGLoopInfo.cpp

if (OpenCLHint) {
 
       Option = LH->getOption();
       State = LH->getState();
     }
     switch (State) {
     case LoopHintAttr::Disable:
       switch (Option) {
       case LoopHintAttr::Vectorize:
-        // Disable vectorization by specifying a width of 1.
-        setVectorizeWidth(1);
+        setVectorizeEnable(false);
         break;
       case LoopHintAttr::Interleave:
         // Disable interleaving by speciyfing a count of 1.
         setInterleaveCount(1);
         break;
       case LoopHintAttr::Unroll:
         setUnrollState(LoopAttributes::Disable);
         break;

clang/test/CodeGenCXX/pragma-loop-predicate.cpp

 // CHECK-NEXT: !5 = !{!"llvm.loop.vectorize.predicate.enable", i1 true}
 
 // CHECK-NEXT: ![[LOOP2]] = distinct !{![[LOOP2]], !7, !3}
 // CHECK-NEXT: !7 = !{!"llvm.loop.vectorize.predicate.enable", i1 false}
 
 // CHECK-NEXT: ![[LOOP3]] = distinct !{![[LOOP3]], !5, !3}
 
 // CHECK-NEXT: ![[LOOP4]] = distinct !{![[LOOP4]], !10}
-// CHECK-NEXT: !10 = !{!"llvm.loop.vectorize.width", i32 1}
+// CHECK-NEXT: !10 = !{!"llvm.loop.vectorize.enable", i1 false}
 
 // CHECK-NEXT: ![[LOOP5]] = distinct !{![[LOOP5]], !10}

clang/test/CodeGenCXX/pragma-loop-safety.cpp

 
 // CHECK: ![[ACCESS_GROUP_2]] = distinct !{}
 // CHECK: ![[LOOP1_HINTS]] = distinct !{![[LOOP1_HINTS]], ![[PARALLEL_ACCESSES_7:[0-9]+]], ![[UNROLL_DISABLE:[0-9]+]], ![[INTERLEAVE_1:[0-9]+]], ![[INTENABLE_1:[0-9]+]]}
 // CHECK: ![[PARALLEL_ACCESSES_7]] = !{!"llvm.loop.parallel_accesses", ![[ACCESS_GROUP_2]]}
 // CHECK: ![[UNROLL_DISABLE]] = !{!"llvm.loop.unroll.disable"}
 // 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]], ![[PARALLEL_ACCESSES_11:[0-9]+]], ![[UNROLL_DISABLE]], ![[WIDTH_1:[0-9]+]], ![[INTENABLE_1]]}
+// CHECK: ![[LOOP2_HINTS]] = distinct !{![[LOOP2_HINTS]], ![[PARALLEL_ACCESSES_11:[0-9]+]], ![[UNROLL_DISABLE]], ![[VECTORIZE_DISABLE:[0-9]+]]}
 // CHECK: ![[PARALLEL_ACCESSES_11]] = !{!"llvm.loop.parallel_accesses", ![[ACCESS_GROUP_8]]}
-// CHECK: ![[WIDTH_1]] = !{!"llvm.loop.vectorize.width", i32 1}
+// CHECK: ![[VECTORIZE_DISABLE]] = !{!"llvm.loop.vectorize.enable", i1 false}

clang/test/CodeGenCXX/pragma-loop.cpp

 // CHECK: ![[AFTER_VECTOR_3]] = distinct !{![[AFTER_VECTOR_3]], ![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}
 // CHECK: ![[ISVECTORIZED]] = !{!"llvm.loop.isvectorized"}
 // CHECK: ![[UNROLL_8]] = !{!"llvm.loop.unroll.count", i32 8}
 
 // CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[WIDTH_2:.*]], ![[INTERLEAVE_2:.*]]}
 // CHECK: ![[WIDTH_2]] = !{!"llvm.loop.vectorize.width", i32 2}
 // CHECK: ![[INTERLEAVE_2]] = !{!"llvm.loop.interleave.count", i32 2}
 
-// CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_1:.*]]}
-// CHECK: ![[WIDTH_1]] = !{!"llvm.loop.vectorize.width", i32 1}
+// CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[VECTORIZE_DISABLE:.*]]}
+// CHECK: ![[VECTORIZE_DISABLE]] = !{!"llvm.loop.vectorize.enable", i1 false}
 
 // CHECK: ![[LOOP_6]] = distinct !{![[LOOP_6]], ![[WIDTH_2:.*]], ![[INTERLEAVE_2:.*]], ![[FOLLOWUP_VECTOR_6:.*]]}
 // CHECK: ![[FOLLOWUP_VECTOR_6]] = !{!"llvm.loop.vectorize.followup_all", ![[AFTER_VECTOR_6:.*]]}
 // CHECK: ![[AFTER_VECTOR_6]] = distinct !{![[AFTER_VECTOR_6]], ![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}
 
 // CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[WIDTH_5:.*]]}
 // CHECK: ![[WIDTH_5]] = !{!"llvm.loop.vectorize.width", i32 5}