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

[Jordan Rupprecht via cfe-commits]

On Mon, Oct 21, 2019 at 5:12 PM Michael Kruse <llvm-commits at meinersbur.de>
wrote:

> Am Mo., 21. Okt. 2019 um 16:01 Uhr schrieb Jordan Rupprecht via
> cfe-commits <cfe-commits at lists.llvm.org>:
> > There's also a curious failure caused by this patch (confirmed passing
> @r374287, failing @r374288):
>
> It's a warning, not a failure.
>
True. We build with -Werror.

At any rate, it sounds like this is not a codegen bug at all, but just an
over-eager warning? If so, I'll double check the IR of the internal repro
isn't affected by this patch when -Wno-pass-failed, and use that as a
temporary measure until the eager warning can be removed.


>
> > $ 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.
>
> https://reviews.llvm.org/D66290
>
> generates the following metadata for #pragma clang loop vectorize(disable):
>
> !3 = distinct !{!3, !4, !5}
> !4 = !{!"llvm.loop.vectorize.enable", i1 true}
> !5 = !{!"llvm.loop.vectorize.width", i32 1}
>
> which is interpreted as "enable LoopVectorize, but only the interleave
> part" by LoopVectorize and WarnMissedTransformations.
>
> The tests in D66290 missed that because the regex may matche multiple
> MDNodes:
>
> // CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]],
> ![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_1:.*]]}
>
> The additional "llvm.loop.vectorize.enable" is generated by:
>
>   if (Attrs.VectorizeWidth > 0) {
>     // This implies vectorize.enable = true, but only add it when it is not
>     // already enabled.
>     if (Attrs.VectorizeEnable != LoopAttributes::Enable)
>
> Attr.VectorizeWidth is set by the vectorize(disable) LoopHint:
>
>     case LoopHintAttr::Disable:
>       switch (Option) {
>       case LoopHintAttr::Vectorize:
>         // Disable vectorization by specifying a width of 1.
>         setVectorizeWidth(1);
>
Might setVectorizeWidth(0) (or -1, or llvm::Optional<int>) be a better
signal of "vectorization is disabled" than 1 (which could later be
interpreted as "we failed to vectorize when requested")?

(No familiarity with this code, not sure if this suggestion makes sense)


>
>
>
> Michael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191021/5209379b/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/5209379b/attachment-0001.bin>