Patchwork Fix two bugs in gimple-fold.c and/or folding (PR tree-optimization/46909)

login
register
mail settings
Submitter Jakub Jelinek
Date Dec. 14, 2010, 9:45 a.m.
Message ID <20101214094528.GE27214@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/75478/
State New
Headers show

Comments

Jakub Jelinek - Dec. 14, 2010, 9:45 a.m.
Hi!

This patch fixes two bugs in or_var_with_comparison_1
- one is that bool BIT_IOR_EXPR was mistakenly handled as TRUTH_AND_EXPR
  instead of TRUTH_OR_EXPR
- another one is that it tried to optimize (x AND x) for arbitrary x
  into 1 instead of x
and makes it optimize even when both partial results are the same in the
and/and and or/or case.

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

2010-12-14  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/46909
	* gimple-fold.c (and_var_with_comparison_1): Save partial
	result even in the is_and case, if both partial results
	are the same, return it.
	(or_var_with_comparison_1): Use is_or predicate instead of
	innercode == TRUTH_OR_EXPR test.  Save partial result
	even in the is_or case, if both partial results are the
	same, return it.  In the !is_or case when both partial
	results are the same, return the partial result instead
	of boolean_true_node.

	* gcc.c-torture/execute/pr46909-1.c: New test.
	* gcc.c-torture/execute/pr46909-2.c: New test.
	* gcc.dg/pr46909.c: New test.


	Jakub
Ramana Radhakrishnan - Dec. 14, 2010, 12:15 p.m.
On Tue, 2010-12-14 at 10:45 +0100, Jakub Jelinek wrote:
> Hi!
> 
> This patch fixes two bugs in or_var_with_comparison_1
> - one is that bool BIT_IOR_EXPR was mistakenly handled as TRUTH_AND_EXPR
>   instead of TRUTH_OR_EXPR
> - another one is that it tried to optimize (x AND x) for arbitrary x
>   into 1 instead of x
> and makes it optimize even when both partial results are the same in the
> and/and and or/or case.

Coincidentally I was looking at PR46693 last night and stumbled upon the
same problem in or_var_with_comparison_1 and I didn't manage to finish
fixing it up.

Your patch appears to fix that testcase on trunk though today I cannot
replicate PR46693 using the RC that I had lying around.

cheers
Ramana
 





> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2010-12-14  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/46909
> 	* gimple-fold.c (and_var_with_comparison_1): Save partial
> 	result even in the is_and case, if both partial results
> 	are the same, return it.
> 	(or_var_with_comparison_1): Use is_or predicate instead of
> 	innercode == TRUTH_OR_EXPR test.  Save partial result
> 	even in the is_or case, if both partial results are the
> 	same, return it.  In the !is_or case when both partial
> 	results are the same, return the partial result instead
> 	of boolean_true_node.
> 
> 	* gcc.c-torture/execute/pr46909-1.c: New test.
> 	* gcc.c-torture/execute/pr46909-2.c: New test.
> 	* gcc.dg/pr46909.c: New test.
> 
> --- gcc/gimple-fold.c.jj	2010-11-29 13:27:43.000000000 +0100
> +++ gcc/gimple-fold.c	2010-12-13 14:57:12.000000000 +0100
> @@ -2004,14 +2004,11 @@ and_var_with_comparison_1 (gimple stmt,
>  	  /* Handle the OR case, where we are redistributing:
>  	     (inner1 OR inner2) AND (op2a code2 op2b)
>  	     => (t OR (inner2 AND (op2a code2 op2b)))  */
> -	  else
> -	    {
> -	      if (integer_onep (t))
> -		return boolean_true_node;
> -	      else
> -		/* Save partial result for later.  */
> -		partial = t;
> -	    }
> +	  else if (integer_onep (t))
> +	    return boolean_true_node;
> +
> +	  /* Save partial result for later.  */
> +	  partial = t;
>  	}
>        
>        /* Compute the second partial result, (inner2 AND (op2a code op2b)) */
> @@ -2032,6 +2029,10 @@ and_var_with_comparison_1 (gimple stmt,
>  		return inner1;
>  	      else if (integer_zerop (t))
>  		return boolean_false_node;
> +	      /* If both are the same, we can apply the identity
> +		 (x AND x) == x.  */
> +	      else if (partial && same_bool_result_p (t, partial))
> +		return t;
>  	    }
>  
>  	  /* Handle the OR case. where we are redistributing:
> @@ -2441,7 +2442,7 @@ or_var_with_comparison_1 (gimple stmt,
>  	     => (t OR inner2)
>  	     If the partial result t is a constant, we win.  Otherwise
>  	     continue on to try reassociating with the other inner test.  */
> -	  if (innercode == TRUTH_OR_EXPR)
> +	  if (is_or)
>  	    {
>  	      if (integer_onep (t))
>  		return boolean_true_node;
> @@ -2452,14 +2453,11 @@ or_var_with_comparison_1 (gimple stmt,
>  	  /* Handle the AND case, where we are redistributing:
>  	     (inner1 AND inner2) OR (op2a code2 op2b)
>  	     => (t AND (inner2 OR (op2a code op2b)))  */
> -	  else
> -	    {
> -	      if (integer_zerop (t))
> -		return boolean_false_node;
> -	      else
> -		/* Save partial result for later.  */
> -		partial = t;
> -	    }
> +	  else if (integer_zerop (t))
> +	    return boolean_false_node;
> +
> +	  /* Save partial result for later.  */
> +	  partial = t;
>  	}
>        
>        /* Compute the second partial result, (inner2 OR (op2a code op2b)) */
> @@ -2473,13 +2471,18 @@ or_var_with_comparison_1 (gimple stmt,
>  	{
>  	  /* Handle the OR case, where we are reassociating:
>  	     (inner1 OR inner2) OR (op2a code2 op2b)
> -	     => (inner1 OR t)  */
> -	  if (innercode == TRUTH_OR_EXPR)
> +	     => (inner1 OR t)
> +	     => (t OR partial)  */
> +	  if (is_or)
>  	    {
>  	      if (integer_zerop (t))
>  		return inner1;
>  	      else if (integer_onep (t))
>  		return boolean_true_node;
> +	      /* If both are the same, we can apply the identity
> +		 (x OR x) == x.  */
> +	      else if (partial && same_bool_result_p (t, partial))
> +		return t;
>  	    }
>  	  
>  	  /* Handle the AND case, where we are redistributing:
> @@ -2496,13 +2499,13 @@ or_var_with_comparison_1 (gimple stmt,
>  		     operand to the redistributed AND expression.  The
>  		     interesting case is when at least one is true.
>  		     Or, if both are the same, we can apply the identity
> -		     (x AND x) == true.  */
> +		     (x AND x) == x.  */
>  		  if (integer_onep (partial))
>  		    return t;
>  		  else if (integer_onep (t))
>  		    return partial;
>  		  else if (same_bool_result_p (t, partial))
> -		    return boolean_true_node;
> +		    return t;
>  		}
>  	    }
>  	}
> --- gcc/testsuite/gcc.c-torture/execute/pr46909-1.c.jj	2010-12-13 15:06:13.000000000 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr46909-1.c	2010-12-13 15:08:03.000000000 +0100
> @@ -0,0 +1,22 @@
> +/* PR tree-optimization/46909 */
> +
> +extern void abort ();
> +
> +int
> +__attribute__ ((__noinline__))
> +foo (unsigned int x)
> +{
> +  if (! (x == 4 || x == 6) || (x == 2 || x == 6))
> +    return 1;
> +  return -1;
> +}
> +
> +int
> +main ()
> +{
> +  int i;
> +  for (i = -10; i < 10; i++)
> +    if (foo (i) != 1 - 2 * (i == 4))
> +      abort ();
> +  return 0;
> +}
> --- gcc/testsuite/gcc.c-torture/execute/pr46909-2.c.jj	2010-11-19 19:58:10.083000000 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr46909-2.c	2010-12-14 00:57:03.000000000 +0100
> @@ -0,0 +1,22 @@
> +/* PR tree-optimization/46909 */
> +
> +extern void abort (void);
> +
> +int
> +__attribute__((noinline))
> +foo (int x)
> +{
> +  if ((x != 0 && x != 13) || x == 5 || x == 20)
> +    return 1;
> +  return -1;
> +}
> +
> +int
> +main (void)
> +{
> +  int i;
> +  for (i = -10; i < 30; i++)
> +    if (foo (i) != 1 - 2 * (i == 0) - 2 * (i == 13))
> +      abort ();
> +  return 0;
> +}
> --- gcc/testsuite/gcc.dg/pr46909.c.jj	2010-12-13 15:07:31.000000000 +0100
> +++ gcc/testsuite/gcc.dg/pr46909.c	2010-12-13 15:11:30.000000000 +0100
> @@ -0,0 +1,17 @@
> +/* PR tree-optimization/46909 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-ifcombine" } */
> +
> +extern void abort ();
> +
> +int
> +__attribute__ ((__noinline__))
> +foo (unsigned int x)
> +{
> +  if (! (x == 4 || x == 6) || (x == 2 || x == 6))
> +    return 1;
> +  return -1;
> +}
> +
> +/* { dg-final { scan-tree-dump "optimizing two comparisons to x_\[0-9\]+\\(D\\) != 4" "ifcombine" } } */
> +/* { dg-final { cleanup-tree-dump "ifcombine" } } */
> 
> 	Jakub
Jeff Law - Dec. 14, 2010, 2:03 p.m.
On 12/14/10 02:45, Jakub Jelinek wrote:
> Hi!
>
> This patch fixes two bugs in or_var_with_comparison_1
> - one is that bool BIT_IOR_EXPR was mistakenly handled as TRUTH_AND_EXPR
>    instead of TRUTH_OR_EXPR
> - another one is that it tried to optimize (x AND x) for arbitrary x
>    into 1 instead of x
> and makes it optimize even when both partial results are the same in the
> and/and and or/or case.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2010-12-14  Jakub Jelinek<jakub@redhat.com>
>
> 	PR tree-optimization/46909
> 	* gimple-fold.c (and_var_with_comparison_1): Save partial
> 	result even in the is_and case, if both partial results
> 	are the same, return it.
> 	(or_var_with_comparison_1): Use is_or predicate instead of
> 	innercode == TRUTH_OR_EXPR test.  Save partial result
> 	even in the is_or case, if both partial results are the
> 	same, return it.  In the !is_or case when both partial
> 	results are the same, return the partial result instead
> 	of boolean_true_node.
>
> 	* gcc.c-torture/execute/pr46909-1.c: New test.
> 	* gcc.c-torture/execute/pr46909-2.c: New test.
> 	* gcc.dg/pr46909.c: New test.
OK.
jeff

