diff mbox

[rs6000] pr80482 Relax vector builtin parameter checks

Message ID c7c6490e-9c6d-f01c-5633-235656214745@linux.vnet.ibm.com
State New
Headers show

Commit Message

Bill Seurer April 24, 2017, 10:38 p.m. UTC 
[PATCH, rs6000] pr80482 Relax vector builtin parameter checks

This patch changes the parameter testing for powerpc vector builtins to relax
the existing requirement that the parameters be identical to instead that they
be compatible.  This allows for mixing parameters with differing qualified
(const, volatile, etc.) types.

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80482 for more information.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu and
powerpc64be-unknown-linux-gnu with no regressions.  Is this ok for trunk?

[gcc]

2017-04-24  Bill Seurer  <seurer@linux.vnet.ibm.com>

	* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): Change
	type checks to test for compatibility instead of equality.

[gcc/testsuite]

2017-04-24  Bill Seurer  <seurer@linux.vnet.ibm.com>

	* gcc.target/powerpc/vec-constvolatile.c: New test.

Comments

Jakub Jelinek April 24, 2017, 10:42 p.m. UTC | #1 
On Mon, Apr 24, 2017 at 05:38:58PM -0500, Bill Seurer wrote:
> [PATCH, rs6000] pr80482 Relax vector builtin parameter checks
> 
> This patch changes the parameter testing for powerpc vector builtins to relax
> the existing requirement that the parameters be identical to instead that they
> be compatible.  This allows for mixing parameters with differing qualified
> (const, volatile, etc.) types.
> 
> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80482 for more information.
> 
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu and
> powerpc64be-unknown-linux-gnu with no regressions.  Is this ok for trunk?
> 
> [gcc]
> 
> 2017-04-24  Bill Seurer  <seurer@linux.vnet.ibm.com>
> 
The ChangeLog entries as well as the commit message should contain
	PR target/80482

> 	* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): Change
> 	type checks to test for compatibility instead of equality.

I'll defer the actual review to rs6000 maintainers.

	Jakub
Segher Boessenkool April 25, 2017, 4:58 a.m. UTC | #2 
On Mon, Apr 24, 2017 at 05:38:58PM -0500, Bill Seurer wrote:
> [PATCH, rs6000] pr80482 Relax vector builtin parameter checks
> 
> This patch changes the parameter testing for powerpc vector builtins to relax
> the existing requirement that the parameters be identical to instead that they
> be compatible.  This allows for mixing parameters with differing qualified
> (const, volatile, etc.) types.
> 
> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80482 for more information.
> 
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu and
> powerpc64be-unknown-linux-gnu with no regressions.  Is this ok for trunk?

It looks fine to me, okay for trunk, thanks (with Jakub's comment taken
care of).

Also okay for the 7 branch if the RMs agree (it fixes a regression from
GCC 6 and it seems unlikely to cause new problems).


Segher


> [gcc]
> 
> 2017-04-24  Bill Seurer  <seurer@linux.vnet.ibm.com>
> 
> 	* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): Change
> 	type checks to test for compatibility instead of equality.
> 
> [gcc/testsuite]
> 
> 2017-04-24  Bill Seurer  <seurer@linux.vnet.ibm.com>
> 
> 	* gcc.target/powerpc/vec-constvolatile.c: New test.
Jakub Jelinek April 25, 2017, 7:44 a.m. UTC | #3 
On Mon, Apr 24, 2017 at 11:58:03PM -0500, Segher Boessenkool wrote:
> On Mon, Apr 24, 2017 at 05:38:58PM -0500, Bill Seurer wrote:
> > [PATCH, rs6000] pr80482 Relax vector builtin parameter checks
> > 
> > This patch changes the parameter testing for powerpc vector builtins to relax
> > the existing requirement that the parameters be identical to instead that they
> > be compatible.  This allows for mixing parameters with differing qualified
> > (const, volatile, etc.) types.
> > 
> > See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80482 for more information.
> > 
> > Bootstrapped and tested on powerpc64le-unknown-linux-gnu and
> > powerpc64be-unknown-linux-gnu with no regressions.  Is this ok for trunk?
> 
> It looks fine to me, okay for trunk, thanks (with Jakub's comment taken
> care of).
> 
> Also okay for the 7 branch if the RMs agree (it fixes a regression from
> GCC 6 and it seems unlikely to cause new problems).

Ok for 7.1 if you commit soon.

	Jakub
diff mbox

Patch

