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

Mon Oct 21 14:00:38 PDT 2019

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.

On Mon, Oct 21, 2019 at 9:18 AM Hans Wennborg via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> 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
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191021/f92aefda/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4849 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191021/f92aefda/attachment-0001.bin>