public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH], Fix PR/target 80099 (internal error with -mno-upper-regs-sf)
@ 2017-04-12 22:45 Michael Meissner
  2017-04-14  8:48 ` Segher Boessenkool
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Meissner @ 2017-04-12 22:45 UTC (permalink / raw)
  To: gcc-patches, Segher Boessenkool, David Edelsohn, Bill Schmidt

[-- Attachment #1: Type: text/plain, Size: 1244 bytes --]

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

I have tested this patch on a little endian power8 system and there were no
regressions with either bootstrap or make check.

[gcc]
2017-04-12  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/80099
	* config/rs6000/rs6000.c (rs6000_expand_vector_extract): Make sure
	that DFmode or DImode as appropriate can go in Altivec registers
	before generating the faster sequences for variable vec_extracts.
	* config/rs6000/vsx.md (vsx_extract_v4sf): Remove unneeded
	TARGET_UPPER_REGS_SF condition.

[gcc/testsuite]
2017-04-12  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/80099
	* gcc.target/powerpc/pr80099.c: New test.

-- 
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.patch01b --]
[-- Type: text/plain, Size: 3000 bytes --]

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 246852)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -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;
 
 	case V2DImode:
-	  emit_insn (gen_vsx_extract_v2di_var (target, vec, elt));
-	  return;
+	  if (TARGET_UPPER_REGS_DI)
+	    {
+	      emit_insn (gen_vsx_extract_v2di_var (target, vec, elt));
+	      return;
+	    }
+	  break;
 
 	case V4SFmode:
-	  if (TARGET_UPPER_REGS_SF)
+	  if (TARGET_UPPER_REGS_DI)
 	    {
 	      emit_insn (gen_vsx_extract_v4sf_var (target, vec, elt));
 	      return;
@@ -7602,16 +7610,28 @@ rs6000_expand_vector_extract (rtx target
 	  break;
 
 	case V4SImode:
-	  emit_insn (gen_vsx_extract_v4si_var (target, vec, elt));
-	  return;
+	  if (TARGET_UPPER_REGS_DI)
+	    {
+	      emit_insn (gen_vsx_extract_v4si_var (target, vec, elt));
+	      return;
+	    }
+	  break;
 
 	case V8HImode:
-	  emit_insn (gen_vsx_extract_v8hi_var (target, vec, elt));
-	  return;
+	  if (TARGET_UPPER_REGS_DI)
+	    {
+	      emit_insn (gen_vsx_extract_v8hi_var (target, vec, elt));
+	      return;
+	    }
+	  break;
 
 	case V16QImode:
-	  emit_insn (gen_vsx_extract_v16qi_var (target, vec, elt));
-	  return;
+	  if (TARGET_UPPER_REGS_DI)
+	    {
+	      emit_insn (gen_vsx_extract_v16qi_var (target, vec, elt));
+	      return;
+	    }
+	  break;
 
 	default:
 	  gcc_unreachable ();
Index: gcc/config/rs6000/vsx.md
===================================================================
--- gcc/config/rs6000/vsx.md	(revision 246852)
+++ 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.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr80099.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr80099.c	(working copy)
@@ -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-di" } */
+
+/* PR target/80099: compiler internal error if -mno-upper-regs-di used.  */
+
+int a;
+int int_from_mem (vector float *c)
+{
+  return __builtin_vec_extract (*c, a);
+}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH], Fix PR/target 80099 (internal error with -mno-upper-regs-sf)
  2017-04-12 22:45 [PATCH], Fix PR/target 80099 (internal error with -mno-upper-regs-sf) Michael Meissner
@ 2017-04-14  8:48 ` Segher Boessenkool
  2017-04-14 22:07   ` Michael Meissner
  0 siblings, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2017-04-14  8:48 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, David Edelsohn, Bill Schmidt

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?


Segher

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH], Fix PR/target 80099 (internal error with -mno-upper-regs-sf)
  2017-04-14  8:48 ` Segher Boessenkool
@ 2017-04-14 22:07   ` Michael Meissner
  2017-04-14 22:56     ` Segher Boessenkool
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Meissner @ 2017-04-14 22:07 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Michael Meissner, gcc-patches, David Edelsohn, Bill Schmidt

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.  We DO NOT NEED the test for SFmode in Altivec
registers here, but we do need DImode in Altivec registers.  The problem was
the generator did not not have the test, but this place did.

Before the split, the variable insn looks like:

   (parallel [(set (reg:SF t)			;; constraint ww
		   (unspec:SF [(reg:V4SF u)	;; constraint v
			       (reg:DI v)]	;; constraint r
		   UNSPEC_VSX_EXTRACT))
	      (clobber (reg:DI w))		;; constraint r
	      (clobber (reg:V2DI x))])		;; constraint v

The split generates:

    <various insns to prepared the shift amount for VSLO into 2nd clobber>

    ;; VSLO
    (set (reg:DI x)				;; this is the 2nd clobber
	 (unspec:DI [(reg:V2DI u)		;; this is the vector input
		     (reg:V2DI x)]		;; this is the 2nd clobber
		    UNSPEC_VSX_VSLO))

    ;; XSCVSPDPN
    (set (reg:SF t)				;; this is the destination
	 (unspec:SF [(reg:V2DI x)]		;; this is the 2nd clobber
		    UNSPEC_VSX_CVSPDP))

Because the target constraint is ww, that will be VSX_REGS normally on power8,
and FLOAT_REGS if -mupper-regs-sf is used.  Note, the variable extraction will
not be done this way on power7, since it does not have direct move.

So, since the only place we care about whether or not SFmode can go in Altivec
registers is the desination, the test should be eliminated.  However, in
looking at the code generated, I did discover that we need DImode to go in the
Altivec registers and we weren't checking for that.

Note, variable extraction of DFmode values does not need DImode in Altivec
registers, but it does need DFmode in Altivec registers.

Now that it is on by default, we really should eliminate the -mno-upper-regs-*
options.  They were useful when the code was being debugged, but they are less
useful now.  However, that is not something we should do in stage4.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH], Fix PR/target 80099 (internal error with -mno-upper-regs-sf)
  2017-04-14 22:07   ` Michael Meissner
@ 2017-04-14 22:56     ` Segher Boessenkool
  2017-04-15  6:10       ` Michael Meissner
  0 siblings, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2017-04-14 22:56 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, David Edelsohn, Bill Schmidt

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.


Segher

^ permalink raw reply	[flat|nested] 6+ messages in thread

* 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

* Re: [PATCH], Fix PR/target 80099 (internal error with -mno-upper-regs-sf)
  2017-04-15  6:10       ` Michael Meissner
@ 2017-04-18 10:38         ` Segher Boessenkool
  0 siblings, 0 replies; 6+ messages in thread
From: Segher Boessenkool @ 2017-04-18 10:38 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, David Edelsohn, Bill Schmidt

On Sat, Apr 15, 2017 at 01:43:18AM -0400, Michael Meissner wrote:
> 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?

Much simpler indeed :-)  Looks fine to me, please commit.  Thanks,


Segher


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-04-18 10:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-12 22:45 [PATCH], Fix PR/target 80099 (internal error with -mno-upper-regs-sf) Michael Meissner
2017-04-14  8:48 ` Segher Boessenkool
2017-04-14 22:07   ` Michael Meissner
2017-04-14 22:56     ` Segher Boessenkool
2017-04-15  6:10       ` Michael Meissner
2017-04-18 10:38         ` Segher Boessenkool

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