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

[Richard Biener <richard dot guenther at gmail dot com>]

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?

>    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
>
>