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