On Sat, Jun 29, 2013 at 9:02 AM, Marc Glisse <marc.gli...@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.gli...@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
>
>
