diff mbox series

Fix vector handling in simplify-rtx.c (PR rtl-optimization/82973)

Message ID 20171220221850.GR2353@tucnak
State New
Headers show
Series Fix vector handling in simplify-rtx.c (PR rtl-optimization/82973) | expand

Commit Message

Jakub Jelinek Dec. 20, 2017, 10:18 p.m. UTC 
Hi!

In rtl.texi we say:
@findex const_vector
@item (const_vector:@var{m} [@var{x0} @var{x1} @dots{}])
Represents a vector constant.  The square brackets stand for the vector
containing the constant elements.  @var{x0}, @var{x1} and so on are
the @code{const_int}, @code{const_wide_int}, @code{const_double} or
@code{const_fixed} elements.
and it is a very reasonable requirement. 
simplify_const_{unary,binary}_operation can violate that though, because
the recursion can return also other expressions, like in this case
a (mult (const_double) (const_double)) that can't be simplified because
of -frounding-math.  We need to punt on those.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-12-20  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/82973
	* simplify-rtx.c (simplify_const_unary_operation): Don't optimize into
	CONST_VECTOR if some element isn't simplified into CONST_{,WIDE_}INT,
	CONST_DOUBLE or CONST_FIXED.
	(simplify_const_binary_operation): Likewise.  Use CONST_FIXED_P macro
	instead of GET_CODE == CONST_FIXED.
	(simplify_subreg): Use CONST_FIXED_P macro instead of
	GET_CODE == CONST_FIXED.

	* gfortran.dg/pr82973.f90: New test.


	Jakub

Comments

Richard Sandiford Dec. 21, 2017, 12:12 a.m. UTC | #1 
Jakub Jelinek <jakub@redhat.com> writes:
> Hi!
>
> In rtl.texi we say:
> @findex const_vector
> @item (const_vector:@var{m} [@var{x0} @var{x1} @dots{}])
> Represents a vector constant.  The square brackets stand for the vector
> containing the constant elements.  @var{x0}, @var{x1} and so on are
> the @code{const_int}, @code{const_wide_int}, @code{const_double} or
> @code{const_fixed} elements.
> and it is a very reasonable requirement. 
> simplify_const_{unary,binary}_operation can violate that though, because
> the recursion can return also other expressions, like in this case
> a (mult (const_double) (const_double)) that can't be simplified because
> of -frounding-math.  We need to punt on those.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

How about renaming valid_for_const_vec_duplicate_p to something more
generic and using it here too?  The requirements should be the same.

Thanks,
Richard
Jakub Jelinek Dec. 21, 2017, 12:16 a.m. UTC | #2 
On Thu, Dec 21, 2017 at 12:12:14AM +0000, Richard Sandiford wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> > Hi!
> >
> > In rtl.texi we say:
> > @findex const_vector
> > @item (const_vector:@var{m} [@var{x0} @var{x1} @dots{}])
> > Represents a vector constant.  The square brackets stand for the vector
> > containing the constant elements.  @var{x0}, @var{x1} and so on are
> > the @code{const_int}, @code{const_wide_int}, @code{const_double} or
> > @code{const_fixed} elements.
> > and it is a very reasonable requirement. 
> > simplify_const_{unary,binary}_operation can violate that though, because
> > the recursion can return also other expressions, like in this case
> > a (mult (const_double) (const_double)) that can't be simplified because
> > of -frounding-math.  We need to punt on those.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> How about renaming valid_for_const_vec_duplicate_p to something more
> generic and using it here too?  The requirements should be the same.

As it generates CONST_VECTOR, renaming it to valid_for_const_vector_p
and using it also in simplify-rtx.c looks reasonable to me.

	Jakub