Patch

--- gcc/gimple-fold.c.jj	2010-11-29 13:27:43.000000000 +0100
+++ gcc/gimple-fold.c	2010-12-13 14:57:12.000000000 +0100
@@ -2004,14 +2004,11 @@  and_var_with_comparison_1 (gimple stmt,
 	  /* Handle the OR case, where we are redistributing:
 	     (inner1 OR inner2) AND (op2a code2 op2b)
 	     => (t OR (inner2 AND (op2a code2 op2b)))  */
-	  else
-	    {
-	      if (integer_onep (t))
-		return boolean_true_node;
-	      else
-		/* Save partial result for later.  */
-		partial = t;
-	    }
+	  else if (integer_onep (t))
+	    return boolean_true_node;
+
+	  /* Save partial result for later.  */
+	  partial = t;
 	}
       
       /* Compute the second partial result, (inner2 AND (op2a code op2b)) */
@@ -2032,6 +2029,10 @@  and_var_with_comparison_1 (gimple stmt,
 		return inner1;
 	      else if (integer_zerop (t))
 		return boolean_false_node;
+	      /* If both are the same, we can apply the identity
+		 (x AND x) == x.  */
+	      else if (partial && same_bool_result_p (t, partial))
+		return t;
 	    }
 
 	  /* Handle the OR case. where we are redistributing:
@@ -2441,7 +2442,7 @@  or_var_with_comparison_1 (gimple stmt,
 	     => (t OR inner2)
 	     If the partial result t is a constant, we win.  Otherwise
 	     continue on to try reassociating with the other inner test.  */
