r374288 - Recommit "[Clang] Pragma vectorize_width() implies vectorize(enable)"

Hans Wennborg via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 21 09:18:13 PDT 2019


As expected, this broke the Chromium build again (but it seems only at
-Oz this time).

I'm still not a big fan of the warning: the #pragma tells the compiler
to vectorize, and then vectorization doesn't happen -- that sounds
like a compiler bug to me, and instead of pushing the problem on the
developer, I wish that could have been fixed before this landed.

We'll probably fix our build by removing the use of the pragma, but
for others who will be broken by this, I think it would be good to
have a release note about the change in behaviour (even though it was
always the intended behaviour), and especially how developers are
supposed to deal with it.

Thanks,
hans

On Thu, Oct 10, 2019 at 1:24 AM Sjoerd Meijer via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
> Author: sjoerdmeijer
> Date: Thu Oct 10 01:27:14 2019
> New Revision: 374288
>
> URL: http://llvm.org/viewvc/llvm-project?rev=374288&view=rev
> Log:
> Recommit "[Clang] Pragma vectorize_width() implies vectorize(enable)"
>
> This was further discussed at the llvm dev list:
>
> http://lists.llvm.org/pipermail/llvm-dev/2019-October/135602.html
>
> I think the brief summary of that is that this change is an improvement,
> this is the behaviour that we expect and promise in ours docs, and also
> as a result there are cases where we now emit diagnostics whereas before
> pragmas were silently ignored. Two areas where we can improve: 1) the
> diagnostic message itself, and 2) and in some cases (e.g. -Os and -Oz)
> the vectoriser is (quite understandably) not triggering.
>
> Original commit message:
>
> Specifying the vectorization width was supposed to implicitly enable
> vectorization, except that it wasn't really doing this. It was only
> setting the vectorize.width metadata, but not vectorize.enable.
>
> This should fix PR27643.
>
> Modified:
>     cfe/trunk/lib/CodeGen/CGLoopInfo.cpp
>     cfe/trunk/test/CodeGenCXX/pragma-loop-predicate.cpp
>     cfe/trunk/test/CodeGenCXX/pragma-loop.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGLoopInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGLoopInfo.cpp?rev=374288&r1=374287&r2=374288&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGLoopInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGLoopInfo.cpp Thu Oct 10 01:27:14 2019
> @@ -270,6 +270,14 @@ LoopInfo::createLoopVectorizeMetadata(co
>
>    // Setting vectorize.width
>    if (Attrs.VectorizeWidth > 0) {
> +    // This implies vectorize.enable = true, but only add it when it is not
> +    // already enabled.
> +    if (Attrs.VectorizeEnable != LoopAttributes::Enable)
> +      Args.push_back(
> +          MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.enable"),
> +                            ConstantAsMetadata::get(ConstantInt::get(
> +                                llvm::Type::getInt1Ty(Ctx), 1))}));
> +
>      Metadata *Vals[] = {
>          MDString::get(Ctx, "llvm.loop.vectorize.width"),
>          ConstantAsMetadata::get(ConstantInt::get(llvm::Type::getInt32Ty(Ctx),
>
> Modified: cfe/trunk/test/CodeGenCXX/pragma-loop-predicate.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/pragma-loop-predicate.cpp?rev=374288&r1=374287&r2=374288&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/pragma-loop-predicate.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/pragma-loop-predicate.cpp Thu Oct 10 01:27:14 2019
> @@ -58,7 +58,6 @@ void test5(int *List, int Length) {
>      List[i] = i * 2;
>  }
>
> -
>  // CHECK:      ![[LOOP0]] = distinct !{![[LOOP0]], !3}
>  // CHECK-NEXT: !3 = !{!"llvm.loop.vectorize.enable", i1 true}
>
> @@ -70,7 +69,7 @@ void test5(int *List, int Length) {
>
>  // CHECK-NEXT: ![[LOOP3]] = distinct !{![[LOOP3]], !5, !3}
>
> -// CHECK-NEXT: ![[LOOP4]] = distinct !{![[LOOP4]], !10}
> +// CHECK-NEXT: ![[LOOP4]] = distinct !{![[LOOP4]], !3, !10}
>  // CHECK-NEXT: !10 = !{!"llvm.loop.vectorize.width", i32 1}
>
> -// CHECK-NEXT: ![[LOOP5]] = distinct !{![[LOOP5]], !10}
> +// CHECK-NEXT: ![[LOOP5]] = distinct !{![[LOOP5]], !3, !10}
>
> Modified: cfe/trunk/test/CodeGenCXX/pragma-loop.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/pragma-loop.cpp?rev=374288&r1=374287&r2=374288&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/pragma-loop.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/pragma-loop.cpp Thu Oct 10 01:27:14 2019
> @@ -158,23 +158,41 @@ void template_test(double *List, int Len
>    for_template_constant_expression_test<double, 2, 4, 8>(List, Length);
>  }
>
> +void vec_width_1(int *List, int Length) {
> +// CHECK-LABEL: @{{.*}}vec_width_1{{.*}}(
> +// CHECK: br label {{.*}}, !llvm.loop ![[LOOP_15:.*]]
> +
> +  #pragma clang loop vectorize(enable) vectorize_width(1)
> +  for (int i = 0; i < Length; i++)
> +    List[i] = i * 2;
> +}
> +
> +void width_1(int *List, int Length) {
> +// CHECK-LABEL: @{{.*}}width_1{{.*}}(
> +// CHECK: br label {{.*}}, !llvm.loop ![[LOOP_16:.*]]
> +
> +  #pragma clang loop vectorize_width(1)
> +  for (int i = 0; i < Length; i++)
> +    List[i] = i * 2;
> +}
> +
>  // CHECK: ![[LOOP_1]] = distinct !{![[LOOP_1]], ![[UNROLL_FULL:.*]]}
>  // CHECK: ![[UNROLL_FULL]] = !{!"llvm.loop.unroll.full"}
>
> -// CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2]], ![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_8:.*]], ![[INTERLEAVE_4:.*]]}
> +// CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2]], ![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[VECTORIZE_ENABLE:.*]], ![[WIDTH_8:.*]], ![[INTERLEAVE_4:.*]]}
>  // CHECK: ![[UNROLL_DISABLE]] = !{!"llvm.loop.unroll.disable"}
>  // CHECK: ![[DISTRIBUTE_DISABLE]] = !{!"llvm.loop.distribute.enable", i1 false}
> +// CHECK: ![[VECTORIZE_ENABLE]] = !{!"llvm.loop.vectorize.enable", i1 true}
>  // CHECK: ![[WIDTH_8]] = !{!"llvm.loop.vectorize.width", i32 8}
>  // CHECK: ![[INTERLEAVE_4]] = !{!"llvm.loop.interleave.count", i32 4}
>
> -// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[INTERLEAVE_4:.*]], ![[INTENABLE_1:.*]], ![[FOLLOWUP_VECTOR_3:.*]]}
> -// CHECK: ![[INTENABLE_1]] = !{!"llvm.loop.vectorize.enable", i1 true}
> +// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[INTERLEAVE_4:.*]], ![[VECTORIZE_ENABLE]], ![[FOLLOWUP_VECTOR_3:.*]]}
>  // CHECK: ![[FOLLOWUP_VECTOR_3]] = !{!"llvm.loop.vectorize.followup_all", ![[AFTER_VECTOR_3:.*]]}
>  // 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: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[VECTORIZE_ENABLE]], ![[WIDTH_2:.*]], ![[INTERLEAVE_2:.*]]}
>  // CHECK: ![[WIDTH_2]] = !{!"llvm.loop.vectorize.width", i32 2}
>  // CHECK: ![[INTERLEAVE_2]] = !{!"llvm.loop.interleave.count", i32 2}
>
> @@ -185,7 +203,7 @@ void template_test(double *List, int Len
>  // 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: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[VECTORIZE_ENABLE]], ![[WIDTH_5:.*]]}
>  // CHECK: ![[WIDTH_5]] = !{!"llvm.loop.vectorize.width", i32 5}
>
>  // CHECK: ![[LOOP_8]] = distinct !{![[LOOP_8]], ![[WIDTH_5:.*]]}
> @@ -213,5 +231,9 @@ void template_test(double *List, int Len
>  // CHECK: ![[AFTER_VECTOR_13]] = distinct !{![[AFTER_VECTOR_13]], ![[ISVECTORIZED:.*]], ![[UNROLL_32:.*]]}
>  // CHECK: ![[UNROLL_32]] = !{!"llvm.loop.unroll.count", i32 32}
>
> -// CHECK: ![[LOOP_14]] = distinct !{![[LOOP_14]], ![[WIDTH_10:.*]]}
> +// CHECK: ![[LOOP_14]] = distinct !{![[LOOP_14]], ![[VECTORIZE_ENABLE]], ![[WIDTH_10:.*]]}
>  // CHECK: ![[WIDTH_10]] = !{!"llvm.loop.vectorize.width", i32 10}
> +
> +// CHECK:      ![[LOOP_15]] = distinct !{![[LOOP_15]], ![[WIDTH_1]], ![[VECTORIZE_ENABLE]]}
> +
> +// CHECK-NEXT: ![[LOOP_16]] = distinct !{![[LOOP_16]], ![[VECTORIZE_ENABLE]], ![[WIDTH_1]]}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


More information about the cfe-commits mailing list