Jeff Law Dec. 21, 2017, 5:28 a.m. UTC | #3 
On 12/20/2017 03:18 PM, Jakub Jelinek wrote:
> Hi!
> 
> In rtl.texi we say:
> @findex const_vector
> @item (const_vector:@var{m} [@var{x0} @var{x1} @dots{}])
> Represents a vector constant.  The square brackets stand for the vector
> containing the constant elements.  @var{x0}, @var{x1} and so on are
> the @code{const_int}, @code{const_wide_int}, @code{const_double} or
> @code{const_fixed} elements.
> and it is a very reasonable requirement. 
> simplify_const_{unary,binary}_operation can violate that though, because
> the recursion can return also other expressions, like in this case
> a (mult (const_double) (const_double)) that can't be simplified because
> of -frounding-math.  We need to punt on those.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2017-12-20  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR rtl-optimization/82973
> 	* simplify-rtx.c (simplify_const_unary_operation): Don't optimize into
> 	CONST_VECTOR if some element isn't simplified into CONST_{,WIDE_}INT,
> 	CONST_DOUBLE or CONST_FIXED.
> 	(simplify_const_binary_operation): Likewise.  Use CONST_FIXED_P macro
> 	instead of GET_CODE == CONST_FIXED.
> 	(simplify_subreg): Use CONST_FIXED_P macro instead of
> 	GET_CODE == CONST_FIXED.
> 
> 	* gfortran.dg/pr82973.f90: New test.
OK.
jeff
Jakub Jelinek Dec. 21, 2017, 9:13 a.m. UTC | #4 
On Wed, Dec 20, 2017 at 10:28:53PM -0700, Jeff Law wrote:
> On 12/20/2017 03:18 PM, Jakub Jelinek wrote:
> > In rtl.texi we say:
> > @findex const_vector
> > @item (const_vector:@var{m} [@var{x0} @var{x1} @dots{}])
> > Represents a vector constant.  The square brackets stand for the vector
> > containing the constant elements.  @var{x0}, @var{x1} and so on are
> > the @code{const_int}, @code{const_wide_int}, @code{const_double} or
> > @code{const_fixed} elements.
> > and it is a very reasonable requirement. 
> > simplify_const_{unary,binary}_operation can violate that though, because
> > the recursion can return also other expressions, like in this case
> > a (mult (const_double) (const_double)) that can't be simplified because
> > of -frounding-math.  We need to punt on those.
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > 
> > 2017-12-20  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR rtl-optimization/82973
> > 	* simplify-rtx.c (simplify_const_unary_operation): Don't optimize into
> > 	CONST_VECTOR if some element isn't simplified into CONST_{,WIDE_}INT,
> > 	CONST_DOUBLE or CONST_FIXED.
> > 	(simplify_const_binary_operation): Likewise.  Use CONST_FIXED_P macro
> > 	instead of GET_CODE == CONST_FIXED.
> > 	(simplify_subreg): Use CONST_FIXED_P macro instead of
> > 	GET_CODE == CONST_FIXED.
> > 
> > 	* gfortran.dg/pr82973.f90: New test.
> OK.

Richard Sandiford suggested using a renamed
vector_for_vec_constant_duplicate_p predicate instead, so here is what I've
committed instead:

2017-12-21  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/82973
	* emit-rtl.h (valid_for_const_vec_duplicate_p): Rename to ...
	(valid_for_const_vector_p): ... this.
	* emit-rtl.c (valid_for_const_vec_duplicate_p): Rename to ...
	(valid_for_const_vector_p): ... this.  Adjust function comment.
	(gen_vec_duplicate): Adjust caller.
	* optabs.c (expand_vector_broadcast): Likewise.
	* simplify-rtx.c (simplify_const_unary_operation): Don't optimize into
	CONST_VECTOR if some element isn't simplified valid_for_const_vector_p
	constant.
	(simplify_const_binary_operation): Likewise.  Use CONST_FIXED_P macro
	instead of GET_CODE == CONST_FIXED.
	(simplify_subreg): Use CONST_FIXED_P macro instead of
	GET_CODE == CONST_FIXED.

	* gfortran.dg/pr82973.f90: New test.

--- gcc/emit-rtl.h.jj	2017-12-21 09:43:24.000000000 +0100
+++ gcc/emit-rtl.h	2017-12-21 09:59:39.422636912 +0100
@@ -439,7 +439,7 @@ get_max_uid (void)
   return crtl->emit.x_cur_insn_uid;
 }
 
-extern bool valid_for_const_vec_duplicate_p (machine_mode, rtx);
+extern bool valid_for_const_vector_p (machine_mode, rtx);
 extern rtx gen_const_vec_duplicate (machine_mode, rtx);
 extern rtx gen_vec_duplicate (machine_mode, rtx);
 
--- gcc/emit-rtl.c.jj	2017-12-21 09:43:24.000000000 +0100
+++ gcc/emit-rtl.c	2017-12-21 10:01:22.278333211 +0100
@@ -5861,11 +5861,11 @@ init_emit (void)
 #endif
 }
 
-/* Return true if X is a valid element for a duplicated vector constant
-   of the given mode.  */
+/* Return true if X is a valid element for a CONST_VECTOR of the given
+  mode.  */
 
 bool
