diff mbox

Re-fix PR target/47755 on powerpc VSX

Message ID 20110308235610.GA19622@hungry-tiger.westford.ibm.com
State New
Headers show

Commit Message

Michael Meissner March 8, 2011, 11:56 p.m. UTC
PR 47755 fixed some bugs on VSX with V2DI constants, but the patch itself had
problems.  In particular, the easy_altivec_constant support assumed the largest
int size was SImode.  This would cause the compiler to generate VSPLTI{W,S,B}
to load up a constant instead of loading it up from memory.

This patch only allows (vector long long) { 0, 0 } and
(vector long long) { -1, -1 } as easy V2DI constants.  There are some other
constants that could be generated using the Altivec instructions in the future.

I bootstrapped this patch and had no regressions with make check.  Is it ok to
install?

[gcc]
2011-03-08  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/47755
	* config/rs6000/rs6000.c (easy_altivec_constant): Correctly handle
	V2DI/V2DF constants.  Only all 0's or all 1's are easy.
	(output_vec_const_move): Ditto.

[gcc/testsuite]
2011-03-08  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/47755
	* gcc.target/powerpc/pr47755-2.c: New file.

Comments

David Edelsohn March 9, 2011, 12:12 a.m. UTC | #1
On Tue, Mar 8, 2011 at 6:56 PM, Michael Meissner
<meissner@linux.vnet.ibm.com> wrote:
> PR 47755 fixed some bugs on VSX with V2DI constants, but the patch itself had
> problems.  In particular, the easy_altivec_constant support assumed the largest
> int size was SImode.  This would cause the compiler to generate VSPLTI{W,S,B}
> to load up a constant instead of loading it up from memory.
>
> This patch only allows (vector long long) { 0, 0 } and
> (vector long long) { -1, -1 } as easy V2DI constants.  There are some other
> constants that could be generated using the Altivec instructions in the future.
>
> I bootstrapped this patch and had no regressions with make check.  Is it ok to
> install?
>
> [gcc]
> 2011-03-08  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>        PR target/47755
>        * config/rs6000/rs6000.c (easy_altivec_constant): Correctly handle
>        V2DI/V2DF constants.  Only all 0's or all 1's are easy.
>        (output_vec_const_move): Ditto.
>
> [gcc/testsuite]
> 2011-03-08  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>        PR target/47755
>        * gcc.target/powerpc/pr47755-2.c: New file.

Should the CONST_INT test be wrapped in #ifdef HOST_BITS_PER_WIDE_INT?

Okay.

Thanks, David
Michael Meissner March 9, 2011, 12:18 a.m. UTC | #2
On Tue, Mar 08, 2011 at 07:12:27PM -0500, David Edelsohn wrote:
> On Tue, Mar 8, 2011 at 6:56 PM, Michael Meissner
> <meissner@linux.vnet.ibm.com> wrote:
> > PR 47755 fixed some bugs on VSX with V2DI constants, but the patch itself had
> > problems.  In particular, the easy_altivec_constant support assumed the largest
> > int size was SImode.  This would cause the compiler to generate VSPLTI{W,S,B}
> > to load up a constant instead of loading it up from memory.
> >
> > This patch only allows (vector long long) { 0, 0 } and
> > (vector long long) { -1, -1 } as easy V2DI constants.  There are some other
> > constants that could be generated using the Altivec instructions in the future.
> >
> > I bootstrapped this patch and had no regressions with make check.  Is it ok to
> > install?
> >
> > [gcc]
> > 2011-03-08  Michael Meissner  <meissner@linux.vnet.ibm.com>
> >
> >        PR target/47755
> >        * config/rs6000/rs6000.c (easy_altivec_constant): Correctly handle
> >        V2DI/V2DF constants.  Only all 0's or all 1's are easy.
> >        (output_vec_const_move): Ditto.
> >
> > [gcc/testsuite]
> > 2011-03-08  Michael Meissner  <meissner@linux.vnet.ibm.com>
> >
> >        PR target/47755
> >        * gcc.target/powerpc/pr47755-2.c: New file.
> 
> Should the CONST_INT test be wrapped in #ifdef HOST_BITS_PER_WIDE_INT?

