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

Finkel, Hal J. via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 21 14:18:16 PDT 2019


On 10/21/19 4:00 PM, Jordan Rupprecht via cfe-commits wrote:
There's also a curious failure caused by this patch (confirmed passing @r374287, failing @r374288):

$ cat /tmp/vectorize.cc
void a() {
#pragma clang loop vectorize(disable)
  for (;;)
    ;
}

$ clang++ -Werror -O3 -c /tmp/vectorize.cc
/tmp/vectorize.cc:1:6: error: loop not interleaved: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering [-Werror,-Wpass-failed=transform-warning]
void a() {

I don't understand this warning -- there is no requested transformation; in fact, we've explicitly specified that vectorization *should* be disabled.


Oh. That's funny.

So the problem is that the warning is just responding to metadata that the vectorizer didn't remove, it doesn't understand anything about what the metadata is requesting. WarnMissedTransforms.cpp should probably be updated to avoid this issue.


On Mon, Oct 21, 2019 at 9:18 AM Hans Wennborg via cfe-commits <cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>> wrote:
As expected, this broke the Chromium build again (but it seems only at
-Oz this time).


To be clear, is the desired behavior that the vectorization is performed even at -Oz? I suspect that it is, but I just want to confirm.

 -Hal


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<mailto: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<mailto:cfe-commits at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits



_______________________________________________
cfe-commits mailing list
cfe-commits at lists.llvm.org<mailto:cfe-commits at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191021/b88c01f2/attachment-0001.html>


More information about the cfe-commits mailing list