Index: gcc/config/rs6000/rs6000-c.c
===================================================================
--- gcc/config/rs6000/rs6000-c.c	(revision 247111)
+++ gcc/config/rs6000/rs6000-c.c	(working copy)
@@ -5595,11 +5595,11 @@  altivec_resolve_overloaded_builtin (location_t loc
       tree arg1 = (*arglist)[1];
       tree arg1_type = TREE_TYPE (arg1);
 
-      /* Both arguments must be vectors and the types must match.  */
-      if (arg0_type != arg1_type)
-	goto bad;
+      /* Both arguments must be vectors and the types must be compatible.  */
       if (TREE_CODE (arg0_type) != VECTOR_TYPE)
 	goto bad;
+      if (!lang_hooks.types_compatible_p (arg0_type, arg1_type))
+	goto bad;
 
       switch (TYPE_MODE (TREE_TYPE (arg0_type)))
 	{
@@ -5610,8 +5610,8 @@  altivec_resolve_overloaded_builtin (location_t loc
 	  case TImode:
 	    {
 	      /* For scalar types just use a multiply expression.  */
-	      return fold_build2_loc (loc, MULT_EXPR, TREE_TYPE (arg0),
-					arg0, arg1);
+	      return fold_build2_loc (loc, MULT_EXPR, TREE_TYPE (arg0), arg0,
+				      fold_convert (TREE_TYPE (arg0), arg1));
 	    }
 	  case SFmode:
 	    {
@@ -5655,13 +5655,12 @@  altivec_resolve_overloaded_builtin (location_t loc
 	  || (TYPE_MODE (TREE_TYPE (arg0_type)) == SFmode)
 	  || (TYPE_MODE (TREE_TYPE (arg0_type)) == DFmode))
 	{
-	  /* Both arguments must be vectors and the types must match.  */
-	  if (arg0_type != arg1_type)
-	    goto bad;
+	  /* Both arguments must be vectors and the types must be compatible.  */
 	  if (TREE_CODE (arg0_type) != VECTOR_TYPE)
 	    goto bad;
+	  if (!lang_hooks.types_compatible_p (arg0_type, arg1_type))
+	    goto bad;
 
-
 	  switch (TYPE_MODE (TREE_TYPE (arg0_type)))
 	    {
 	      /* vec_cmpneq (va, vb) == vec_nor (vec_cmpeq (va, vb),
@@ -5720,11 +5719,12 @@  altivec_resolve_overloaded_builtin (location_t loc
       tree arg2_type = TREE_TYPE (arg2);
 
       /* All 3 arguments must be vectors of (signed or unsigned) (int or
-	  __int128) and the types must match.  */
-      if ((arg0_type != arg1_type) || (arg1_type != arg2_type))
-	goto bad;
+	  __int128) and the types must be compatible.  */
       if (TREE_CODE (arg0_type) != VECTOR_TYPE)
 	goto bad;
+      if (!lang_hooks.types_compatible_p (arg0_type, arg1_type) ||
+	  !lang_hooks.types_compatible_p (arg1_type, arg2_type))
+	goto bad;
 
       switch (TYPE_MODE (TREE_TYPE (arg0_type)))
 	{
@@ -5783,11 +5783,13 @@  altivec_resolve_overloaded_builtin (location_t loc
       tree arg2_type = TREE_TYPE (arg2);
 
       /* All 3 arguments must be vectors of (signed or unsigned) (int or
-	__int128) and the types must match.  */
-      if (arg0_type != arg1_type || arg1_type != arg2_type)
-	goto bad;
+	__int128) and the types must be compatible.  */
+      /* Both arguments must be vectors and the types must be compatible.  */
       if (TREE_CODE (arg0_type) != VECTOR_TYPE)
 	goto bad;
+      if (!lang_hooks.types_compatible_p (arg0_type, arg1_type) ||
+	  !lang_hooks.types_compatible_p (arg1_type, arg2_type))
+	goto bad;
 
       switch (TYPE_MODE (TREE_TYPE (arg0_type)))
 	{
Index: gcc/testsuite/gcc.target/powerpc/vec-constvolatile.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-constvolatile.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/vec-constvolatile.c	(working copy)
@@ -0,0 +1,31 @@ 
+/* Test that const and volatile qualifiers can mix on vec_mul operands.  */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-maltivec -mvsx" } */
+
+#include <altivec.h>
+
+void P() {
+  const volatile vector float cvva = vec_splats(0.00187682f);
+  volatile vector float vva = vec_splats(0.00187682f);
+  const vector float cva = vec_splats(0.00187682f);
+  vector float va = vec_splats(0.00187682f);
+  vector float dx = {1.0f, 2.0f, 3.0f, 4.0f};
+
+  vector float X1m0 = vec_mul(va, va);
+  vector float X2m0 = vec_mul(va, dx);
+  vector float X3m0 = vec_mul(dx, va);
+
+  vector float X1m1 = vec_mul(cva, cva);
+  vector float X2m1 = vec_mul(cva, dx);
+  vector float X3m1 = vec_mul(dx, cva);
+
+  vector float Y1m2 = vec_mul(vva, vva);
+  vector float Y2m2 = vec_mul(vva, dx);
+  vector float Y3m2 = vec_mul(dx, vva);
+
+  vector float X1m3 = vec_mul(cvva, cvva);
+  vector float X2m3 = vec_mul(cvva, dx);
+  vector float X3m3 = vec_mul(dx, cvva);
+}Index: gcc/config/rs6000/rs6000-c.c
===================================================================
--- gcc/config/rs6000/rs6000-c.c	(revision 247116)
+++ gcc/config/rs6000/rs6000-c.c	(working copy)
@@ -5595,11 +5595,11 @@  altivec_resolve_overloaded_builtin (location_t loc
       tree arg1 = (*arglist)[1];
       tree arg1_type = TREE_TYPE (arg1);
 
-      /* Both arguments must be vectors and the types must match.  */
-      if (arg0_type != arg1_type)
-	goto bad;
+      /* Both arguments must be vectors and the types must be compatible.  */
       if (TREE_CODE (arg0_type) != VECTOR_TYPE)
 	goto bad;
+      if (!lang_hooks.types_compatible_p (arg0_type, arg1_type))
+	goto bad;
 
       switch (TYPE_MODE (TREE_TYPE (arg0_type)))
 	{
@@ -5610,8 +5610,8 @@  altivec_resolve_overloaded_builtin (location_t loc
 	  case TImode:
 	    {
 	      /* For scalar types just use a multiply expression.  */
-	      return fold_build2_loc (loc, MULT_EXPR, TREE_TYPE (arg0),
-					arg0, arg1);
+	      return fold_build2_loc (loc, MULT_EXPR, TREE_TYPE (arg0), arg0,
+				      fold_convert (TREE_TYPE (arg0), arg1));
 	    }
 	  case SFmode:
 	    {
@@ -5655,13 +5655,12 @@  altivec_resolve_overloaded_builtin (location_t loc
 	  || (TYPE_MODE (TREE_TYPE (arg0_type)) == SFmode)
 	  || (TYPE_MODE (TREE_TYPE (arg0_type)) == DFmode))
 	{
-	  /* Both arguments must be vectors and the types must match.  */
-	  if (arg0_type != arg1_type)
-	    goto bad;
+	  /* Both arguments must be vectors and the types must be compatible.  */
 	  if (TREE_CODE (arg0_type) != VECTOR_TYPE)
 	    goto bad;
+	  if (!lang_hooks.types_compatible_p (arg0_type, arg1_type))
+	    goto bad;
 
-
 	  switch (TYPE_MODE (TREE_TYPE (arg0_type)))
 	    {
 	      /* vec_cmpneq (va, vb) == vec_nor (vec_cmpeq (va, vb),
@@ -5720,11 +5719,12 @@  altivec_resolve_overloaded_builtin (location_t loc
       tree arg2_type = TREE_TYPE (arg2);
 
       /* All 3 arguments must be vectors of (signed or unsigned) (int or
-	  __int128) and the types must match.  */
-      if ((arg0_type != arg1_type) || (arg1_type != arg2_type))
-	goto bad;
+	 __int128) and the types must be compatible.  */
       if (TREE_CODE (arg0_type) != VECTOR_TYPE)
 	goto bad;
+      if (!lang_hooks.types_compatible_p (arg0_type, arg1_type) ||
+	  !lang_hooks.types_compatible_p (arg1_type, arg2_type))
+	goto bad;
 
       switch (TYPE_MODE (TREE_TYPE (arg0_type)))
 	{
@@ -5783,11 +5783,12 @@  altivec_resolve_overloaded_builtin (location_t loc
       tree arg2_type = TREE_TYPE (arg2);
 
       /* All 3 arguments must be vectors of (signed or unsigned) (int or
-	__int128) and the types must match.  */
-      if (arg0_type != arg1_type || arg1_type != arg2_type)
-	goto bad;
+	 __int128) and the types must be compatible.  */
       if (TREE_CODE (arg0_type) != VECTOR_TYPE)
 	goto bad;
+      if (!lang_hooks.types_compatible_p (arg0_type, arg1_type) ||
+	  !lang_hooks.types_compatible_p (arg1_type, arg2_type))
+	goto bad;
 
       switch (TYPE_MODE (TREE_TYPE (arg0_type)))
 	{
Index: gcc/testsuite/gcc.target/powerpc/vec-constvolatile.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/vec-constvolatile.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/vec-constvolatile.c	(working copy)
@@ -0,0 +1,31 @@ 
+/* Test that const and volatile qualifiers can mix on vec_mul operands.  */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-maltivec -mvsx" } */
+
+#include <altivec.h>
+
+void P() {
+  const volatile vector float cvva = vec_splats(0.00187682f);
+  volatile vector float vva = vec_splats(0.00187682f);
+  const vector float cva = vec_splats(0.00187682f);
+  vector float va = vec_splats(0.00187682f);
+  vector float dx = {1.0f, 2.0f, 3.0f, 4.0f};
+
+  vector float X1m0 = vec_mul(va, va);
+  vector float X2m0 = vec_mul(va, dx);
+  vector float X3m0 = vec_mul(dx, va);
+
+  vector float X1m1 = vec_mul(cva, cva);
+  vector float X2m1 = vec_mul(cva, dx);
+  vector float X3m1 = vec_mul(dx, cva);
+
+  vector float Y1m2 = vec_mul(vva, vva);
+  vector float Y2m2 = vec_mul(vva, dx);
+  vector float Y3m2 = vec_mul(dx, vva);
+
+  vector float X1m3 = vec_mul(cvva, cvva);
+  vector float X2m3 = vec_mul(cvva, dx);
+  vector float X3m3 = vec_mul(dx, cvva);
+}