<div dir="ltr">There's also a curious failure caused by this patch (confirmed passing @r374287, failing @r374288):<div><br></div><div>$ cat /tmp/vectorize.cc<br>void a() {<br>#pragma clang loop vectorize(disable)<br> for (;;)<br> ;<br>}<br></div><div><br></div><div>$ clang++ -Werror -O3 -c /tmp/vectorize.cc<br></div><div>/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]<br>void a() {<br></div><div><br></div><div>I don't understand this warning -- there is no requested transformation; in fact, we've explicitly specified that vectorization *should* be disabled.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 21, 2019 at 9:18 AM Hans Wennborg via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">As expected, this broke the Chromium build again (but it seems only at<br>
-Oz this time).<br>
<br>
I'm still not a big fan of the warning: the #pragma tells the compiler<br>
to vectorize, and then vectorization doesn't happen -- that sounds<br>
like a compiler bug to me, and instead of pushing the problem on the<br>
developer, I wish that could have been fixed before this landed.<br>
<br>
We'll probably fix our build by removing the use of the pragma, but<br>
for others who will be broken by this, I think it would be good to<br>
have a release note about the change in behaviour (even though it was<br>
always the intended behaviour), and especially how developers are<br>
supposed to deal with it.<br>
<br>
Thanks,<br>
hans<br>
<br>
On Thu, Oct 10, 2019 at 1:24 AM Sjoerd Meijer via cfe-commits<br>
<<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
><br>
> Author: sjoerdmeijer<br>
> Date: Thu Oct 10 01:27:14 2019<br>
> New Revision: 374288<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=374288&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=374288&view=rev</a><br>
> Log:<br>
> Recommit "[Clang] Pragma vectorize_width() implies vectorize(enable)"<br>
><br>
> This was further discussed at the llvm dev list:<br>
><br>
> <a href="http://lists.llvm.org/pipermail/llvm-dev/2019-October/135602.html" rel="noreferrer" target="_blank">http://lists.llvm.org/pipermail/llvm-dev/2019-October/135602.html</a><br>
><br>
> I think the brief summary of that is that this change is an improvement,<br>
> this is the behaviour that we expect and promise in ours docs, and also<br>
> as a result there are cases where we now emit diagnostics whereas before<br>
> pragmas were silently ignored. Two areas where we can improve: 1) the<br>
> diagnostic message itself, and 2) and in some cases (e.g. -Os and -Oz)<br>
> the vectoriser is (quite understandably) not triggering.<br>
><br>
> Original commit message:<br>
><br>
> Specifying the vectorization width was supposed to implicitly enable<br>
> vectorization, except that it wasn't really doing this. It was only<br>
> setting the vectorize.width metadata, but not vectorize.enable.<br>
><br>
> This should fix PR27643.<br>
><br>
> Modified:<br>
> cfe/trunk/lib/CodeGen/CGLoopInfo.cpp<br>
> cfe/trunk/test/CodeGenCXX/pragma-loop-predicate.cpp<br>
> cfe/trunk/test/CodeGenCXX/pragma-loop.cpp<br>
><br>
> Modified: cfe/trunk/lib/CodeGen/CGLoopInfo.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGLoopInfo.cpp?rev=374288&r1=374287&r2=374288&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGLoopInfo.cpp?rev=374288&r1=374287&r2=374288&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/CodeGen/CGLoopInfo.cpp (original)<br>
> +++ cfe/trunk/lib/CodeGen/CGLoopInfo.cpp Thu Oct 10 01:27:14 2019<br>
> @@ -270,6 +270,14 @@ LoopInfo::createLoopVectorizeMetadata(co<br>
><br>
> // Setting vectorize.width<br>
> if (Attrs.VectorizeWidth > 0) {<br>
> + // This implies vectorize.enable = true, but only add it when it is not<br>
> + // already enabled.<br>
> + if (Attrs.VectorizeEnable != LoopAttributes::Enable)<br>
> + Args.push_back(<br>
> + MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.enable"),<br>
> + ConstantAsMetadata::get(ConstantInt::get(<br>
> + llvm::Type::getInt1Ty(Ctx), 1))}));<br>
> +<br>
> Metadata *Vals[] = {<br>
> MDString::get(Ctx, "llvm.loop.vectorize.width"),<br>
> ConstantAsMetadata::get(ConstantInt::get(llvm::Type::getInt32Ty(Ctx),<br>
><br>
> Modified: cfe/trunk/test/CodeGenCXX/pragma-loop-predicate.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/pragma-loop-predicate.cpp?rev=374288&r1=374287&r2=374288&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/pragma-loop-predicate.cpp?rev=374288&r1=374287&r2=374288&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/CodeGenCXX/pragma-loop-predicate.cpp (original)<br>
> +++ cfe/trunk/test/CodeGenCXX/pragma-loop-predicate.cpp Thu Oct 10 01:27:14 2019<br>
> @@ -58,7 +58,6 @@ void test5(int *List, int Length) {<br>
> List[i] = i * 2;<br>
> }<br>
><br>
> -<br>
> // CHECK: ![[LOOP0]] = distinct !{![[LOOP0]], !3}<br>
> // CHECK-NEXT: !3 = !{!"llvm.loop.vectorize.enable", i1 true}<br>
><br>
> @@ -70,7 +69,7 @@ void test5(int *List, int Length) {<br>
><br>
> // CHECK-NEXT: ![[LOOP3]] = distinct !{![[LOOP3]], !5, !3}<br>
><br>
> -// CHECK-NEXT: ![[LOOP4]] = distinct !{![[LOOP4]], !10}<br>
> +// CHECK-NEXT: ![[LOOP4]] = distinct !{![[LOOP4]], !3, !10}<br>
> // CHECK-NEXT: !10 = !{!"llvm.loop.vectorize.width", i32 1}<br>
><br>
> -// CHECK-NEXT: ![[LOOP5]] = distinct !{![[LOOP5]], !10}<br>
> +// CHECK-NEXT: ![[LOOP5]] = distinct !{![[LOOP5]], !3, !10}<br>
><br>
> Modified: cfe/trunk/test/CodeGenCXX/pragma-loop.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/pragma-loop.cpp?rev=374288&r1=374287&r2=374288&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/pragma-loop.cpp?rev=374288&r1=374287&r2=374288&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/CodeGenCXX/pragma-loop.cpp (original)<br>
> +++ cfe/trunk/test/CodeGenCXX/pragma-loop.cpp Thu Oct 10 01:27:14 2019<br>
> @@ -158,23 +158,41 @@ void template_test(double *List, int Len<br>
> for_template_constant_expression_test<double, 2, 4, 8>(List, Length);<br>
> }<br>
><br>
> +void vec_width_1(int *List, int Length) {<br>
> +// CHECK-LABEL: @{{.*}}vec_width_1{{.*}}(<br>
> +// CHECK: br label {{.*}}, !llvm.loop ![[LOOP_15:.*]]<br>
> +<br>
> + #pragma clang loop vectorize(enable) vectorize_width(1)<br>
> + for (int i = 0; i < Length; i++)<br>
> + List[i] = i * 2;<br>
> +}<br>
> +<br>
> +void width_1(int *List, int Length) {<br>
> +// CHECK-LABEL: @{{.*}}width_1{{.*}}(<br>
> +// CHECK: br label {{.*}}, !llvm.loop ![[LOOP_16:.*]]<br>
> +<br>
> + #pragma clang loop vectorize_width(1)<br>
> + for (int i = 0; i < Length; i++)<br>
> + List[i] = i * 2;<br>
> +}<br>
> +<br>
> // CHECK: ![[LOOP_1]] = distinct !{![[LOOP_1]], ![[UNROLL_FULL:.*]]}<br>
> // CHECK: ![[UNROLL_FULL]] = !{!"llvm.loop.unroll.full"}<br>
><br>
> -// CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2]], ![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_8:.*]], ![[INTERLEAVE_4:.*]]}<br>
> +// CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2]], ![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[VECTORIZE_ENABLE:.*]], ![[WIDTH_8:.*]], ![[INTERLEAVE_4:.*]]}<br>
> // CHECK: ![[UNROLL_DISABLE]] = !{!"llvm.loop.unroll.disable"}<br>
> // CHECK: ![[DISTRIBUTE_DISABLE]] = !{!"llvm.loop.distribute.enable", i1 false}<br>
> +// CHECK: ![[VECTORIZE_ENABLE]] = !{!"llvm.loop.vectorize.enable", i1 true}<br>
> // CHECK: ![[WIDTH_8]] = !{!"llvm.loop.vectorize.width", i32 8}<br>
> // CHECK: ![[INTERLEAVE_4]] = !{!"llvm.loop.interleave.count", i32 4}<br>
><br>
> -// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[INTERLEAVE_4:.*]], ![[INTENABLE_1:.*]], ![[FOLLOWUP_VECTOR_3:.*]]}<br>
> -// CHECK: ![[INTENABLE_1]] = !{!"llvm.loop.vectorize.enable", i1 true}<br>
> +// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[INTERLEAVE_4:.*]], ![[VECTORIZE_ENABLE]], ![[FOLLOWUP_VECTOR_3:.*]]}<br>
> // CHECK: ![[FOLLOWUP_VECTOR_3]] = !{!"llvm.loop.vectorize.followup_all", ![[AFTER_VECTOR_3:.*]]}<br>
> // CHECK: ![[AFTER_VECTOR_3]] = distinct !{![[AFTER_VECTOR_3]], ![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}<br>
> // CHECK: ![[ISVECTORIZED]] = !{!"llvm.loop.isvectorized"}<br>
> // CHECK: ![[UNROLL_8]] = !{!"llvm.loop.unroll.count", i32 8}<br>
><br>
> -// CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[WIDTH_2:.*]], ![[INTERLEAVE_2:.*]]}<br>
> +// CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[VECTORIZE_ENABLE]], ![[WIDTH_2:.*]], ![[INTERLEAVE_2:.*]]}<br>
> // CHECK: ![[WIDTH_2]] = !{!"llvm.loop.vectorize.width", i32 2}<br>
> // CHECK: ![[INTERLEAVE_2]] = !{!"llvm.loop.interleave.count", i32 2}<br>
><br>
> @@ -185,7 +203,7 @@ void template_test(double *List, int Len<br>
> // CHECK: ![[FOLLOWUP_VECTOR_6]] = !{!"llvm.loop.vectorize.followup_all", ![[AFTER_VECTOR_6:.*]]}<br>
> // CHECK: ![[AFTER_VECTOR_6]] = distinct !{![[AFTER_VECTOR_6]], ![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}<br>
><br>
> -// CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[WIDTH_5:.*]]}<br>
> +// CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[VECTORIZE_ENABLE]], ![[WIDTH_5:.*]]}<br>
> // CHECK: ![[WIDTH_5]] = !{!"llvm.loop.vectorize.width", i32 5}<br>
><br>
> // CHECK: ![[LOOP_8]] = distinct !{![[LOOP_8]], ![[WIDTH_5:.*]]}<br>
> @@ -213,5 +231,9 @@ void template_test(double *List, int Len<br>
> // CHECK: ![[AFTER_VECTOR_13]] = distinct !{![[AFTER_VECTOR_13]], ![[ISVECTORIZED:.*]], ![[UNROLL_32:.*]]}<br>
> // CHECK: ![[UNROLL_32]] = !{!"llvm.loop.unroll.count", i32 32}<br>
><br>
> -// CHECK: ![[LOOP_14]] = distinct !{![[LOOP_14]], ![[WIDTH_10:.*]]}<br>
> +// CHECK: ![[LOOP_14]] = distinct !{![[LOOP_14]], ![[VECTORIZE_ENABLE]], ![[WIDTH_10:.*]]}<br>
> // CHECK: ![[WIDTH_10]] = !{!"llvm.loop.vectorize.width", i32 10}<br>
> +<br>
> +// CHECK: ![[LOOP_15]] = distinct !{![[LOOP_15]], ![[WIDTH_1]], ![[VECTORIZE_ENABLE]]}<br>
> +<br>
> +// CHECK-NEXT: ![[LOOP_16]] = distinct !{![[LOOP_16]], ![[VECTORIZE_ENABLE]], ![[WIDTH_1]]}<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>