Re: [PATCH, rs6000] PR89424: __builtin_vec_ext_v1ti (v, i) results in ICE with variable i (RS6000)
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
[PATCH, rs6000] PR89424: __builtin_vec_ext_v1ti (v, i) results in ICE with variable i (RS6000)
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-unknown-linux-gnu (P7 and P8, both -m32 and -m64). Is this ok for trunk and backports? Thanks. gcc/ChangeLog: 2019-04-25 Kelvin Nilsen PR target/89424 * config/rs6000/rs6000.c (rs6000_expand_vector_extract): Add handling of V1TImode. gcc/testsuite/ChangeLog: 2019-04-25 Kelvin Nilsen 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. 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 + +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 /* 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 /* 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 /* 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 /* Def
[PATCH, rs6000] PR89424: __builtin_vec_ext_v1ti (v, i) results in ICE with variable i (RS6000)
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 PR target/89424 * config/rs6000/rs6000.c (rs6000_expand_vector_extract): Add handling of V1TImode. gcc/testsuite/ChangeLog: 2019-04-25 Kelvin Nilsen 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. 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 + +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 /* 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 /* 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 /* 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 /* Define this after