This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Folding (a ? b : c) op d


On Fri, 30 Aug 2013, Richard Biener wrote:

On Sat, Jun 29, 2013 at 9:02 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
Hello,

this patch changes the test deciding whether to fold "op d" with both
branches in (a ? b : c) op d. I don't know if the new test is right, it
gives what I expect on the new testcase, but I may have missed important
cases. Cc: Eric for comments as the author of the previous conditions.

Passes bootstrap+testsuite on x86_64-unknown-linux-gnu.

2013-06-29  Marc Glisse  <marc.glisse@inria.fr>

        PR tree-optimization/57755
gcc/
        * fold-const.c (fold_binary_op_with_conditional_arg): Change
        condition under which the transformation is performed.

gcc/testsuite/
        * gcc.dg/pr57755.c: New testcase.
        * gcc.dg/binop-notand1a.c: Remove xfail.
        * gcc.dg/binop-notand4a.c: Likewise.

--
Marc Glisse
Index: fold-const.c
===================================================================
--- fold-const.c        (revision 200556)
+++ fold-const.c        (working copy)
@@ -6097,26 +6097,33 @@ constant_boolean_node (bool value, tree
    given here), it is the second argument.  TYPE is the type of the
    original expression.  Return NULL_TREE if no simplification is
    possible.  */

 static tree
 fold_binary_op_with_conditional_arg (location_t loc,
                                     enum tree_code code,
                                     tree type, tree op0, tree op1,
                                     tree cond, tree arg, int cond_first_p)
 {
-  tree cond_type = cond_first_p ? TREE_TYPE (op0) : TREE_TYPE (op1);
-  tree arg_type = cond_first_p ? TREE_TYPE (op1) : TREE_TYPE (op0);
+  /* ??? This transformation is only worthwhile if we don't have
+     to wrap ARG in a SAVE_EXPR.  */
+  if (TREE_SIDE_EFFECTS (arg))
+    return NULL_TREE;
+
+  /* Avoid exponential recursion.  */
+  static int depth = 0;
+  if (depth > 1)
+    return NULL_TREE;
+

I don't like this kind of measures ... which one again is the transform that
undoes what this function does?

There is no undoing here (you may be thinking of http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57286 ), it is just a recursion that gets very slow for nested conditions:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55219

   tree test, true_value, false_value;
   tree lhs = NULL_TREE;
   tree rhs = NULL_TREE;
-  enum tree_code cond_code = COND_EXPR;

   if (TREE_CODE (cond) == COND_EXPR
       || TREE_CODE (cond) == VEC_COND_EXPR)
     {
       test = TREE_OPERAND (cond, 0);
       true_value = TREE_OPERAND (cond, 1);
       false_value = TREE_OPERAND (cond, 2);
       /* If this operand throws an expression, then it does not make
         sense to try to perform a logical or arithmetic operation
         involving it.  */
@@ -6126,54 +6133,49 @@ fold_binary_op_with_conditional_arg (loc
        rhs = false_value;
     }
   else
     {
       tree testtype = TREE_TYPE (cond);
       test = cond;
       true_value = constant_boolean_node (true, testtype);
       false_value = constant_boolean_node (false, testtype);
     }

-  if (TREE_CODE (TREE_TYPE (test)) == VECTOR_TYPE)
-    cond_code = VEC_COND_EXPR;
-
-  /* This transformation is only worthwhile if we don't have to wrap ARG
-     in a SAVE_EXPR and the operation can be simplified without recursing
-     on at least one of the branches once its pushed inside the COND_EXPR.
*/
-  if (!TREE_CONSTANT (arg)
-      && (TREE_SIDE_EFFECTS (arg)
-         || TREE_CODE (arg) == COND_EXPR || TREE_CODE (arg) ==
VEC_COND_EXPR
-         || TREE_CONSTANT (true_value) || TREE_CONSTANT (false_value)))
-    return NULL_TREE;
+  tree cond_type = cond_first_p ? TREE_TYPE (op0) : TREE_TYPE (op1);
+  tree arg_type = cond_first_p ? TREE_TYPE (op1) : TREE_TYPE (op0);

   arg = fold_convert_loc (loc, arg_type, arg);
+  ++depth;
   if (lhs == 0)
     {
       true_value = fold_convert_loc (loc, cond_type, true_value);
       if (cond_first_p)
        lhs = fold_build2_loc (loc, code, type, true_value, arg);
       else
        lhs = fold_build2_loc (loc, code, type, arg, true_value);
     }
   if (rhs == 0)
     {
       false_value = fold_convert_loc (loc, cond_type, false_value);
       if (cond_first_p)
        rhs = fold_build2_loc (loc, code, type, false_value, arg);
       else
        rhs = fold_build2_loc (loc, code, type, arg, false_value);
     }
+  --depth;

   /* Check that we have simplified at least one of the branches.  */
-  if (!TREE_CONSTANT (arg) && !TREE_CONSTANT (lhs) && !TREE_CONSTANT (rhs))
+  if (!TREE_CONSTANT (lhs) && !TREE_CONSTANT (rhs))
     return NULL_TREE;

+  enum tree_code cond_code = VECTOR_TYPE_P (TREE_TYPE (test))
+                            ? VEC_COND_EXPR : COND_EXPR;
   return fold_build3_loc (loc, cond_code, type, test, lhs, rhs);
 }


 /* Subroutine of fold() that checks for the addition of +/- 0.0.

    If !NEGATE, return true if ADDEND is +/-0.0 and, for all X of type
    TYPE, X + ADDEND is the same as X.  If NEGATE, return true if X -
    ADDEND is the same as X.

Index: testsuite/gcc.dg/binop-notand1a.c
===================================================================
--- testsuite/gcc.dg/binop-notand1a.c   (revision 200556)
+++ testsuite/gcc.dg/binop-notand1a.c   (working copy)
@@ -1,13 +1,14 @@
 /* { dg-do compile } */
 /* { dg-options "-O2 -fdump-tree-optimized" } */

 int
 foo (char a, unsigned short b)
 {
   return (a & !a) | (b & !b);
 }

 /* As long as comparisons aren't boolified and casts from boolean-types
-   aren't preserved, the folding of  X & !X to zero fails.  */
-/* { dg-final { scan-tree-dump-times "return 0" 1 "optimized" { xfail *-*-*
} } } */
+   aren't preserved, the direct folding of X & !X to zero fails.
+   However, fold_binary_op_with_conditional_arg undirectly helps it.  */
+/* { dg-final { scan-tree-dump-times "return 0" 1 "optimized" } } */
 /* { dg-final { cleanup-tree-dump "optimized" } } */
Index: testsuite/gcc.dg/binop-notand4a.c
===================================================================
--- testsuite/gcc.dg/binop-notand4a.c   (revision 200556)
+++ testsuite/gcc.dg/binop-notand4a.c   (working copy)
@@ -1,13 +1,14 @@
 /* { dg-do compile } */
 /* { dg-options "-O2 -fdump-tree-optimized" } */

 int
 foo (unsigned char a, _Bool b)
 {
   return (!a & a) | (b & !b);
 }

 /* As long as comparisons aren't boolified and casts from boolean-types
-   aren't preserved, the folding of  X & !X to zero fails.  */
-/* { dg-final { scan-tree-dump-times "return 0" 1 "optimized" { xfail *-*-*
} } } */
+   aren't preserved, the direct folding of X & !X to zero fails.
+   However, fold_binary_op_with_conditional_arg undirectly helps it.  */
+/* { dg-final { scan-tree-dump-times "return 0" 1 "optimized" } } */
 /* { dg-final { cleanup-tree-dump "optimized" } } */
Index: testsuite/gcc.dg/pr57755.c
===================================================================
--- testsuite/gcc.dg/pr57755.c  (revision 0)
+++ testsuite/gcc.dg/pr57755.c  (revision 0)
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-original" } */
+
+int f(int a,int b){
+  return (((a<=3)?-1:0)&((b<=2)?-1:0))!=0;
+}
+
+unsigned g(unsigned a,unsigned b,unsigned c){
+  return ((a<b)?a:c)*3/42+1;
+}
+
+unsigned h(unsigned a,unsigned b){
+  return ((a<=42)?b:0)&b;
+}
+
+/* { dg-final { scan-tree-dump "return a <= 3 && b <= 2;" "original" } } */
+/* { dg-final { scan-tree-dump-times "/ 42" 1 "original" } } */
+/* { dg-final { scan-tree-dump "return a <= 42 \\\? NON_LVALUE_EXPR <b> :
0;" "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */

Property changes on: testsuite/gcc.dg/pr57755.c
___________________________________________________________________
Added: svn:keywords
   + Author Date Id Revision URL
Added: svn:eol-style
   + native

--
Marc Glisse


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]