Message ID | 20101214094528.GE27214@tyan-ft48-01.lab.bos.redhat.com |
---|---|
State | New |
Headers | show |
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
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
--- 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" } } */