public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] i386: Fix up ix86_convert_const_wide_int_to_broadcast [PR108599]
@ 2023-01-31  8:01 Jakub Jelinek
  2023-01-31  9:05 ` Uros Bizjak
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2023-01-31  8:01 UTC (permalink / raw)
  To: Hongtao Liu, Uros Bizjak; +Cc: gcc-patches

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.

--- 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


^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-01-31  9:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-31  8:01 [PATCH] i386: Fix up ix86_convert_const_wide_int_to_broadcast [PR108599] Jakub Jelinek
2023-01-31  9:05 ` Uros Bizjak

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).