From: Uros Bizjak <ubizjak@gmail.com>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Hongtao Liu <hongtao.liu@intel.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] i386: Fix up ix86_convert_const_wide_int_to_broadcast [PR108599]
Date: Tue, 31 Jan 2023 10:05:39 +0100 [thread overview]
Message-ID: <CAFULd4ZAn5YtDy2-9FFJ1XfwJ7GZbyV5AaDQxiFO2te7hkSCxQ@mail.gmail.com> (raw)
In-Reply-To: <Y9jK8Bk6l6sxbuvC@tucnak>
On Tue, Jan 31, 2023 at 9:02 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> The following testcase is miscompiled. The problem is that during
> RTL DSE we see a V4DI register is being loaded { 16, 16, 0, 0 }
> value and DSE mostly works in terms of scalar modes, so it calls
> movoi to set an OImode REG to (const_wide_int 0x100000000000000010)
> and ix86_convert_const_wide_int_to_broadcast thinks it can compute
> that value by broadcasting DImode 0x10. While it is true that
> for TImode result the broadcast could be used, for OImode/XImode
> it can't be, because all but the lowest 2 HOST_WIDE_INTs aren't
> present (so are 0 or -1 depending on sign), not 0x10 in this case.
> The function checks if the least significant HOST_WIDE_INT elt
> of the CONST_WIDE_INT is broadcastable from QI/HI/SI/DImode and then
> /* Check if OP can be broadcasted from VAL. */
> for (int i = 1; i < CONST_WIDE_INT_NUNITS (op); i++)
> if (val != CONST_WIDE_INT_ELT (op, i))
> return nullptr;
> That is needed of course, but nothing checks that
> CONST_WIDE_INT_NUNITS (op) isn't too small for the mode in question.
> I think if op would be 0 or -1, it ought to be never CONST_WIDE_INT,
> but CONST_INT and so we can just punt whenever the number of
> CONST_WIDE_INT elts is not the expected one.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2023-01-31 Jakub Jelinek <jakub@redhat.com>
>
> PR target/108599
> * config/i386/i386-expand.cc
> (ix86_convert_const_wide_int_to_broadcast): Return nullptr if
> CONST_WIDE_INT_NUNITS (op) times HOST_BITS_PER_WIDE_INT isn't
> equal to bitsize of mode.
>
> * gcc.target/i386/avx2-pr108599.c: New test.
OK.
Thanks,
Uros.
>
> --- gcc/config/i386/i386-expand.cc.jj 2023-01-19 23:22:05.306066616 +0100
> +++ gcc/config/i386/i386-expand.cc 2023-01-30 15:33:43.418598714 +0100
> @@ -291,7 +291,9 @@ ix86_convert_const_wide_int_to_broadcast
> broadcast only if vector broadcast is available. */
> if (!TARGET_AVX
> || !CONST_WIDE_INT_P (op)
> - || standard_sse_constant_p (op, mode))
> + || standard_sse_constant_p (op, mode)
> + || (CONST_WIDE_INT_NUNITS (op) * HOST_BITS_PER_WIDE_INT
> + != GET_MODE_BITSIZE (mode)))
> return nullptr;
>
> HOST_WIDE_INT val = CONST_WIDE_INT_ELT (op, 0);
> --- gcc/testsuite/gcc.target/i386/avx2-pr108599.c.jj 2023-01-30 16:04:31.984429702 +0100
> +++ gcc/testsuite/gcc.target/i386/avx2-pr108599.c 2023-01-30 16:04:24.459540223 +0100
> @@ -0,0 +1,32 @@
> +/* PR target/108599 */
> +/* { dg-do run { target avx2 } } */
> +/* { dg-options "-O2 -mavx2 -mtune=skylake-avx512" } */
> +
> +#include "avx2-check.h"
> +
> +struct S { unsigned long long a, b, c, d; };
> +
> +__attribute__((noipa)) void
> +foo (unsigned long long x, unsigned long long y,
> + unsigned long long z, unsigned long long w, const struct S s)
> +{
> + if (s.a != x || s.b != y || s.c != z || s.d != w)
> + abort ();
> +}
> +
> +typedef unsigned long long V __attribute__((may_alias, vector_size (4 * sizeof (unsigned long long))));
> +
> +static void
> +avx2_test (void)
> +{
> + {
> + struct S s;
> + *(V *)&s = (V) { 16, 0, 0, 0 };
> + foo (16, 0, 0, 0, s);
> + }
> + {
> + struct S s;
> + *(V *)&s = (V) { 16, 16, 0, 0 };
> + foo (16, 16, 0, 0, s);
> + }
> +}
>
> Jakub
>
prev parent reply other threads:[~2023-01-31 9:05 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-31 8:01 Jakub Jelinek
2023-01-31 9:05 ` Uros Bizjak [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFULd4ZAn5YtDy2-9FFJ1XfwJ7GZbyV5AaDQxiFO2te7hkSCxQ@mail.gmail.com \
--to=ubizjak@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=hongtao.liu@intel.com \
--cc=jakub@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).