-	  if (innercode == TRUTH_OR_EXPR)
+	  if (is_or)
 	    {
 	      if (integer_onep (t))
 		return boolean_true_node;
@@ -2452,14 +2453,11 @@  or_var_with_comparison_1 (gimple stmt,
 	  /* Handle the AND case, where we are redistributing:
 	     (inner1 AND inner2) OR (op2a code2 op2b)
 	     => (t AND (inner2 OR (op2a code op2b)))  */
-	  else
-	    {
-	      if (integer_zerop (t))
-		return boolean_false_node;
-	      else
-		/* Save partial result for later.  */
-		partial = t;
-	    }
+	  else if (integer_zerop (t))
+	    return boolean_false_node;
+
+	  /* Save partial result for later.  */
+	  partial = t;
 	}
       
       /* Compute the second partial result, (inner2 OR (op2a code op2b)) */
@@ -2473,13 +2471,18 @@  or_var_with_comparison_1 (gimple stmt,
 	{
 	  /* Handle the OR case, where we are reassociating:
 	     (inner1 OR inner2) OR (op2a code2 op2b)
-	     => (inner1 OR t)  */
-	  if (innercode == TRUTH_OR_EXPR)
+	     => (inner1 OR t)
+	     => (t OR partial)  */
+	  if (is_or)
 	    {
 	      if (integer_zerop (t))
 		return inner1;
 	      else if (integer_onep (t))
 		return boolean_true_node;
+	      /* If both are the same, we can apply the identity
+		 (x OR x) == x.  */
+	      else if (partial && same_bool_result_p (t, partial))
+		return t;
 	    }
 	  
 	  /* Handle the AND case, where we are redistributing:
@@ -2496,13 +2499,13 @@  or_var_with_comparison_1 (gimple stmt,
 		     operand to the redistributed AND expression.  The
 		     interesting case is when at least one is true.
 		     Or, if both are the same, we can apply the identity
-		     (x AND x) == true.  */
+		     (x AND x) == x.  */
 		  if (integer_onep (partial))
 		    return t;
 		  else if (integer_onep (t))
 		    return partial;
 		  else if (same_bool_result_p (t, partial))
-		    return boolean_true_node;
+		    return t;
 		}
 	    }
 	}
--- gcc/testsuite/gcc.c-torture/execute/pr46909-1.c.jj	2010-12-13 15:06:13.000000000 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr46909-1.c	2010-12-13 15:08:03.000000000 +0100
@@ -0,0 +1,22 @@ 
+/* PR tree-optimization/46909 */
+
+extern void abort ();
+
+int
+__attribute__ ((__noinline__))
+foo (unsigned int x)
+{
+  if (! (x == 4 || x == 6) || (x == 2 || x == 6))
+    return 1;
+  return -1;
+}
+
+int
+main ()
+{
+  int i;
+  for (i = -10; i < 10; i++)
+    if (foo (i) != 1 - 2 * (i == 4))
+      abort ();
+  return 0;
+}
--- gcc/testsuite/gcc.c-torture/execute/pr46909-2.c.jj	2010-11-19 19:58:10.083000000 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr46909-2.c	2010-12-14 00:57:03.000000000 +0100
@@ -0,0 +1,22 @@ 
+/* PR tree-optimization/46909 */
+
+extern void abort (void);
+
+int
+__attribute__((noinline))
+foo (int x)
+{
+  if ((x != 0 && x != 13) || x == 5 || x == 20)
+    return 1;
+  return -1;
+}
+
+int
+main (void)
+{
+  int i;
+  for (i = -10; i < 30; i++)
+    if (foo (i) != 1 - 2 * (i == 0) - 2 * (i == 13))
+      abort ();
+  return 0;
+}
--- gcc/testsuite/gcc.dg/pr46909.c.jj	2010-12-13 15:07:31.000000000 +0100
+++ gcc/testsuite/gcc.dg/pr46909.c	2010-12-13 15:11:30.000000000 +0100
@@ -0,0 +1,17 @@ 
+/* PR tree-optimization/46909 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-ifcombine" } */
+
+extern void abort ();
+
+int
+__attribute__ ((__noinline__))
+foo (unsigned int x)
+{
+  if (! (x == 4 || x == 6) || (x == 2 || x == 6))
+    return 1;
+  return -1;
+}
+
+/* { dg-final { scan-tree-dump "optimizing two comparisons to x_\[0-9\]+\\(D\\) != 4" "ifcombine" } } */
+/* { dg-final { cleanup-tree-dump "ifcombine" } } */