-valid_for_const_vec_duplicate_p (machine_mode, rtx x)
+valid_for_const_vector_p (machine_mode, rtx x)
 {
   return (CONST_SCALAR_INT_P (x)
 	  || CONST_DOUBLE_AS_FLOAT_P (x)
@@ -5907,7 +5907,7 @@ gen_const_vec_duplicate (machine_mode mo
 rtx
 gen_vec_duplicate (machine_mode mode, rtx x)
 {
-  if (valid_for_const_vec_duplicate_p (mode, x))
+  if (valid_for_const_vector_p (mode, x))
     return gen_const_vec_duplicate (mode, x);
   return gen_rtx_VEC_DUPLICATE (mode, x);
 }
--- gcc/optabs.c.jj	2017-12-20 20:40:12.000000000 +0100
+++ gcc/optabs.c	2017-12-21 10:01:43.719061449 +0100
@@ -377,7 +377,7 @@ expand_vector_broadcast (machine_mode vm
 
   gcc_checking_assert (VECTOR_MODE_P (vmode));
 
-  if (valid_for_const_vec_duplicate_p (vmode, op))
+  if (valid_for_const_vector_p (vmode, op))
     return gen_const_vec_duplicate (vmode, op);
 
   icode = optab_handler (vec_duplicate_optab, vmode);
--- gcc/simplify-rtx.c.jj	2017-12-21 09:43:24.630981321 +0100
+++ gcc/simplify-rtx.c	2017-12-21 10:03:39.542593616 +0100
@@ -1768,7 +1768,7 @@ simplify_const_unary_operation (enum rtx
 	  rtx x = simplify_unary_operation (code, GET_MODE_INNER (mode),
 					    CONST_VECTOR_ELT (op, i),
 					    GET_MODE_INNER (opmode));
-	  if (!x)
+	  if (!x || !valid_for_const_vector_p (mode, x))
 	    return 0;
 	  RTVEC_ELT (v, i) = x;
 	}
@@ -4030,7 +4030,7 @@ simplify_const_binary_operation (enum rt
 	  rtx x = simplify_binary_operation (code, GET_MODE_INNER (mode),
 					     CONST_VECTOR_ELT (op0, i),
 					     CONST_VECTOR_ELT (op1, i));
-	  if (!x)
+	  if (!x || !valid_for_const_vector_p (mode, x))
 	    return 0;
 	  RTVEC_ELT (v, i) = x;
 	}
@@ -4041,11 +4041,11 @@ simplify_const_binary_operation (enum rt
   if (VECTOR_MODE_P (mode)
       && code == VEC_CONCAT
       && (CONST_SCALAR_INT_P (op0)
-	  || GET_CODE (op0) == CONST_FIXED
+	  || CONST_FIXED_P (op0)
 	  || CONST_DOUBLE_AS_FLOAT_P (op0))
       && (CONST_SCALAR_INT_P (op1)
 	  || CONST_DOUBLE_AS_FLOAT_P (op1)
-	  || GET_CODE (op1) == CONST_FIXED))
+	  || CONST_FIXED_P (op1)))
     {
       unsigned n_elts = GET_MODE_NUNITS (mode);
       rtvec v = rtvec_alloc (n_elts);
@@ -6268,7 +6268,7 @@ simplify_subreg (machine_mode outermode,
 
   if (CONST_SCALAR_INT_P (op)
       || CONST_DOUBLE_AS_FLOAT_P (op)
-      || GET_CODE (op) == CONST_FIXED
+      || CONST_FIXED_P (op)
       || GET_CODE (op) == CONST_VECTOR)
     {
       /* simplify_immed_subreg deconstructs OP into bytes and constructs
--- gcc/testsuite/gfortran.dg/pr82973.f90.jj	2017-12-21 09:58:50.168261214 +0100
+++ gcc/testsuite/gfortran.dg/pr82973.f90	2017-12-21 09:58:50.168261214 +0100
@@ -0,0 +1,31 @@
+! PR rtl-optimization/82973
+! { dg-do compile }
+! { dg-options "-Ofast -frounding-math" }
+
+program pr82973
+  integer, parameter :: n=16
+  real, dimension(n) :: ar, br, modulo_result, floor_result
+  integer, dimension(n) :: ai, bi , imodulo_result, ifloor_result
+  ai(1:4) = 5
+  ai(5:8) = -5
+  ai(9:12) = 1
+  ai(13:16) = -1
+  bi(1:4) = (/ 3,-3, 1, -1/)
+  bi(5:8) = bi(1:4)
+  bi(9:12) = bi(1:4)
+  bi(13:16) = bi(1:4)
+  ar = ai
+  br = bi
+  modulo_result = modulo(ar,br)
+  imodulo_result = modulo(ai,bi)
+  floor_result = ar-floor(ar/br)*br
+  ifloor_result = nint(real(ai-floor(real(ai)/real(bi))*bi))
+  do i=1,n
+    if (modulo_result(i) /= floor_result(i)) then
+      call abort()
+    end if
+    if (imodulo_result(i) /= ifloor_result(i)) then
+      call abort ()
+    end if
+  end do
+end program pr82973


	Jakub
diff mbox series

Patch

--- gcc/simplify-rtx.c.jj	2017-12-19 18:09:05.000000000 +0100
+++ gcc/simplify-rtx.c	2017-12-20 18:29:55.190089315 +0100
@@ -1776,7 +1776,12 @@  simplify_const_unary_operation (enum rtx
 	  rtx x = simplify_unary_operation (code, GET_MODE_INNER (mode),
 					    CONST_VECTOR_ELT (op, i),
 					    GET_MODE_INNER (opmode));
-	  if (!x)
+	  /* Only CONST_INT, CONST_WIDE_INT, CONST_DOUBLE or CONST_FIXED
+	     is valid inside of CONST_VECTOR.  */
+	  if (!x
+	      || !(CONST_SCALAR_INT_P (x)
+		   || CONST_DOUBLE_AS_FLOAT_P (x)
+		   || CONST_FIXED_P (x)))
 	    return 0;
 	  RTVEC_ELT (v, i) = x;
 	}
@@ -4006,7 +4011,12 @@  simplify_const_binary_operation (enum rt
 	  rtx x = simplify_binary_operation (code, GET_MODE_INNER (mode),
 					     CONST_VECTOR_ELT (op0, i),
 					     CONST_VECTOR_ELT (op1, i));
-	  if (!x)
+	  /* Only CONST_INT, CONST_WIDE_INT, CONST_DOUBLE or CONST_FIXED
+	     is valid inside of CONST_VECTOR.  */
+	  if (!x
+	      || !(CONST_SCALAR_INT_P (x)
+		   || CONST_DOUBLE_AS_FLOAT_P (x)
+		   || CONST_FIXED_P (x)))
 	    return 0;
 	  RTVEC_ELT (v, i) = x;
 	}
@@ -4017,11 +4027,11 @@  simplify_const_binary_operation (enum rt
   if (VECTOR_MODE_P (mode)
       && code == VEC_CONCAT
       && (CONST_SCALAR_INT_P (op0)
-	  || GET_CODE (op0) == CONST_FIXED
+	  || CONST_FIXED_P (op0)
 	  || CONST_DOUBLE_AS_FLOAT_P (op0))
       && (CONST_SCALAR_INT_P (op1)
 	  || CONST_DOUBLE_AS_FLOAT_P (op1)
-	  || GET_CODE (op1) == CONST_FIXED))
+	  || CONST_FIXED_P (op1)))
     {
       unsigned n_elts = GET_MODE_NUNITS (mode);
       rtvec v = rtvec_alloc (n_elts);
@@ -6193,7 +6203,7 @@  simplify_subreg (machine_mode outermode,
 
   if (CONST_SCALAR_INT_P (op)
       || CONST_DOUBLE_AS_FLOAT_P (op)
-      || GET_CODE (op) == CONST_FIXED
+      || CONST_FIXED_P (op)
       || GET_CODE (op) == CONST_VECTOR)
     {
       /* simplify_immed_subreg deconstructs OP into bytes and constructs
--- gcc/testsuite/gfortran.dg/pr82973.f90.jj	2017-12-20 18:53:14.392540473 +0100
+++ gcc/testsuite/gfortran.dg/pr82973.f90	2017-12-20 18:52:47.000000000 +0100
@@ -0,0 +1,31 @@ 
+! PR rtl-optimization/82973
+! { dg-do compile }
+! { dg-options "-Ofast -frounding-math" }
+
+program pr82973
+  integer, parameter :: n=16
+  real, dimension(n) :: ar, br, modulo_result, floor_result
+  integer, dimension(n) :: ai, bi , imodulo_result, ifloor_result
+  ai(1:4) = 5
+  ai(5:8) = -5
+  ai(9:12) = 1
+  ai(13:16) = -1
+  bi(1:4) = (/ 3,-3, 1, -1/)
+  bi(5:8) = bi(1:4)
+  bi(9:12) = bi(1:4)
+  bi(13:16) = bi(1:4)
+  ar = ai
+  br = bi
+  modulo_result = modulo(ar,br)
+  imodulo_result = modulo(ai,bi)
+  floor_result = ar-floor(ar/br)*br
+  ifloor_result = nint(real(ai-floor(real(ai)/real(bi))*bi))
+  do i=1,n
+    if (modulo_result(i) /= floor_result(i)) then
+      call abort()
+    end if
+    if (imodulo_result(i) /= ifloor_result(i)) then
+      call abort ()
+    end if
+  end do
+end program pr82973