diff mbox series

[rs6000] PR89424: __builtin_vec_ext_v1ti (v, i) results in ICE with variable i (RS6000)

Message ID ec96a9a6-71c3-4f60-94c3-3bcabd95c22a@linux.ibm.com
State New
Headers show
Series [rs6000] PR89424: __builtin_vec_ext_v1ti (v, i) results in ICE with variable i (RS6000) | expand

Commit Message

Kelvin Nilsen April 25, 2019, 4:02 p.m. UTC 
The attached patch resolves the issue described in this problem report.  The patch has been bootstrapped and tested without regressions on powerpc64le-unknown-linux-gnu (both P8 and P9) and on powerpc64-linux (P7 and P8, both -m32 and -m64).

Is this ok for trunk and backports?


Thanks.

gcc/ChangeLog:

2019-04-25  Kelvin Nilsen  <kelvin@gcc.gnu.org>

	PR target/89424
	* config/rs6000/rs6000.c (rs6000_expand_vector_extract): Add
	handling of V1TImode.

gcc/testsuite/ChangeLog:

2019-04-25  Kelvin Nilsen  <kelvin@gcc.gnu.org>

	PR target/89424
	* gcc.target/powerpc/pr89424-0.c: New test.
	* gcc.target/powerpc/vsx-builtin-13a.c: Define macro PR89424 to
	enable testing of newly patched capability.
	* gcc.target/powerpc/vsx-builtin-13b.c: Likewise.
	* gcc.target/powerpc/vsx-builtin-20a.c: Likewise.
	* gcc.target/powerpc/vsx-builtin-20b.c: Likewise.

Comments

Segher Boessenkool April 25, 2019, 7:40 p.m. UTC | #1 
On Thu, Apr 25, 2019 at 11:02:09AM -0500, Kelvin Nilsen wrote:
> The attached patch resolves the issue described in this problem report.  The patch has been bootstrapped and tested without regressions on powerpc64le-unknown-linux-gnu (both P8 and P9) and on powerpc64-linux (P7 and P8, both -m32 and -m64).
> 
> Is this ok for trunk and backports?

It is okay for trunk and backports, but you may want to hold off until
after the GCC 9 release.  For the gcc-9 branch you need the RMs' approval.

A few testcase comments:

> --- gcc/testsuite/gcc.target/powerpc/pr89424-0.c	(nonexistent)
> +++ gcc/testsuite/gcc.target/powerpc/pr89424-0.c	(working copy)
> @@ -0,0 +1,78 @@
> +/* { dg-do run { target { powerpc*-*-* && lp64 } } } */
> +/* { dg-skip-if "" { powerpc*-*-darwin* } } */
> +/* { dg-require-effective-target vsx_hw } */

I think such "darwin" lines are useless; vsx_hw will not be true on Darwin.

> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { } } */

We do not need this anymore either (and this test never needed it).

> +/* { dg-options "-mvsx" } */


Thanks,


Segher
diff mbox series

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 270513)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -6944,6 +6944,10 @@  rs6000_expand_vector_extract (rtx target, rtx vec,
 
       switch (mode)
 	{
+	case E_V1TImode:
+	  emit_move_insn (target, gen_lowpart (TImode, vec));
+	  return;
+
 	case E_V2DFmode:
 	  emit_insn (gen_vsx_extract_v2df_var (target, vec, elt));
 	  return;
Index: gcc/testsuite/gcc.target/powerpc/pr89424-0.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr89424-0.c	(nonexistent)
+++ gcc/testsuite/gcc.target/powerpc/pr89424-0.c	(working copy)
@@ -0,0 +1,78 @@ 
+/* { dg-do run { target { powerpc*-*-* && lp64 } } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } } */
+/* { dg-require-effective-target vsx_hw } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { } } */
+/* { dg-options "-mvsx" } */
+
+/* This test should run the same on any target that supports vsx
+   instructions.  Intentionally not specifying cpu in order to test
+   all code generation paths.  */
+
+#include <altivec.h>
+
+extern void abort (void);
+
+/* Define PR89626 after that pr is addressed.  */
+#ifdef PR89626
+#define SIGNED
+#else
+#define SIGNED signed
+#endif
+
+#define CONST0		(((__int128) 31415926539) << 60)
+
+/* Test that indices > length of vector are applied modulo the vector
+   length.  */
+
+
+/* Test for variable selector and vector residing in register.  */
+__attribute__((noinline))
+__int128 ei (vector SIGNED __int128 v, int i)
+{
+  return __builtin_vec_ext_v1ti (v, i);
+}
+
+/* Test for variable selector and vector residing in memory.  */
+__int128 mei (vector SIGNED __int128 *vp, int i)
+{
+  return __builtin_vec_ext_v1ti (*vp, i);
+}
+
+int main (int argc, char *argv[]) {
+  vector SIGNED __int128 dv = { CONST0 };
+  __int128 d;
+
+  d = ei (dv, 0);
+  if (d != CONST0)
+    abort ();
+
+  d = ei (dv, 1);
+  if (d != CONST0)
+    abort ();
+
+  d = ei (dv, 2);
+  if (d != CONST0)
+    abort ();
+
+  d = ei (dv, 3);
+  if (d != CONST0)
+    abort ();
+
+  d = mei (&dv, 0);
+  if (d != CONST0)
+    abort ();
+
+  d = mei (&dv, 1);
+  if (d != CONST0)
+    abort ();
+
+  d = mei (&dv, 2);
+  if (d != CONST0)
+    abort ();
+
+  d = mei (&dv, 3);
+  if (d != CONST0)
+    abort ();
+
+  return 0;
+}
Index: gcc/testsuite/gcc.target/powerpc/vsx-builtin-13a.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vsx-builtin-13a.c	(revision 270513)
+++ gcc/testsuite/gcc.target/powerpc/vsx-builtin-13a.c	(working copy)
@@ -9,7 +9,7 @@ 
 #include <altivec.h>
 
 /* Define this after PR89424 is addressed.  */
-#undef PR89424
+#define PR89424
 
 /* Define this after PR89626 is addressed.  */
 #undef PR89626
Index: gcc/testsuite/gcc.target/powerpc/vsx-builtin-13b.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vsx-builtin-13b.c	(revision 270513)
+++ gcc/testsuite/gcc.target/powerpc/vsx-builtin-13b.c	(working copy)
@@ -9,7 +9,7 @@ 
 #include <altivec.h>
 
 /* Define this after PR89424 is addressed.  */
-#undef PR89424
+#define PR89424
 
 /* Define this after PR89626 is addressed.  */
 #undef PR89626
Index: gcc/testsuite/gcc.target/powerpc/vsx-builtin-20a.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vsx-builtin-20a.c	(revision 270513)
+++ gcc/testsuite/gcc.target/powerpc/vsx-builtin-20a.c	(working copy)
@@ -9,7 +9,7 @@ 
 #include <altivec.h>
 
 /* Define this after PR89424 is addressed.  */
-#undef PR89424
+#define PR89424
 
 extern void abort (void);
 
Index: gcc/testsuite/gcc.target/powerpc/vsx-builtin-20b.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vsx-builtin-20b.c	(revision 270513)
+++ gcc/testsuite/gcc.target/powerpc/vsx-builtin-20b.c	(working copy)
@@ -9,7 +9,7 @@ 
 #include <altivec.h>
 
 /* Define this after PR89424 is addressed.  */
-#undef PR89424
+#define PR89424
 
 extern void abort (void);