* Re: [PATCH], Fix PR/target 80099 (internal error with -mno-upper-regs-sf)
2017-04-14 22:56 ` Segher Boessenkool
@ 2017-04-15 6:10 ` Michael Meissner
2017-04-18 10:38 ` Segher Boessenkool
0 siblings, 1 reply; 6+ messages in thread
From: Michael Meissner @ 2017-04-15 6:10 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Michael Meissner, gcc-patches, David Edelsohn, Bill Schmidt
[-- Attachment #1: Type: text/plain, Size: 3205 bytes --]
On Fri, Apr 14, 2017 at 05:07:17PM -0500, Segher Boessenkool wrote:
> On Fri, Apr 14, 2017 at 05:43:28PM -0400, Michael Meissner wrote:
> > On Fri, Apr 14, 2017 at 03:48:47AM -0500, Segher Boessenkool wrote:
> > > On Wed, Apr 12, 2017 at 06:45:31PM -0400, Michael Meissner wrote:
> > > > The problem is rs6000_expand_vector_extract did not check for SFmode being
> > > > allowed in the Altivec (upper) registers, but the insn implementing the
> > > > variable extract had it as a condition.
> > > >
> > > > In looking at the variable extract code, it currently does not require SFmode
> > > > to go in the Altivec registers, but it does require DImode to go into the
> > > > Altivec registers (vec_extract of V2DFmode will require DFmode to go in Altivec
> > > > registers instead of DImode).
> > >
> > >
> > > > @@ -7586,15 +7586,23 @@ rs6000_expand_vector_extract (rtx target
> > > > switch (mode)
> > > > {
> > > > case V2DFmode:
> > > > - emit_insn (gen_vsx_extract_v2df_var (target, vec, elt));
> > > > - return;
> > > > + if (TARGET_UPPER_REGS_DF)
> > > > + {
> > > > + emit_insn (gen_vsx_extract_v2df_var (target, vec, elt));
> > > > + return;
> > > > + }
> > > > + break;
> > >
> > > If the option is not set, we then ICE later on as far as I see (since
> > > elt is not a CONST_INT). Is that what we want, can that not happen?
> > > In that case, please just do an assert (TARGET_UPPER_REGS_DF) here?
> >
> > No. I guess I was unclear.
>
> I'm asking about this exact code that I quoted. After the change here,
> if not TARGET_UPPER_REGS_DF, it breaks out of the switch, and then
> immediately ICEs (failing assert on CONST_INT_P). Right? Or do I read
> this wrong.
You are right, and my patch was too complicated. All I needed to do was remove
the upper register checks. In looking at it, since the insn before being split
has both register and memory versions, if the register allocator can't allocate
a register, it will push the value on to the stack, and adjust the address with
the variable index and do a load. Performance with the store and load, likely
will not be ideal, but it should work.
Because of the interactions with the debug switches -mno-upper-regs-<xxx>, I
decided to add tests for all of the variable extract built-ins with each of the
no-upper regs switches.
I've tested this on a little endian power8 system and it bootstrapped and ran
make check with no regressions. Is it ok for the trunk?
[gcc]
2017-04-15 Michael Meissner <meissner@linux.vnet.ibm.com>
PR target/80099
* config/rs6000/rs6000.c (rs6000_expand_vector_extract): Eliminate
unneeded test for TARGET_UPPER_REGS_SF.
* config/rs6000/vsx.md (vsx_extract_v4sf_var): Likewise.
[gcc/testsuite]
2017-04-15 Michael Meissner <meissner@linux.vnet.ibm.com>
PR target/80099
* gcc.target/powerpc/pr80099-1.c: New test.
* gcc.target/powerpc/pr80099-2.c: Likewise.
* gcc.target/powerpc/pr80099-3.c: Likewise.
* gcc.target/powerpc/pr80099-4.c: Likewise.
* gcc.target/powerpc/pr80099-5.c: Likewise.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797
[-- Attachment #2: pr80099.patch02b --]
[-- Type: text/plain, Size: 14545 bytes --]
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 246815)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -7562,12 +7562,8 @@ rs6000_expand_vector_extract (rtx target
return;
case V4SFmode:
- if (TARGET_UPPER_REGS_SF)
- {
- emit_insn (gen_vsx_extract_v4sf_var (target, vec, elt));
- return;
- }
- break;
+ emit_insn (gen_vsx_extract_v4sf_var (target, vec, elt));
+ return;
case V4SImode:
emit_insn (gen_vsx_extract_v4si_var (target, vec, elt));
Index: gcc/config/rs6000/vsx.md
===================================================================
--- gcc/config/rs6000/vsx.md (revision 246815)
+++ gcc/config/rs6000/vsx.md (working copy)
@@ -2419,8 +2419,7 @@ (define_insn_and_split "vsx_extract_v4sf
UNSPEC_VSX_EXTRACT))
(clobber (match_scratch:DI 3 "=r,&b,&b"))
(clobber (match_scratch:V2DI 4 "=&v,X,X"))]
- "VECTOR_MEM_VSX_P (V4SFmode) && TARGET_DIRECT_MOVE_64BIT
- && TARGET_UPPER_REGS_SF"
+ "VECTOR_MEM_VSX_P (V4SFmode) && TARGET_DIRECT_MOVE_64BIT"
"#"
"&& reload_completed"
[(const_int 0)]
Index: gcc/testsuite/gcc.target/powerpc/pr80099-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr80099-1.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr80099-1.c (revision 0)
@@ -0,0 +1,12 @@
+/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
+/* { dg-options "-mcpu=power8 -O2 -mno-upper-regs-sf" } */
+
+/* PR target/80099: compiler internal error if -mno-upper-regs-sf used. */
+
+int a;
+int int_from_mem (vector float *c)
+{
+ return __builtin_vec_extract (*c, a);
+}
Index: gcc/testsuite/gcc.target/powerpc/pr80099-2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr80099-2.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr80099-2.c (revision 0)
@@ -0,0 +1,128 @@
+/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
+/* { dg-options "-mcpu=power8 -O2 -mno-upper-regs-sf" } */
+
+/* PR target/80099 was an issue with -mno-upper-regs-sf. Test for all variable
+ extract types with various -mno-upper-regs-* options. */
+
+double
+d_extract_arg_n (vector double v, unsigned long n)
+{
+ return __builtin_vec_extract (v, n);
+}
+
+float
+f_extract_arg_n (vector float v, unsigned long n)
+{
+ return __builtin_vec_extract (v, n);
+}
+
+long
+sl_extract_arg_n (vector long v, unsigned long n)
+{
+ return (long) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+ul_extract_arg_n (vector unsigned long v, unsigned long n)
+{
+ return (unsigned long) __builtin_vec_extract (v, n);
+}
+
+long
+si_extract_arg_n (vector int v, unsigned long n)
+{
+ return (int) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+ui_extract_arg_n (vector unsigned int v, unsigned long n)
+{
+ return (unsigned int) __builtin_vec_extract (v, n);
+}
+
+long
+ss_extract_arg_n (vector short v, unsigned long n)
+{
+ return (short) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+us_extract_arg_n (vector unsigned short v, unsigned long n)
+{
+ return (unsigned short) __builtin_vec_extract (v, n);
+}
+
+long
+sc_extract_arg_n (vector signed char v, unsigned long n)
+{
+ return (signed char) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+uc_extract_arg_n (vector unsigned char v, unsigned long n)
+{
+ return (unsigned char) __builtin_vec_extract (v, n);
+}
+
+\f
+double
+d_extract_mem_n (vector double *p, unsigned long n)
+{
+ return __builtin_vec_extract (*p, n);
+}
+
+float
+f_extract_mem_n (vector float *p, unsigned long n)
+{
+ return __builtin_vec_extract (*p, n);
+}
+
+long
+sl_extract_mem_n (vector long *p, unsigned long n)
+{
+ return (long) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+ul_extract_mem_n (vector unsigned long *p, unsigned long n)
+{
+ return (unsigned long) __builtin_vec_extract (*p, n);
+}
+
+long
+si_extract_mem_n (vector int *p, unsigned long n)
+{
+ return (int) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+ui_extract_mem_n (vector unsigned int *p, unsigned long n)
+{
+ return (unsigned int) __builtin_vec_extract (*p, n);
+}
+
+long
+ss_extract_mem_n (vector short *p, unsigned long n)
+{
+ return (short) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+us_extract_mem_n (vector unsigned short *p, unsigned long n)
+{
+ return (unsigned short) __builtin_vec_extract (*p, n);
+}
+
+long
+sc_extract_mem_n (vector signed char *p, unsigned long n)
+{
+ return (signed char) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+uc_extract_mem_n (vector unsigned char *p, unsigned long n)
+{
+ return (unsigned char) __builtin_vec_extract (*p, n);
+}
Index: gcc/testsuite/gcc.target/powerpc/pr80099-3.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr80099-3.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr80099-3.c (revision 0)
@@ -0,0 +1,128 @@
+/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
+/* { dg-options "-mcpu=power8 -O2 -mno-upper-regs-df" } */
+
+/* PR target/80099 was an issue with -mno-upper-regs-sf. Test for all variable
+ extract types with various -mno-upper-regs-* options. */
+
+double
+d_extract_arg_n (vector double v, unsigned long n)
+{
+ return __builtin_vec_extract (v, n);
+}
+
+float
+f_extract_arg_n (vector float v, unsigned long n)
+{
+ return __builtin_vec_extract (v, n);
+}
+
+long
+sl_extract_arg_n (vector long v, unsigned long n)
+{
+ return (long) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+ul_extract_arg_n (vector unsigned long v, unsigned long n)
+{
+ return (unsigned long) __builtin_vec_extract (v, n);
+}
+
+long
+si_extract_arg_n (vector int v, unsigned long n)
+{
+ return (int) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+ui_extract_arg_n (vector unsigned int v, unsigned long n)
+{
+ return (unsigned int) __builtin_vec_extract (v, n);
+}
+
+long
+ss_extract_arg_n (vector short v, unsigned long n)
+{
+ return (short) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+us_extract_arg_n (vector unsigned short v, unsigned long n)
+{
+ return (unsigned short) __builtin_vec_extract (v, n);
+}
+
+long
+sc_extract_arg_n (vector signed char v, unsigned long n)
+{
+ return (signed char) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+uc_extract_arg_n (vector unsigned char v, unsigned long n)
+{
+ return (unsigned char) __builtin_vec_extract (v, n);
+}
+
+\f
+double
+d_extract_mem_n (vector double *p, unsigned long n)
+{
+ return __builtin_vec_extract (*p, n);
+}
+
+float
+f_extract_mem_n (vector float *p, unsigned long n)
+{
+ return __builtin_vec_extract (*p, n);
+}
+
+long
+sl_extract_mem_n (vector long *p, unsigned long n)
+{
+ return (long) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+ul_extract_mem_n (vector unsigned long *p, unsigned long n)
+{
+ return (unsigned long) __builtin_vec_extract (*p, n);
+}
+
+long
+si_extract_mem_n (vector int *p, unsigned long n)
+{
+ return (int) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+ui_extract_mem_n (vector unsigned int *p, unsigned long n)
+{
+ return (unsigned int) __builtin_vec_extract (*p, n);
+}
+
+long
+ss_extract_mem_n (vector short *p, unsigned long n)
+{
+ return (short) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+us_extract_mem_n (vector unsigned short *p, unsigned long n)
+{
+ return (unsigned short) __builtin_vec_extract (*p, n);
+}
+
+long
+sc_extract_mem_n (vector signed char *p, unsigned long n)
+{
+ return (signed char) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+uc_extract_mem_n (vector unsigned char *p, unsigned long n)
+{
+ return (unsigned char) __builtin_vec_extract (*p, n);
+}
Index: gcc/testsuite/gcc.target/powerpc/pr80099-4.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr80099-4.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr80099-4.c (revision 0)
@@ -0,0 +1,128 @@
+/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
+/* { dg-options "-mcpu=power8 -O2 -mno-upper-regs-di" } */
+
+/* PR target/80099 was an issue with -mno-upper-regs-sf. Test for all variable
+ extract types with various -mno-upper-regs-* options. */
+
+double
+d_extract_arg_n (vector double v, unsigned long n)
+{
+ return __builtin_vec_extract (v, n);
+}
+
+float
+f_extract_arg_n (vector float v, unsigned long n)
+{
+ return __builtin_vec_extract (v, n);
+}
+
+long
+sl_extract_arg_n (vector long v, unsigned long n)
+{
+ return (long) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+ul_extract_arg_n (vector unsigned long v, unsigned long n)
+{
+ return (unsigned long) __builtin_vec_extract (v, n);
+}
+
+long
+si_extract_arg_n (vector int v, unsigned long n)
+{
+ return (int) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+ui_extract_arg_n (vector unsigned int v, unsigned long n)
+{
+ return (unsigned int) __builtin_vec_extract (v, n);
+}
+
+long
+ss_extract_arg_n (vector short v, unsigned long n)
+{
+ return (short) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+us_extract_arg_n (vector unsigned short v, unsigned long n)
+{
+ return (unsigned short) __builtin_vec_extract (v, n);
+}
+
+long
+sc_extract_arg_n (vector signed char v, unsigned long n)
+{
+ return (signed char) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+uc_extract_arg_n (vector unsigned char v, unsigned long n)
+{
+ return (unsigned char) __builtin_vec_extract (v, n);
+}
+
+\f
+double
+d_extract_mem_n (vector double *p, unsigned long n)
+{
+ return __builtin_vec_extract (*p, n);
+}
+
+float
+f_extract_mem_n (vector float *p, unsigned long n)
+{
+ return __builtin_vec_extract (*p, n);
+}
+
+long
+sl_extract_mem_n (vector long *p, unsigned long n)
+{
+ return (long) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+ul_extract_mem_n (vector unsigned long *p, unsigned long n)
+{
+ return (unsigned long) __builtin_vec_extract (*p, n);
+}
+
+long
+si_extract_mem_n (vector int *p, unsigned long n)
+{
+ return (int) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+ui_extract_mem_n (vector unsigned int *p, unsigned long n)
+{
+ return (unsigned int) __builtin_vec_extract (*p, n);
+}
+
+long
+ss_extract_mem_n (vector short *p, unsigned long n)
+{
+ return (short) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+us_extract_mem_n (vector unsigned short *p, unsigned long n)
+{
+ return (unsigned short) __builtin_vec_extract (*p, n);
+}
+
+long
+sc_extract_mem_n (vector signed char *p, unsigned long n)
+{
+ return (signed char) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+uc_extract_mem_n (vector unsigned char *p, unsigned long n)
+{
+ return (unsigned char) __builtin_vec_extract (*p, n);
+}
Index: gcc/testsuite/gcc.target/powerpc/pr80099-5.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr80099-5.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr80099-5.c (revision 0)
@@ -0,0 +1,128 @@
+/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
+/* { dg-options "-mcpu=power8 -O2 -mno-upper-regs" } */
+
+/* PR target/80099 was an issue with -mno-upper-regs-sf. Test for all variable
+ extract types with various -mno-upper-regs-* options. */
+
+double
+d_extract_arg_n (vector double v, unsigned long n)
+{
+ return __builtin_vec_extract (v, n);
+}
+
+float
+f_extract_arg_n (vector float v, unsigned long n)
+{
+ return __builtin_vec_extract (v, n);
+}
+
+long
+sl_extract_arg_n (vector long v, unsigned long n)
+{
+ return (long) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+ul_extract_arg_n (vector unsigned long v, unsigned long n)
+{
+ return (unsigned long) __builtin_vec_extract (v, n);
+}
+
+long
+si_extract_arg_n (vector int v, unsigned long n)
+{
+ return (int) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+ui_extract_arg_n (vector unsigned int v, unsigned long n)
+{
+ return (unsigned int) __builtin_vec_extract (v, n);
+}
+
+long
+ss_extract_arg_n (vector short v, unsigned long n)
+{
+ return (short) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+us_extract_arg_n (vector unsigned short v, unsigned long n)
+{
+ return (unsigned short) __builtin_vec_extract (v, n);
+}
+
+long
+sc_extract_arg_n (vector signed char v, unsigned long n)
+{
+ return (signed char) __builtin_vec_extract (v, n);
+}
+
+unsigned long
+uc_extract_arg_n (vector unsigned char v, unsigned long n)
+{
+ return (unsigned char) __builtin_vec_extract (v, n);
+}
+
+\f
+double
+d_extract_mem_n (vector double *p, unsigned long n)
+{
+ return __builtin_vec_extract (*p, n);
+}
+
+float
+f_extract_mem_n (vector float *p, unsigned long n)
+{
+ return __builtin_vec_extract (*p, n);
+}
+
+long
+sl_extract_mem_n (vector long *p, unsigned long n)
+{
+ return (long) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+ul_extract_mem_n (vector unsigned long *p, unsigned long n)
+{
+ return (unsigned long) __builtin_vec_extract (*p, n);
+}
+
+long
+si_extract_mem_n (vector int *p, unsigned long n)
+{
+ return (int) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+ui_extract_mem_n (vector unsigned int *p, unsigned long n)
+{
+ return (unsigned int) __builtin_vec_extract (*p, n);
+}
+
+long
+ss_extract_mem_n (vector short *p, unsigned long n)
+{
+ return (short) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+us_extract_mem_n (vector unsigned short *p, unsigned long n)
+{
+ return (unsigned short) __builtin_vec_extract (*p, n);
+}
+
+long
+sc_extract_mem_n (vector signed char *p, unsigned long n)
+{
+ return (signed char) __builtin_vec_extract (*p, n);
+}
+
+unsigned long
+uc_extract_mem_n (vector unsigned char *p, unsigned long n)
+{
+ return (unsigned char) __builtin_vec_extract (*p, n);
+}
^ permalink raw reply [flat|nested] 6+ messages in thread