I dunno.  Given that it is harmless on 64-bit, I would tend to not put the
#ifdef there.  But I would have no objection to adding it.
 
> Okay.
> 
> Thanks, David
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 170788)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -4946,6 +4946,29 @@  easy_altivec_constant (rtx op, enum mach
   else if (mode != GET_MODE (op))
     return false;
 
+  /* V2DI/V2DF was added with VSX.  Only allow 0 and all 1's as easy
+     constants.  */
+  if (mode == V2DFmode)
+    return zero_constant (op, mode);
+
+  if (mode == V2DImode)
+    {
+      /* In case the compiler is built 32-bit, CONST_DOUBLE constants are not
+	 easy.  */
+      if (GET_CODE (CONST_VECTOR_ELT (op, 0)) != CONST_INT
+	  || GET_CODE (CONST_VECTOR_ELT (op, 1)) != CONST_INT)
+	return false;
+
+      if (zero_constant (op, mode))
+	return true;
+
+      if (INTVAL (CONST_VECTOR_ELT (op, 0)) == -1
+	  && INTVAL (CONST_VECTOR_ELT (op, 1)) == -1)
+	return true;
+
+      return false;
+    }
+
   /* Start with a vspltisw.  */
   step = GET_MODE_NUNITS (mode) / 4;
   copies = 1;
@@ -5022,8 +5045,16 @@  output_vec_const_move (rtx *operands)
   vec = operands[1];
   mode = GET_MODE (dest);
 
-  if (TARGET_VSX && zero_constant (vec, mode))
-    return "xxlxor %x0,%x0,%x0";
+  if (TARGET_VSX)
+    {
+      if (zero_constant (vec, mode))
+	return "xxlxor %x0,%x0,%x0";
+
+      if (mode == V2DImode
+	  && INTVAL (CONST_VECTOR_ELT (vec, 0)) == -1
+	  && INTVAL (CONST_VECTOR_ELT (vec, 1)) == -1)
+	return "vspltisw %0,-1";
+    }
 
   if (TARGET_ALTIVEC)
     {
Index: gcc/testsuite/gcc.target/powerpc/pr47755-2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr47755-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr47755-2.c	(revision 0)
@@ -0,0 +1,134 @@ 
+/* { dg-do run { target { powerpc*-*-* } } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-O3 -mcpu=power7" } */
+
+/* PR 47755: Make sure compiler generates correct code for various
+   V2DI constants.  */
+
+#ifdef DEBUG
+#include <stdio.h>
+
+static int num_errors;
+#define FAIL_LL(A, B) \
+  (num_errors++, printf ("Fail (%i, %i)\n", (int)(A), (int)(B)))
+#define FAIL_I(A, B, C, D) \
+  (num_errors++, \
+  printf ("Fail (%i, %i, %i, %i)\n", (int)(A), (int)(B), (int)(C), (int)(D)))
+
+#else
+extern void abort (void) __attribute__((__noreturn__));
+#define FAIL_LL(A, B) abort ()
+#define FAIL_I(A, B, C, D) abort ()
+#endif
+
+static test_ll (vector long long, long long, long long) __attribute__((__noinline__));
+
+static
+test_ll (vector long long v, long long a, long long b)
+{
+  union {
+    vector long long v;
+    long long ll[2];
+  } u;
+
+  u.v = v;
+  if (u.ll[0] != a && u.ll[1] != b)
+    FAIL_LL (a, b);
+}
+
+#define TEST_LL(A,B) test_ll ((vector long long){ (A), (B) }, (A), (B))
+
+static test_i (vector int, int, int, int, int) __attribute__((__noinline__));
+
+static
+test_i (vector int v, int a, int b, int c, int d)
+{
+  union {
+    vector int v;
+    int i[4];
+  } u;
+
+  u.v = v;
+  if (u.i[0] != a && u.i[1] != b && u.i[2] != c && u.i[3] != d)
+    FAIL_I (a, b, c, d);
+}
+
+#define TEST_I(A,B,C,D) \
+  test_i ((vector int){ (A), (B), (C), (D) }, (A), (B), (C), (D))
+
+int
+main (void)
+{
+  TEST_LL (-2LL, -2LL);
+  TEST_LL (-2LL, -1LL);
+  TEST_LL (-2LL,  0LL);
+  TEST_LL (-2LL,  1LL);
+  TEST_LL (-2LL,  2LL);
+
+  TEST_LL (-1LL, -2LL);
+  TEST_LL (-1LL, -1LL);
+  TEST_LL (-1LL,  0LL);
+  TEST_LL (-1LL,  1LL);
+  TEST_LL (-1LL,  2LL);
+
+  TEST_LL (0LL, -2LL);
+  TEST_LL (0LL, -1LL);
+  TEST_LL (0LL,  0LL);
+  TEST_LL (0LL,  1LL);
+  TEST_LL (0LL,  2LL);
+
+  TEST_LL (1LL, -2LL);
+  TEST_LL (1LL, -1LL);
+  TEST_LL (1LL,  0LL);
+  TEST_LL (1LL,  1LL);
+  TEST_LL (1LL,  2LL);
+
+  TEST_LL (2LL, -2LL);
+  TEST_LL (2LL, -1LL);
+  TEST_LL (2LL,  0LL);
+  TEST_LL (2LL,  1LL);
+  TEST_LL (2LL,  2LL);
+
+  /* We could use VSPLTI instructions for these tests.  */
+  TEST_LL (0x0101010101010101LL, 0x0101010101010101LL);
+  TEST_LL (0x0001000100010001LL, 0x0001000100010001LL);
+  TEST_LL (0x0000000100000001LL, 0x0000000100000001LL);
+
+  TEST_LL (0x0404040404040404LL, 0x0404040404040404LL);
+  TEST_LL (0x0004000400040004LL, 0x0004000400040004LL);
+  TEST_LL (0x0000000400000004LL, 0x0000000400000004LL);
+
+  TEST_LL (0xf8f8f8f8f8f8f8f8LL, 0xf8f8f8f8f8f8f8f8LL);
+  TEST_LL (0xfff8fff8fff8fff8LL, 0xfff8fff8fff8fff8LL);
+  TEST_LL (0xfffffff8fffffff8LL, 0xfffffff8fffffff8LL);
+
+  /* We could use VSPLTI instructions for these tests.  */
+  TEST_I (-2, -2, -2, -2);
+  TEST_I (-1, -1, -1, -1);
+  TEST_I ( 0,  0,  0,  0);
+  TEST_I ( 1,  1,  1,  1);
+  TEST_I ( 2,  2,  2,  2);
+
+  TEST_I (0x01010101, 0x01010101, 0x01010101, 0x01010101);
+  TEST_I (0x00010001, 0x00010001, 0x00010001, 0x00010001);
+
+  TEST_I (0x02020202, 0x02020202, 0x02020202, 0x02020202);
+  TEST_I (0x00020002, 0x00020002, 0x00020002, 0x00020002);
+
+  TEST_I (0xf8f8f8f8, 0xf8f8f8f8, 0xf8f8f8f8, 0xf8f8f8f8);
+  TEST_I (0xfff8fff8, 0xfff8fff8, 0xfff8fff8, 0xfff8fff8);
+
+  /* non-easy constants.  */
+  TEST_I (-2, -1,  0,  1);
+  TEST_I ( 1,  0, -1, -2);
+
+  TEST_I (-1, -1,  0,  0);
+  TEST_I ( 0,  0, -1, -1);
+
+#ifdef DEBUG
+  printf ("%d error%s\n", num_errors, (num_errors == 1) ? "" : "s");
+#endif
+
+  return 0;
+};