public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc r13-5529] i386: Fix up ix86_convert_const_wide_int_to_broadcast [PR108599]
Date: Tue, 31 Jan 2023 09:12:52 +0000 (GMT)	[thread overview]
Message-ID: <20230131091252.A77873858D28@sourceware.org> (raw)

https://gcc.gnu.org/g:963315a922e228c4f6853826666151fc540f111a

commit r13-5529-g963315a922e228c4f6853826666151fc540f111a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Jan 31 10:12:19 2023 +0100

    i386: Fix up ix86_convert_const_wide_int_to_broadcast [PR108599]
    
    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.
    
    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.

Diff:
---
 gcc/config/i386/i386-expand.cc                |  4 +++-
 gcc/testsuite/gcc.target/i386/avx2-pr108599.c | 32 +++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc
index e2e2d28bb47..e59c7b0150f 100644
--- a/gcc/config/i386/i386-expand.cc
+++ b/gcc/config/i386/i386-expand.cc
@@ -291,7 +291,9 @@ ix86_convert_const_wide_int_to_broadcast (machine_mode mode, rtx op)
      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);
diff --git a/gcc/testsuite/gcc.target/i386/avx2-pr108599.c b/gcc/testsuite/gcc.target/i386/avx2-pr108599.c
new file mode 100644
index 00000000000..d5ddab7609f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/avx2-pr108599.c
@@ -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);
+  }
+}

                 reply	other threads:[~2023-01-31  9:12 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20230131091252.A77873858D28@sourceware.org \
    --to=jakub@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /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).