diff mbox

Work around sizetype issues in CCP (PR tree-optimization/47538)

Message ID 20110131145256.GC30899@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Jan. 31, 2011, 2:52 p.m. UTC 
Hi!

The middle-end considers conversions between unsigned size_t sized
types and sizetype as useless, so my recent fix in bit_value_binop_1
causes the following testcase to regress.  Making sizetype conversions not
useless or making it work more sanely (not saying TYPE_UNSIGNED when it
is actually signed) unfortunately breaks too much and is not appropriate
for stage4, so this patch basically reverts that check in, and just looks
at operand type for comparisons.  In addition to that if it sees sign
mismatches between lhs and rhs1 for right shifts or rhs1 and rhs2 for
comparisons, it plays safe and doesn't try to optimize.

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

2011-01-31  Jakub Jelinek  <jakub@redhat.com>
	    Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/47538
	* tree-ssa-ccp.c (bit_value_binop_1): For uns computation use
	type instead of r1type, except for comparisons.  For right
	shifts and comparisons punt if there are mismatches in
	sizetype vs. non-sizetype types.

	* gcc.c-torture/execute/pr47538.c: New test.


	Jakub

Comments

Richard Biener Feb. 1, 2011, 10:24 a.m. UTC | #1 
On Mon, Jan 31, 2011 at 3:52 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The middle-end considers conversions between unsigned size_t sized
> types and sizetype as useless, so my recent fix in bit_value_binop_1
> causes the following testcase to regress.  Making sizetype conversions not
> useless or making it work more sanely (not saying TYPE_UNSIGNED when it
> is actually signed) unfortunately breaks too much and is not appropriate
> for stage4, so this patch basically reverts that check in, and just looks
> at operand type for comparisons.  In addition to that if it sees sign
> mismatches between lhs and rhs1 for right shifts or rhs1 and rhs2 for
> comparisons, it plays safe and doesn't try to optimize.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2011-01-31  Jakub Jelinek  <jakub@redhat.com>
>            Richard Guenther  <rguenther@suse.de>
>
>        PR tree-optimization/47538
>        * tree-ssa-ccp.c (bit_value_binop_1): For uns computation use
>        type instead of r1type, except for comparisons.  For right
>        shifts and comparisons punt if there are mismatches in
>        sizetype vs. non-sizetype types.
>
>        * gcc.c-torture/execute/pr47538.c: New test.
>
> --- gcc/tree-ssa-ccp.c.jj       2011-01-31 11:46:43.000000000 +0100
> +++ gcc/tree-ssa-ccp.c  2011-01-31 12:21:32.759546325 +0100
> @@ -1768,8 +1768,8 @@ bit_value_binop_1 (enum tree_code code,
>                   tree r1type, double_int r1val, double_int r1mask,
>                   tree r2type, double_int r2val, double_int r2mask)
>  {
> -  bool uns = (TREE_CODE (r1type) == INTEGER_TYPE
> -             && TYPE_IS_SIZETYPE (r1type) ? 0 : TYPE_UNSIGNED (r1type));
> +  bool uns = (TREE_CODE (type) == INTEGER_TYPE
> +             && TYPE_IS_SIZETYPE (type) ? 0 : TYPE_UNSIGNED (type));
>   /* Assume we'll get a constant result.  Use an initial varying value,
>      we fall back to varying in the end if necessary.  */
>   *mask = double_int_minus_one;
> @@ -1836,6 +1836,13 @@ bit_value_binop_1 (enum tree_code code,
>            }
>          else if (shift < 0)
>            {
> +             /* ???  We can have sizetype related inconsistencies in
> +                the IL.  */
> +             if ((TREE_CODE (r1type) == INTEGER_TYPE
> +                  && (TYPE_IS_SIZETYPE (r1type)
> +                      ? 0 : TYPE_UNSIGNED (r1type))) != uns)
> +               break;
> +
>              shift = -shift;
>              *mask = double_int_rshift (r1mask, shift,
>                                         TYPE_PRECISION (type), !uns);
> @@ -1946,6 +1953,14 @@ bit_value_binop_1 (enum tree_code code,
>        if (double_int_negative_p (r1mask) || double_int_negative_p (r2mask))
>          break;
>
> +       /* For comparisons the signedness is in the comparison operands.  */
> +       uns = (TREE_CODE (r1type) == INTEGER_TYPE
> +              && TYPE_IS_SIZETYPE (r1type) ? 0 : TYPE_UNSIGNED (r1type));
> +       /* ???  We can have sizetype related inconsistencies in the IL.  */
> +       if ((TREE_CODE (r2type) == INTEGER_TYPE
> +            && TYPE_IS_SIZETYPE (r2type) ? 0 : TYPE_UNSIGNED (r2type)) != uns)
> +         break;
> +
>        /* If we know the most significant bits we know the values
>           value ranges by means of treating varying bits as zero
>           or one.  Do a cross comparison of the max/min pairs.  */
> --- gcc/testsuite/gcc.c-torture/execute/pr47538.c.jj    2011-01-31 11:50:24.275638525 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr47538.c       2011-01-31 11:50:24.275638525 +0100
> @@ -0,0 +1,73 @@
> +/* PR tree-optimization/47538 */
> +
> +struct S
> +{
> +  double a, b, *c;
> +  unsigned long d;
> +};
> +
> +__attribute__((noinline, noclone)) void
> +foo (struct S *x, const struct S *y)
> +{
> +  const unsigned long n = y->d + 1;
> +  const double m = 0.25 * (y->b - y->a);
> +  x->a = y->a;
> +  x->b = y->b;
> +  if (n == 1)
> +    {
> +      x->c[0] = 0.;
> +    }
> +  else if (n == 2)
> +    {
> +      x->c[1] = m * y->c[0];
> +      x->c[0] = 2.0 * x->c[1];
> +    }
> +  else
> +    {
> +      double o = 0.0, p = 1.0;
> +      unsigned long i;
> +
> +      for (i = 1; i <= n - 2; i++)
> +       {
> +         x->c[i] = m * (y->c[i - 1] - y->c[i + 1]) / (double) i;
> +         o += p * x->c[i];
> +         p = -p;
> +       }
> +      x->c[n - 1] = m * y->c[n - 2] / (n - 1.0);
> +      o += p * x->c[n - 1];
> +      x->c[0] = 2.0 * o;
> +    }
> +}
> +
> +int
> +main (void)
> +{
> +  struct S x, y;
> +  double c[4] = { 10, 20, 30, 40 }, d[4], e[4] = { 118, 118, 118, 118 };
> +
> +  y.a = 10;
> +  y.b = 6;
> +  y.c = c;
> +  x.c = d;
> +  y.d = 3;
> +  __builtin_memcpy (d, e, sizeof d);
> +  foo (&x, &y);
> +  if (d[0] != 0 || d[1] != 20 || d[2] != 10 || d[3] != -10)
> +    __builtin_abort ();
> +  y.d = 2;
> +  __builtin_memcpy (d, e, sizeof d);
> +  foo (&x, &y);
> +  if (d[0] != 60 || d[1] != 20 || d[2] != -10 || d[3] != 118)
> +    __builtin_abort ();
> +  y.d = 1;
> +  __builtin_memcpy (d, e, sizeof d);
> +  foo (&x, &y);
> +  if (d[0] != -20 || d[1] != -10 || d[2] != 118 || d[3] != 118)
> +    __builtin_abort ();
> +  y.d = 0;
> +  __builtin_memcpy (d, e, sizeof d);
> +  foo (&x, &y);
> +  if (d[0] != 0 || d[1] != 118 || d[2] != 118 || d[3] != 118)
> +    __builtin_abort ();
> +  return 0;
> +}
>
>        Jakub
>
diff mbox

Patch

--- gcc/tree-ssa-ccp.c.jj	2011-01-31 11:46:43.000000000 +0100
+++ gcc/tree-ssa-ccp.c	2011-01-31 12:21:32.759546325 +0100
@@ -1768,8 +1768,8 @@  bit_value_binop_1 (enum tree_code code, 
 		   tree r1type, double_int r1val, double_int r1mask,
 		   tree r2type, double_int r2val, double_int r2mask)
 {
-  bool uns = (TREE_CODE (r1type) == INTEGER_TYPE
-	      && TYPE_IS_SIZETYPE (r1type) ? 0 : TYPE_UNSIGNED (r1type));
+  bool uns = (TREE_CODE (type) == INTEGER_TYPE
+	      && TYPE_IS_SIZETYPE (type) ? 0 : TYPE_UNSIGNED (type));
   /* Assume we'll get a constant result.  Use an initial varying value,
      we fall back to varying in the end if necessary.  */
   *mask = double_int_minus_one;
@@ -1836,6 +1836,13 @@  bit_value_binop_1 (enum tree_code code, 
 	    }
 	  else if (shift < 0)
 	    {
+	      /* ???  We can have sizetype related inconsistencies in
+		 the IL.  */
+	      if ((TREE_CODE (r1type) == INTEGER_TYPE
+		   && (TYPE_IS_SIZETYPE (r1type)
+		       ? 0 : TYPE_UNSIGNED (r1type))) != uns)
+		break;
+
 	      shift = -shift;
 	      *mask = double_int_rshift (r1mask, shift,
 					 TYPE_PRECISION (type), !uns);
@@ -1946,6 +1953,14 @@  bit_value_binop_1 (enum tree_code code, 
 	if (double_int_negative_p (r1mask) || double_int_negative_p (r2mask))
 	  break;
 
+	/* For comparisons the signedness is in the comparison operands.  */
+	uns = (TREE_CODE (r1type) == INTEGER_TYPE
+	       && TYPE_IS_SIZETYPE (r1type) ? 0 : TYPE_UNSIGNED (r1type));
+	/* ???  We can have sizetype related inconsistencies in the IL.  */
+	if ((TREE_CODE (r2type) == INTEGER_TYPE
+	     && TYPE_IS_SIZETYPE (r2type) ? 0 : TYPE_UNSIGNED (r2type)) != uns)
+	  break;
+
 	/* If we know the most significant bits we know the values
 	   value ranges by means of treating varying bits as zero
 	   or one.  Do a cross comparison of the max/min pairs.  */
--- gcc/testsuite/gcc.c-torture/execute/pr47538.c.jj	2011-01-31 11:50:24.275638525 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr47538.c	2011-01-31 11:50:24.275638525 +0100
@@ -0,0 +1,73 @@ 
+/* PR tree-optimization/47538 */
+
+struct S
+{
+  double a, b, *c;
+  unsigned long d;
+};
+
+__attribute__((noinline, noclone)) void
+foo (struct S *x, const struct S *y)
+{
+  const unsigned long n = y->d + 1;
+  const double m = 0.25 * (y->b - y->a);
+  x->a = y->a;
+  x->b = y->b;
+  if (n == 1)
+    {
+      x->c[0] = 0.;
+    }
+  else if (n == 2)
+    {
+      x->c[1] = m * y->c[0];
+      x->c[0] = 2.0 * x->c[1];
+    }
+  else
+    {
+      double o = 0.0, p = 1.0;
+      unsigned long i;
+
+      for (i = 1; i <= n - 2; i++)
+	{
+	  x->c[i] = m * (y->c[i - 1] - y->c[i + 1]) / (double) i;
+	  o += p * x->c[i];
+	  p = -p;
+	}
+      x->c[n - 1] = m * y->c[n - 2] / (n - 1.0);
+      o += p * x->c[n - 1];
+      x->c[0] = 2.0 * o;
+    }
+}
+
+int
+main (void)
+{
+  struct S x, y;
+  double c[4] = { 10, 20, 30, 40 }, d[4], e[4] = { 118, 118, 118, 118 };
+
+  y.a = 10;
+  y.b = 6;
+  y.c = c;
+  x.c = d;
+  y.d = 3;
+  __builtin_memcpy (d, e, sizeof d);
+  foo (&x, &y);
+  if (d[0] != 0 || d[1] != 20 || d[2] != 10 || d[3] != -10)
+    __builtin_abort ();
+  y.d = 2;
+  __builtin_memcpy (d, e, sizeof d);
+  foo (&x, &y);
+  if (d[0] != 60 || d[1] != 20 || d[2] != -10 || d[3] != 118)
+    __builtin_abort ();
+  y.d = 1;
+  __builtin_memcpy (d, e, sizeof d);
+  foo (&x, &y);
+  if (d[0] != -20 || d[1] != -10 || d[2] != 118 || d[3] != 118)
+    __builtin_abort ();
+  y.d = 0;
+  __builtin_memcpy (d, e, sizeof d);
+  foo (&x, &y);
+  if (d[0] != 0 || d[1] != 118 || d[2] != 118 || d[3] != 118)
+    __builtin_abort ();
+  return 0;
+}