[tree-optimization] : [2 of 3]: Boolify compares & more

Message ID	CAEwic4YMRruZLRfCJTGeso1urp_Nq_Ya=G0TSG+8-DYjgYD0gQ@mail.gmail.com
State	New
Headers	show Return-Path: <gcc-patches-return-296240-incoming=patchwork.ozlabs.org@gcc.gnu.org> MIME-Version: 1.0 Date: Thu, 7 Jul 2011 18:07:29 +0200 Message-ID: <CAEwic4YMRruZLRfCJTGeso1urp_Nq_Ya=G0TSG+8-DYjgYD0gQ@mail.gmail.com> Subject: [patch tree-optimization]: [2 of 3]: Boolify compares & more From: Kai Tietz <ktietz70@googlemail.com> To: GCC Patches <gcc-patches@gcc.gnu.org> Cc: Richard Guenther <richard.guenther@gmail.com> Content-Type: text/plain; charset=ISO-8859-1 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk Sender: gcc-patches-owner@gcc.gnu.org

Kai Tietz July 7, 2011, 4:07 p.m. UTC

Hello,

This patch - second of series - adds boolification of comparisions in
gimplifier.  For this
casts from/to boolean are marked as not-useless. And in fold_unary_loc
casts to non-boolean integral types are preserved.
The hunk in tree-ssa-forwprop.c in combine_cond-expr_cond is not strictly
necessary - as long as fold-const handles 1-bit precision bitwise-expression
with truth-logic - but it has shown to short-cut some expensier folding. So
I kept it within this patch.

The adjusted testcase gcc.dg/uninit-15.c indicates that due
optimization we loose
in this case variables declaration.  But this might be to be expected.

In vectorization we have a regression in gcc.dg/vect/vect-cond-3.c
test-case.  It's caused
by always having boolean-type on conditions.  So vectorizer sees

Richard Biener July 8, 2011, 9:20 a.m. UTC | #1

On Thu, Jul 7, 2011 at 6:07 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> Hello,
>
> This patch - second of series - adds boolification of comparisions in
> gimplifier.  For this
> casts from/to boolean are marked as not-useless. And in fold_unary_loc
> casts to non-boolean integral types are preserved.
> The hunk in tree-ssa-forwprop.c in combine_cond-expr_cond is not strictly
> necessary - as long as fold-const handles 1-bit precision bitwise-expression
> with truth-logic - but it has shown to short-cut some expensier folding. So
> I kept it within this patch.

Please split it out.  Also ...

>
> The adjusted testcase gcc.dg/uninit-15.c indicates that due
> optimization we loose
> in this case variables declaration.  But this might be to be expected.
>
> In vectorization we have a regression in gcc.dg/vect/vect-cond-3.c
> test-case.  It's caused
> by always having boolean-type on conditions.  So vectorizer sees
> different types, which
> aren't handled by vectorizer right now.  Maybe this issue could be
> special-cased for
> boolean-types in tree-vect-loop, by making operand for used condition
> equal to vector-type.
> But this is a subject for a different patch and not addressed by this series.
>
> There is a regressions in tree-ssa/vrp47.c, and the fix is addressed
> by the 3rd patch of this
> series.
>
> Bootstrapped and regression tested for all standard-languages (plus
> Ada and Obj-C++) on host x86_64-pc-linux-gnu.
>
> Ok for apply?
>
> Regards,
> Kai
>
>
> ChangeLog
>
> 2011-07-07  Kai Tietz  <ktietz@redhat.com>
>
>        * fold-const.c (fold_unary_loc): Preserve
>        non-boolean-typed casts.
>        * gimplify.c (gimple_boolify): Handle boolification
>        of comparisons.
>        (gimplify_expr): Boolifiy non aggregate-typed
>        comparisons.
>        * tree-cfg.c (verify_gimple_comparison): Check result
>        type of comparison expression.
>        * tree-ssa.c (useless_type_conversion_p): Preserve incompatible
>        casts from/to boolean,
>        * tree-ssa-forwprop.c (combine_cond_expr_cond): Add simplification
>        support for one-bit-precision typed X for cases X != 0 and X == 0.
>        (forward_propagate_comparison): Adjust test of condition
>        result.
>
>
>        * gcc.dg/tree-ssa/builtin-expect-5.c: Adjusted.
>        * gcc.dg/tree-ssa/pr21031.c: Likewise.
>        * gcc.dg/tree-ssa/pr30978.c: Likewise.
>        * gcc.dg/tree-ssa/ssa-fre-6.c: Likewise.
>        * gcc.dg/binop-xor1.c: Mark it as expected fail.
>        * gcc.dg/binop-xor3.c: Likewise.
>        * gcc.dg/uninit-15.c: Adjust reported message.
>
> Index: gcc-head/gcc/fold-const.c
> ===================================================================
> --- gcc-head.orig/gcc/fold-const.c
> +++ gcc-head/gcc/fold-const.c
> @@ -7665,11 +7665,11 @@ fold_unary_loc (location_t loc, enum tre
>             non-integral type.
>             Do not fold the result as that would not simplify further, also
>             folding again results in recursions.  */
> -         if (INTEGRAL_TYPE_P (type))
> +         if (TREE_CODE (type) == BOOLEAN_TYPE)
>            return build2_loc (loc, TREE_CODE (op0), type,
>                               TREE_OPERAND (op0, 0),
>                               TREE_OPERAND (op0, 1));
> -         else
> +         else if (!INTEGRAL_TYPE_P (type))
>            return build3_loc (loc, COND_EXPR, type, op0,
>                               fold_convert (type, boolean_true_node),
>                               fold_convert (type, boolean_false_node));
> Index: gcc-head/gcc/gimplify.c
> ===================================================================
> --- gcc-head.orig/gcc/gimplify.c
> +++ gcc-head/gcc/gimplify.c
> @@ -2842,18 +2842,23 @@ gimple_boolify (tree expr)
>
>     case TRUTH_NOT_EXPR:
>       TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
> -      /* FALLTHRU */
>
> -    case EQ_EXPR: case NE_EXPR:
> -    case LE_EXPR: case GE_EXPR: case LT_EXPR: case GT_EXPR:
>       /* These expressions always produce boolean results.  */
> -      TREE_TYPE (expr) = boolean_type_node;
> +      if (TREE_CODE (type) != BOOLEAN_TYPE)
> +       TREE_TYPE (expr) = boolean_type_node;
>       return expr;
>
>     default:
> +      if (COMPARISON_CLASS_P (expr))
> +       {
> +         /* There expressions always prduce boolean results.  */
> +         if (TREE_CODE (type) != BOOLEAN_TYPE)
> +           TREE_TYPE (expr) = boolean_type_node;
> +         return expr;
> +       }
>       /* Other expressions that get here must have boolean values, but
>         might need to be converted to the appropriate mode.  */
> -      if (type == boolean_type_node)
> +      if (TREE_CODE (type) == BOOLEAN_TYPE)
>        return expr;
>       return fold_convert_loc (loc, boolean_type_node, expr);
>     }
> @@ -6763,7 +6768,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>            tree org_type = TREE_TYPE (*expr_p);
>
>            *expr_p = gimple_boolify (*expr_p);
> -           if (org_type != boolean_type_node)
> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>              {
>                *expr_p = fold_convert (org_type, *expr_p);

Use fold_convert_loc with saved_location

>                ret = GS_OK;
> @@ -7208,7 +7213,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>               fold_truth_not_expr) happily uses operand type and doesn't
>               automatically uses boolean_type as result, we need to keep
>               orignal type.  */
> -           if (org_type != boolean_type_node)
> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>              {
>                *expr_p = fold_convert (org_type, *expr_p);

Likewise.  Maybe this fixes the diagnostic regression.

>                ret = GS_OK;
> @@ -7288,7 +7293,19 @@ gimplify_expr (tree *expr_p, gimple_seq
>                  tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>
>                  if (!AGGREGATE_TYPE_P (type))
> -                   goto expr_2;
> +                   {
> +                     tree org_type = TREE_TYPE (*expr_p);
> +                     *expr_p = gimple_boolify (*expr_p);
> +                     if (!useless_type_conversion_p (org_type,
> +                                                     TREE_TYPE (*expr_p)))
> +                       {
> +                         *expr_p = fold_convert_loc (saved_location,
> +                                                     org_type, *expr_p);
> +                         ret = GS_OK;
> +                       }
> +                     else
> +                       goto expr_2;
> +                   }
>                  else if (TYPE_MODE (type) != BLKmode)
>                    ret = gimplify_scalar_mode_aggregate_compare (expr_p);
>                  else
> Index: gcc-head/gcc/tree-cfg.c
> ===================================================================
> --- gcc-head.orig/gcc/tree-cfg.c
> +++ gcc-head/gcc/tree-cfg.c
> @@ -3203,7 +3203,9 @@ verify_gimple_comparison (tree type, tre
>        && (!POINTER_TYPE_P (op0_type)
>           || !POINTER_TYPE_P (op1_type)
>           || TYPE_MODE (op0_type) != TYPE_MODE (op1_type)))
> -      || !INTEGRAL_TYPE_P (type))
> +      || !INTEGRAL_TYPE_P (type)
> +      || (TREE_CODE (type) != BOOLEAN_TYPE
> +         && TYPE_PRECISION (type) != 1))
>     {
>       error ("type mismatch in comparison expression");
>       debug_generic_expr (type);
> Index: gcc-head/gcc/tree-ssa.c
> ===================================================================
> --- gcc-head.orig/gcc/tree-ssa.c
> +++ gcc-head/gcc/tree-ssa.c
> @@ -1306,10 +1306,10 @@ useless_type_conversion_p (tree outer_ty
>          || TYPE_PRECISION (inner_type) != TYPE_PRECISION (outer_type))
>        return false;
>
> -      /* Preserve conversions to BOOLEAN_TYPE if it is not of precision
> -         one.  */
> -      if (TREE_CODE (inner_type) != BOOLEAN_TYPE
> -         && TREE_CODE (outer_type) == BOOLEAN_TYPE
> +      /* Preserve conversions to/from BOOLEAN_TYPE if types are not
> +        of precision one.  */
> +      if (((TREE_CODE (inner_type) == BOOLEAN_TYPE)
> +          != (TREE_CODE (outer_type) == BOOLEAN_TYPE))
>          && TYPE_PRECISION (outer_type) != 1)
>        return false;
>
> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
> ===================================================================
> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
> @@ -11,5 +11,5 @@ f (int i, float j)
>
>  /* { dg-final { scan-tree-dump-times { if } 2 "forwprop1"} } */
>  /* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 0\);\n[^\n]*if}
> "forwprop1"} } */
> -/* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 1\);\n[^\n]*if}
> "forwprop1"} } */
> +/* { dg-final { scan-tree-dump-not {builtin_expect[^\n]*,
> 1\);\n[^\n]*if} "forwprop1"} } */

Hm?  Why that?

>  /* { dg-final { cleanup-tree-dump "forwprop?" } } */
> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
> ===================================================================
> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
> @@ -16,5 +16,5 @@ foo (int a)
>     return 0;
>  }
>
> -/* { dg-final { scan-tree-dump-times "Replaced" 2 "forwprop1"} } */
> +/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
>  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
> ===================================================================
> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
> @@ -10,5 +10,5 @@ int foo(int a)
>   return e;
>  }
>
> -/* { dg-final { scan-tree-dump "e_. = a_..D. > 0;" "optimized" } } */
> +/* { dg-final { scan-tree-dump " = a_..D. > 0;" "optimized" } } */
>  /* { dg-final { cleanup-tree-dump "optimized" } } */
> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
> ===================================================================
> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
> @@ -2,5 +2,5 @@
>  /* { dg-options "-O -fdump-tree-fre1-details" } */
>
>  int i; int foo(void) { i = 2; int j = i * 2; int k = i + 2; return j == k; }
> -/* { dg-final { scan-tree-dump-times "Replaced " 5 "fre1" } } */
> +/* { dg-final { scan-tree-dump-times "Replaced " 6 "fre1" } } */
>  /* { dg-final { cleanup-tree-dump "fre1" } } */
> Index: gcc-head/gcc/tree-ssa-forwprop.c
> ===================================================================
> --- gcc-head.orig/gcc/tree-ssa-forwprop.c
> +++ gcc-head/gcc/tree-ssa-forwprop.c
> @@ -367,9 +367,61 @@ combine_cond_expr_cond (location_t loc,
>   gcc_assert (TREE_CODE_CLASS (code) == tcc_comparison);
>
>   t = fold_binary_loc (loc, code, type, op0, op1);
> +
> +  if (!t && INTEGRAL_TYPE_P (TREE_TYPE (op1))
> +      && TYPE_PRECISION (TREE_TYPE (op1)) == 1
> +      && (code == EQ_EXPR || code == NE_EXPR))
> +    {
> +      if (TREE_CODE (op1) == INTEGER_CST)
> +        {
> +         if (integer_onep (op1))
> +           {
> +             op1 = fold_convert_loc (loc, TREE_TYPE (op1), integer_zero_node);
> +             code = (code == NE_EXPR ? EQ_EXPR : NE_EXPR);

So you change truthvalue !=/== 1 to truthvalue ==/!= 0 and then
recurse ... that doesn't make sense to me and is super-ugly.
What's the testcase that made you add all this code?

> +             return combine_cond_expr_cond (loc, code, type, op0, op1,
> invariant_only);
> +           }
> +         if (TREE_CODE_CLASS (TREE_CODE (op0)) == tcc_comparison
> +             && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0))))
> +           {
> +             if (code == NE_EXPR)
> +               t = op0;
> +             else
> +               {
> +                 code = invert_tree_comparison (TREE_CODE (op0), false);
> +                 if (code != ERROR_MARK)
> +                   t = fold_build2_loc (loc, code, type,
> +                                        TREE_OPERAND (op0, 0), TREE_OPERAND (op0, 1));
> +               }
> +           }
> +       }
> +      else
> +        {
> +         if (TREE_CODE_CLASS (TREE_CODE (op0)) == tcc_comparison
> +             && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0))))
> +           {
> +             enum tree_code opc = invert_tree_comparison (TREE_CODE (op0), false);
> +             if (opc != ERROR_MARK)
> +               {
> +                 op0 = fold_build2_loc (loc, opc, TREE_TYPE (op0),
> +                                        TREE_OPERAND (op0, 0), TREE_OPERAND (op0, 1));
> +                 t = fold_build2_loc (loc, BIT_XOR_EXPR, type, op0, op1);
> +               }
> +           }
> +         else if (TREE_CODE_CLASS (TREE_CODE (op1)) == tcc_comparison
> +             && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op1, 0))))
> +           {
> +             enum tree_code opc = invert_tree_comparison (TREE_CODE (op1), false);
> +             if (opc != ERROR_MARK)
> +               {
> +                 op1 = fold_build2_loc (loc, opc, TREE_TYPE (op1),
> +                                        TREE_OPERAND (op1, 0), TREE_OPERAND (op1, 1));
> +                 t = fold_build2_loc (loc, BIT_XOR_EXPR, type, op0, op1);
> +               }
> +           }
> +       }
> +    }
>   if (!t)
>     return NULL_TREE;
> -
>   /* Require that we got a boolean type out if we put one in.  */
>   gcc_assert (TREE_CODE (TREE_TYPE (t)) == TREE_CODE (type));
>
> @@ -1122,13 +1174,11 @@ forward_propagate_comparison (gimple stm
>   if (!use_stmt)
>     return false;
>
> -  /* Conversion of the condition result to another integral type.  */
> +  /* Test of the condition result.  */
>   if (is_gimple_assign (use_stmt)
> -      && (CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (use_stmt))
> -         || TREE_CODE_CLASS (gimple_assign_rhs_code (use_stmt))
> -            == tcc_comparison
> -          || gimple_assign_rhs_code (use_stmt) == TRUTH_NOT_EXPR)
> -      && INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_lhs (use_stmt))))
> +      && ((TREE_CODE_CLASS (gimple_assign_rhs_code (use_stmt))
> +          == tcc_comparison)
> +          || gimple_assign_rhs_code (use_stmt) == TRUTH_NOT_EXPR))
>     {
>       tree lhs = gimple_assign_lhs (use_stmt);
>
> Index: gcc-head/gcc/testsuite/gcc.dg/binop-xor1.c
> ===================================================================
> --- gcc-head.orig/gcc/testsuite/gcc.dg/binop-xor1.c
> +++ gcc-head/gcc/testsuite/gcc.dg/binop-xor1.c
> @@ -7,8 +7,5 @@ foo (int a, int b, int c)
>   return ((a && !b && c) || (!a && b && c));
>  }
>
> -/* We expect to see "<bb N>"; confirm that, so that we know to count
> -   it in the real test.  */
> -/* { dg-final { scan-tree-dump-times "<bb\[^>\]*>" 5 "optimized" } } */
> -/* { dg-final { scan-tree-dump-times "\\\^" 1 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "\\\^" 1 "optimized" { xfail
> *-*-* } } } */
>  /* { dg-final { cleanup-tree-dump "optimized" } } */
> Index: gcc-head/gcc/testsuite/gcc.dg/binop-xor3.c
> ===================================================================
> --- gcc-head.orig/gcc/testsuite/gcc.dg/binop-xor3.c
> +++ gcc-head/gcc/testsuite/gcc.dg/binop-xor3.c
> @@ -7,8 +7,5 @@ foo (int a, int b)
>   return ((a && !b) || (!a && b));
>  }
>
> -/* We expect to see "<bb N>"; confirm that, so that we know to count
> -   it in the real test.  */
> -/* { dg-final { scan-tree-dump-times "<bb\[^>\]*>" 1 "optimized" } } */
> -/* { dg-final { scan-tree-dump-times "\\\^" 1 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "\\\^" 1 "optimized" { xfail
> *-*-* } } } */
>  /* { dg-final { cleanup-tree-dump "optimized" } } */
> Index: gcc-head/gcc/testsuite/gcc.dg/uninit-15.c
> ===================================================================
> --- gcc-head.orig/gcc/testsuite/gcc.dg/uninit-15.c
> +++ gcc-head/gcc/testsuite/gcc.dg/uninit-15.c
> @@ -18,9 +18,9 @@ foo (int i)
>  void baz (void);
>
>  void
> -bar (void)
> +bar (void) /* { dg-warning "'j' is used uninitialized" } */
>  {
>   int j; /* { dg-message "note: 'j' was declared here" "" { xfail *-*-* } } */
> -  for (; foo (j); ++j)  /* { dg-warning "'j' is used uninitialized" } */
> +  for (; foo (j); ++j)
>     baz ();
>  }
>

Kai Tietz July 8, 2011, 11:32 a.m. UTC | #2

2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
> On Thu, Jul 7, 2011 at 6:07 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> Hello,
>>
>> This patch - second of series - adds boolification of comparisions in
>> gimplifier.  For this
>> casts from/to boolean are marked as not-useless. And in fold_unary_loc
>> casts to non-boolean integral types are preserved.
>> The hunk in tree-ssa-forwprop.c in combine_cond-expr_cond is not strictly
>> necessary - as long as fold-const handles 1-bit precision bitwise-expression
>> with truth-logic - but it has shown to short-cut some expensier folding. So
>> I kept it within this patch.
>
> Please split it out.  Also ...
>
>>
>> The adjusted testcase gcc.dg/uninit-15.c indicates that due
>> optimization we loose
>> in this case variables declaration.  But this might be to be expected.
>>
>> In vectorization we have a regression in gcc.dg/vect/vect-cond-3.c
>> test-case.  It's caused
>> by always having boolean-type on conditions.  So vectorizer sees
>> different types, which
>> aren't handled by vectorizer right now.  Maybe this issue could be
>> special-cased for
>> boolean-types in tree-vect-loop, by making operand for used condition
>> equal to vector-type.
>> But this is a subject for a different patch and not addressed by this series.
>>
>> There is a regressions in tree-ssa/vrp47.c, and the fix is addressed
>> by the 3rd patch of this
>> series.
>>
>> Bootstrapped and regression tested for all standard-languages (plus
>> Ada and Obj-C++) on host x86_64-pc-linux-gnu.
>>
>> Ok for apply?
>>
>> Regards,
>> Kai
>>
>>
>> ChangeLog
>>
>> 2011-07-07  Kai Tietz  <ktietz@redhat.com>
>>
>>        * fold-const.c (fold_unary_loc): Preserve
>>        non-boolean-typed casts.
>>        * gimplify.c (gimple_boolify): Handle boolification
>>        of comparisons.
>>        (gimplify_expr): Boolifiy non aggregate-typed
>>        comparisons.
>>        * tree-cfg.c (verify_gimple_comparison): Check result
>>        type of comparison expression.
>>        * tree-ssa.c (useless_type_conversion_p): Preserve incompatible
>>        casts from/to boolean,
>>        * tree-ssa-forwprop.c (combine_cond_expr_cond): Add simplification
>>        support for one-bit-precision typed X for cases X != 0 and X == 0.
>>        (forward_propagate_comparison): Adjust test of condition
>>        result.
>>
>>
>>        * gcc.dg/tree-ssa/builtin-expect-5.c: Adjusted.
>>        * gcc.dg/tree-ssa/pr21031.c: Likewise.
>>        * gcc.dg/tree-ssa/pr30978.c: Likewise.
>>        * gcc.dg/tree-ssa/ssa-fre-6.c: Likewise.
>>        * gcc.dg/binop-xor1.c: Mark it as expected fail.
>>        * gcc.dg/binop-xor3.c: Likewise.
>>        * gcc.dg/uninit-15.c: Adjust reported message.
>>
>> Index: gcc-head/gcc/fold-const.c
>> ===================================================================
>> --- gcc-head.orig/gcc/fold-const.c
>> +++ gcc-head/gcc/fold-const.c
>> @@ -7665,11 +7665,11 @@ fold_unary_loc (location_t loc, enum tre
>>             non-integral type.
>>             Do not fold the result as that would not simplify further, also
>>             folding again results in recursions.  */
>> -         if (INTEGRAL_TYPE_P (type))
>> +         if (TREE_CODE (type) == BOOLEAN_TYPE)
>>            return build2_loc (loc, TREE_CODE (op0), type,
>>                               TREE_OPERAND (op0, 0),
>>                               TREE_OPERAND (op0, 1));
>> -         else
>> +         else if (!INTEGRAL_TYPE_P (type))
>>            return build3_loc (loc, COND_EXPR, type, op0,
>>                               fold_convert (type, boolean_true_node),
>>                               fold_convert (type, boolean_false_node));
>> Index: gcc-head/gcc/gimplify.c
>> ===================================================================
>> --- gcc-head.orig/gcc/gimplify.c
>> +++ gcc-head/gcc/gimplify.c
>> @@ -2842,18 +2842,23 @@ gimple_boolify (tree expr)
>>
>>     case TRUTH_NOT_EXPR:
>>       TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
>> -      /* FALLTHRU */
>>
>> -    case EQ_EXPR: case NE_EXPR:
>> -    case LE_EXPR: case GE_EXPR: case LT_EXPR: case GT_EXPR:
>>       /* These expressions always produce boolean results.  */
>> -      TREE_TYPE (expr) = boolean_type_node;
>> +      if (TREE_CODE (type) != BOOLEAN_TYPE)
>> +       TREE_TYPE (expr) = boolean_type_node;
>>       return expr;
>>
>>     default:
>> +      if (COMPARISON_CLASS_P (expr))
>> +       {
>> +         /* There expressions always prduce boolean results.  */
>> +         if (TREE_CODE (type) != BOOLEAN_TYPE)
>> +           TREE_TYPE (expr) = boolean_type_node;
>> +         return expr;
>> +       }
>>       /* Other expressions that get here must have boolean values, but
>>         might need to be converted to the appropriate mode.  */
>> -      if (type == boolean_type_node)
>> +      if (TREE_CODE (type) == BOOLEAN_TYPE)
>>        return expr;
>>       return fold_convert_loc (loc, boolean_type_node, expr);
>>     }
>> @@ -6763,7 +6768,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>            tree org_type = TREE_TYPE (*expr_p);
>>
>>            *expr_p = gimple_boolify (*expr_p);
>> -           if (org_type != boolean_type_node)
>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>              {
>>                *expr_p = fold_convert (org_type, *expr_p);
>
> Use fold_convert_loc with saved_location

Oh, good catch. Yes, I will adjust that.

>>                ret = GS_OK;
>> @@ -7208,7 +7213,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>               fold_truth_not_expr) happily uses operand type and doesn't
>>               automatically uses boolean_type as result, we need to keep
>>               orignal type.  */
>> -           if (org_type != boolean_type_node)
>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>              {
>>                *expr_p = fold_convert (org_type, *expr_p);
>
> Likewise.  Maybe this fixes the diagnostic regression.
>
>>                ret = GS_OK;
>> @@ -7288,7 +7293,19 @@ gimplify_expr (tree *expr_p, gimple_seq
>>                  tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>
>>                  if (!AGGREGATE_TYPE_P (type))
>> -                   goto expr_2;
>> +                   {
>> +                     tree org_type = TREE_TYPE (*expr_p);
>> +                     *expr_p = gimple_boolify (*expr_p);
>> +                     if (!useless_type_conversion_p (org_type,
>> +                                                     TREE_TYPE (*expr_p)))
>> +                       {
>> +                         *expr_p = fold_convert_loc (saved_location,
>> +                                                     org_type, *expr_p);
>> +                         ret = GS_OK;
>> +                       }
>> +                     else
>> +                       goto expr_2;
>> +                   }
>>                  else if (TYPE_MODE (type) != BLKmode)
>>                    ret = gimplify_scalar_mode_aggregate_compare (expr_p);
>>                  else
>> Index: gcc-head/gcc/tree-cfg.c
>> ===================================================================
>> --- gcc-head.orig/gcc/tree-cfg.c
>> +++ gcc-head/gcc/tree-cfg.c
>> @@ -3203,7 +3203,9 @@ verify_gimple_comparison (tree type, tre
>>        && (!POINTER_TYPE_P (op0_type)
>>           || !POINTER_TYPE_P (op1_type)
>>           || TYPE_MODE (op0_type) != TYPE_MODE (op1_type)))
>> -      || !INTEGRAL_TYPE_P (type))
>> +      || !INTEGRAL_TYPE_P (type)
>> +      || (TREE_CODE (type) != BOOLEAN_TYPE
>> +         && TYPE_PRECISION (type) != 1))
>>     {
>>       error ("type mismatch in comparison expression");
>>       debug_generic_expr (type);
>> Index: gcc-head/gcc/tree-ssa.c
>> ===================================================================
>> --- gcc-head.orig/gcc/tree-ssa.c
>> +++ gcc-head/gcc/tree-ssa.c
>> @@ -1306,10 +1306,10 @@ useless_type_conversion_p (tree outer_ty
>>          || TYPE_PRECISION (inner_type) != TYPE_PRECISION (outer_type))
>>        return false;
>>
>> -      /* Preserve conversions to BOOLEAN_TYPE if it is not of precision
>> -         one.  */
>> -      if (TREE_CODE (inner_type) != BOOLEAN_TYPE
>> -         && TREE_CODE (outer_type) == BOOLEAN_TYPE
>> +      /* Preserve conversions to/from BOOLEAN_TYPE if types are not
>> +        of precision one.  */
>> +      if (((TREE_CODE (inner_type) == BOOLEAN_TYPE)
>> +          != (TREE_CODE (outer_type) == BOOLEAN_TYPE))
>>          && TYPE_PRECISION (outer_type) != 1)
>>        return false;
>>
>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>> ===================================================================
>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>> @@ -11,5 +11,5 @@ f (int i, float j)
>>
>>  /* { dg-final { scan-tree-dump-times { if } 2 "forwprop1"} } */
>>  /* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 0\);\n[^\n]*if}
>> "forwprop1"} } */
>> -/* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 1\);\n[^\n]*if}
>> "forwprop1"} } */
>> +/* { dg-final { scan-tree-dump-not {builtin_expect[^\n]*,
>> 1\);\n[^\n]*if} "forwprop1"} } */
>
> Hm?  Why that?
>
>>  /* { dg-final { cleanup-tree-dump "forwprop?" } } */
>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>> ===================================================================
>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>> @@ -16,5 +16,5 @@ foo (int a)
>>     return 0;
>>  }
>>
>> -/* { dg-final { scan-tree-dump-times "Replaced" 2 "forwprop1"} } */
>> +/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
>>  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>> ===================================================================
>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>> @@ -10,5 +10,5 @@ int foo(int a)
>>   return e;
>>  }
>>
>> -/* { dg-final { scan-tree-dump "e_. = a_..D. > 0;" "optimized" } } */
>> +/* { dg-final { scan-tree-dump " = a_..D. > 0;" "optimized" } } */
>>  /* { dg-final { cleanup-tree-dump "optimized" } } */
>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>> ===================================================================
>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>> @@ -2,5 +2,5 @@
>>  /* { dg-options "-O -fdump-tree-fre1-details" } */
>>
>>  int i; int foo(void) { i = 2; int j = i * 2; int k = i + 2; return j == k; }
>> -/* { dg-final { scan-tree-dump-times "Replaced " 5 "fre1" } } */
>> +/* { dg-final { scan-tree-dump-times "Replaced " 6 "fre1" } } */
>>  /* { dg-final { cleanup-tree-dump "fre1" } } */
>> Index: gcc-head/gcc/tree-ssa-forwprop.c
>> ===================================================================
>> --- gcc-head.orig/gcc/tree-ssa-forwprop.c
>> +++ gcc-head/gcc/tree-ssa-forwprop.c
>> @@ -367,9 +367,61 @@ combine_cond_expr_cond (location_t loc,
>>   gcc_assert (TREE_CODE_CLASS (code) == tcc_comparison);
>>
>>   t = fold_binary_loc (loc, code, type, op0, op1);
>> +
>> +  if (!t && INTEGRAL_TYPE_P (TREE_TYPE (op1))
>> +      && TYPE_PRECISION (TREE_TYPE (op1)) == 1
>> +      && (code == EQ_EXPR || code == NE_EXPR))
>> +    {
>> +      if (TREE_CODE (op1) == INTEGER_CST)
>> +        {
>> +         if (integer_onep (op1))
>> +           {
>> +             op1 = fold_convert_loc (loc, TREE_TYPE (op1), integer_zero_node);
>> +             code = (code == NE_EXPR ? EQ_EXPR : NE_EXPR);
>
> So you change truthvalue !=/== 1 to truthvalue ==/!= 0 and then
> recurse ... that doesn't make sense to me and is super-ugly.
> What's the testcase that made you add all this code?

Well, the convert from truthvalue !=/== 1 to !=/== 0 limits the amount
of cases to handle. As for truthvalued X the we have then just to
handle two cases. X != 0 -> X, and X == 0 -> (X ^ 1).
The recursion is someting I saw as existing pattern (for the same
thing) in truth-op folding in fold-const.

Actual I can remove this optimization here, as it should be convered
by VRP already (when VRP handles 1-bit precision bitwise ops proper).

Richard Biener July 8, 2011, 11:42 a.m. UTC | #3

On Fri, Jul 8, 2011 at 1:32 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>> On Thu, Jul 7, 2011 at 6:07 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>> Hello,
>>>
>>> This patch - second of series - adds boolification of comparisions in
>>> gimplifier.  For this
>>> casts from/to boolean are marked as not-useless. And in fold_unary_loc
>>> casts to non-boolean integral types are preserved.
>>> The hunk in tree-ssa-forwprop.c in combine_cond-expr_cond is not strictly
>>> necessary - as long as fold-const handles 1-bit precision bitwise-expression
>>> with truth-logic - but it has shown to short-cut some expensier folding. So
>>> I kept it within this patch.
>>
>> Please split it out.  Also ...
>>
>>>
>>> The adjusted testcase gcc.dg/uninit-15.c indicates that due
>>> optimization we loose
>>> in this case variables declaration.  But this might be to be expected.
>>>
>>> In vectorization we have a regression in gcc.dg/vect/vect-cond-3.c
>>> test-case.  It's caused
>>> by always having boolean-type on conditions.  So vectorizer sees
>>> different types, which
>>> aren't handled by vectorizer right now.  Maybe this issue could be
>>> special-cased for
>>> boolean-types in tree-vect-loop, by making operand for used condition
>>> equal to vector-type.
>>> But this is a subject for a different patch and not addressed by this series.
>>>
>>> There is a regressions in tree-ssa/vrp47.c, and the fix is addressed
>>> by the 3rd patch of this
>>> series.
>>>
>>> Bootstrapped and regression tested for all standard-languages (plus
>>> Ada and Obj-C++) on host x86_64-pc-linux-gnu.
>>>
>>> Ok for apply?
>>>
>>> Regards,
>>> Kai
>>>
>>>
>>> ChangeLog
>>>
>>> 2011-07-07  Kai Tietz  <ktietz@redhat.com>
>>>
>>>        * fold-const.c (fold_unary_loc): Preserve
>>>        non-boolean-typed casts.
>>>        * gimplify.c (gimple_boolify): Handle boolification
>>>        of comparisons.
>>>        (gimplify_expr): Boolifiy non aggregate-typed
>>>        comparisons.
>>>        * tree-cfg.c (verify_gimple_comparison): Check result
>>>        type of comparison expression.
>>>        * tree-ssa.c (useless_type_conversion_p): Preserve incompatible
>>>        casts from/to boolean,
>>>        * tree-ssa-forwprop.c (combine_cond_expr_cond): Add simplification
>>>        support for one-bit-precision typed X for cases X != 0 and X == 0.
>>>        (forward_propagate_comparison): Adjust test of condition
>>>        result.
>>>
>>>
>>>        * gcc.dg/tree-ssa/builtin-expect-5.c: Adjusted.
>>>        * gcc.dg/tree-ssa/pr21031.c: Likewise.
>>>        * gcc.dg/tree-ssa/pr30978.c: Likewise.
>>>        * gcc.dg/tree-ssa/ssa-fre-6.c: Likewise.
>>>        * gcc.dg/binop-xor1.c: Mark it as expected fail.
>>>        * gcc.dg/binop-xor3.c: Likewise.
>>>        * gcc.dg/uninit-15.c: Adjust reported message.
>>>
>>> Index: gcc-head/gcc/fold-const.c
>>> ===================================================================
>>> --- gcc-head.orig/gcc/fold-const.c
>>> +++ gcc-head/gcc/fold-const.c
>>> @@ -7665,11 +7665,11 @@ fold_unary_loc (location_t loc, enum tre
>>>             non-integral type.
>>>             Do not fold the result as that would not simplify further, also
>>>             folding again results in recursions.  */
>>> -         if (INTEGRAL_TYPE_P (type))
>>> +         if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>            return build2_loc (loc, TREE_CODE (op0), type,
>>>                               TREE_OPERAND (op0, 0),
>>>                               TREE_OPERAND (op0, 1));
>>> -         else
>>> +         else if (!INTEGRAL_TYPE_P (type))
>>>            return build3_loc (loc, COND_EXPR, type, op0,
>>>                               fold_convert (type, boolean_true_node),
>>>                               fold_convert (type, boolean_false_node));
>>> Index: gcc-head/gcc/gimplify.c
>>> ===================================================================
>>> --- gcc-head.orig/gcc/gimplify.c
>>> +++ gcc-head/gcc/gimplify.c
>>> @@ -2842,18 +2842,23 @@ gimple_boolify (tree expr)
>>>
>>>     case TRUTH_NOT_EXPR:
>>>       TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
>>> -      /* FALLTHRU */
>>>
>>> -    case EQ_EXPR: case NE_EXPR:
>>> -    case LE_EXPR: case GE_EXPR: case LT_EXPR: case GT_EXPR:
>>>       /* These expressions always produce boolean results.  */
>>> -      TREE_TYPE (expr) = boolean_type_node;
>>> +      if (TREE_CODE (type) != BOOLEAN_TYPE)
>>> +       TREE_TYPE (expr) = boolean_type_node;
>>>       return expr;
>>>
>>>     default:
>>> +      if (COMPARISON_CLASS_P (expr))
>>> +       {
>>> +         /* There expressions always prduce boolean results.  */
>>> +         if (TREE_CODE (type) != BOOLEAN_TYPE)
>>> +           TREE_TYPE (expr) = boolean_type_node;
>>> +         return expr;
>>> +       }
>>>       /* Other expressions that get here must have boolean values, but
>>>         might need to be converted to the appropriate mode.  */
>>> -      if (type == boolean_type_node)
>>> +      if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>        return expr;
>>>       return fold_convert_loc (loc, boolean_type_node, expr);
>>>     }
>>> @@ -6763,7 +6768,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>            tree org_type = TREE_TYPE (*expr_p);
>>>
>>>            *expr_p = gimple_boolify (*expr_p);
>>> -           if (org_type != boolean_type_node)
>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>              {
>>>                *expr_p = fold_convert (org_type, *expr_p);
>>
>> Use fold_convert_loc with saved_location
>
> Oh, good catch. Yes, I will adjust that.
>
>>>                ret = GS_OK;
>>> @@ -7208,7 +7213,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>               fold_truth_not_expr) happily uses operand type and doesn't
>>>               automatically uses boolean_type as result, we need to keep
>>>               orignal type.  */
>>> -           if (org_type != boolean_type_node)
>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>              {
>>>                *expr_p = fold_convert (org_type, *expr_p);
>>
>> Likewise.  Maybe this fixes the diagnostic regression.
>>
>>>                ret = GS_OK;
>>> @@ -7288,7 +7293,19 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>                  tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>>
>>>                  if (!AGGREGATE_TYPE_P (type))
>>> -                   goto expr_2;
>>> +                   {
>>> +                     tree org_type = TREE_TYPE (*expr_p);
>>> +                     *expr_p = gimple_boolify (*expr_p);
>>> +                     if (!useless_type_conversion_p (org_type,
>>> +                                                     TREE_TYPE (*expr_p)))
>>> +                       {
>>> +                         *expr_p = fold_convert_loc (saved_location,
>>> +                                                     org_type, *expr_p);
>>> +                         ret = GS_OK;
>>> +                       }
>>> +                     else
>>> +                       goto expr_2;
>>> +                   }
>>>                  else if (TYPE_MODE (type) != BLKmode)
>>>                    ret = gimplify_scalar_mode_aggregate_compare (expr_p);
>>>                  else
>>> Index: gcc-head/gcc/tree-cfg.c
>>> ===================================================================
>>> --- gcc-head.orig/gcc/tree-cfg.c
>>> +++ gcc-head/gcc/tree-cfg.c
>>> @@ -3203,7 +3203,9 @@ verify_gimple_comparison (tree type, tre
>>>        && (!POINTER_TYPE_P (op0_type)
>>>           || !POINTER_TYPE_P (op1_type)
>>>           || TYPE_MODE (op0_type) != TYPE_MODE (op1_type)))
>>> -      || !INTEGRAL_TYPE_P (type))
>>> +      || !INTEGRAL_TYPE_P (type)
>>> +      || (TREE_CODE (type) != BOOLEAN_TYPE
>>> +         && TYPE_PRECISION (type) != 1))
>>>     {
>>>       error ("type mismatch in comparison expression");
>>>       debug_generic_expr (type);
>>> Index: gcc-head/gcc/tree-ssa.c
>>> ===================================================================
>>> --- gcc-head.orig/gcc/tree-ssa.c
>>> +++ gcc-head/gcc/tree-ssa.c
>>> @@ -1306,10 +1306,10 @@ useless_type_conversion_p (tree outer_ty
>>>          || TYPE_PRECISION (inner_type) != TYPE_PRECISION (outer_type))
>>>        return false;
>>>
>>> -      /* Preserve conversions to BOOLEAN_TYPE if it is not of precision
>>> -         one.  */
>>> -      if (TREE_CODE (inner_type) != BOOLEAN_TYPE
>>> -         && TREE_CODE (outer_type) == BOOLEAN_TYPE
>>> +      /* Preserve conversions to/from BOOLEAN_TYPE if types are not
>>> +        of precision one.  */
>>> +      if (((TREE_CODE (inner_type) == BOOLEAN_TYPE)
>>> +          != (TREE_CODE (outer_type) == BOOLEAN_TYPE))
>>>          && TYPE_PRECISION (outer_type) != 1)
>>>        return false;
>>>
>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>> ===================================================================
>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>> @@ -11,5 +11,5 @@ f (int i, float j)
>>>
>>>  /* { dg-final { scan-tree-dump-times { if } 2 "forwprop1"} } */
>>>  /* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 0\);\n[^\n]*if}
>>> "forwprop1"} } */
>>> -/* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 1\);\n[^\n]*if}
>>> "forwprop1"} } */
>>> +/* { dg-final { scan-tree-dump-not {builtin_expect[^\n]*,
>>> 1\);\n[^\n]*if} "forwprop1"} } */
>>
>> Hm?  Why that?
>>
>>>  /* { dg-final { cleanup-tree-dump "forwprop?" } } */
>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>> ===================================================================
>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>> @@ -16,5 +16,5 @@ foo (int a)
>>>     return 0;
>>>  }
>>>
>>> -/* { dg-final { scan-tree-dump-times "Replaced" 2 "forwprop1"} } */
>>> +/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
>>>  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>> ===================================================================
>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>> @@ -10,5 +10,5 @@ int foo(int a)
>>>   return e;
>>>  }
>>>
>>> -/* { dg-final { scan-tree-dump "e_. = a_..D. > 0;" "optimized" } } */
>>> +/* { dg-final { scan-tree-dump " = a_..D. > 0;" "optimized" } } */
>>>  /* { dg-final { cleanup-tree-dump "optimized" } } */
>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>> ===================================================================
>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>> @@ -2,5 +2,5 @@
>>>  /* { dg-options "-O -fdump-tree-fre1-details" } */
>>>
>>>  int i; int foo(void) { i = 2; int j = i * 2; int k = i + 2; return j == k; }
>>> -/* { dg-final { scan-tree-dump-times "Replaced " 5 "fre1" } } */
>>> +/* { dg-final { scan-tree-dump-times "Replaced " 6 "fre1" } } */
>>>  /* { dg-final { cleanup-tree-dump "fre1" } } */
>>> Index: gcc-head/gcc/tree-ssa-forwprop.c
>>> ===================================================================
>>> --- gcc-head.orig/gcc/tree-ssa-forwprop.c
>>> +++ gcc-head/gcc/tree-ssa-forwprop.c
>>> @@ -367,9 +367,61 @@ combine_cond_expr_cond (location_t loc,
>>>   gcc_assert (TREE_CODE_CLASS (code) == tcc_comparison);
>>>
>>>   t = fold_binary_loc (loc, code, type, op0, op1);
>>> +
>>> +  if (!t && INTEGRAL_TYPE_P (TREE_TYPE (op1))
>>> +      && TYPE_PRECISION (TREE_TYPE (op1)) == 1
>>> +      && (code == EQ_EXPR || code == NE_EXPR))
>>> +    {
>>> +      if (TREE_CODE (op1) == INTEGER_CST)
>>> +        {
>>> +         if (integer_onep (op1))
>>> +           {
>>> +             op1 = fold_convert_loc (loc, TREE_TYPE (op1), integer_zero_node);
>>> +             code = (code == NE_EXPR ? EQ_EXPR : NE_EXPR);
>>
>> So you change truthvalue !=/== 1 to truthvalue ==/!= 0 and then
>> recurse ... that doesn't make sense to me and is super-ugly.
>> What's the testcase that made you add all this code?
>
> Well, the convert from truthvalue !=/== 1 to !=/== 0 limits the amount
> of cases to handle. As for truthvalued X the we have then just to
> handle two cases. X != 0 -> X, and X == 0 -> (X ^ 1).
> The recursion is someting I saw as existing pattern (for the same
> thing) in truth-op folding in fold-const.
>
> Actual I can remove this optimization here, as it should be convered
> by VRP already (when VRP handles 1-bit precision bitwise ops proper).

We should have a canonical form for those compares and change
them accordingly, best in fold_stmt.

Richard.

Kai Tietz July 11, 2011, 3:37 p.m. UTC | #4

2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
> On Fri, Jul 8, 2011 at 1:32 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>>> On Thu, Jul 7, 2011 at 6:07 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>> Hello,
>>>>
>>>> This patch - second of series - adds boolification of comparisions in
>>>> gimplifier.  For this
>>>> casts from/to boolean are marked as not-useless. And in fold_unary_loc
>>>> casts to non-boolean integral types are preserved.
>>>> The hunk in tree-ssa-forwprop.c in combine_cond-expr_cond is not strictly
>>>> necessary - as long as fold-const handles 1-bit precision bitwise-expression
>>>> with truth-logic - but it has shown to short-cut some expensier folding. So
>>>> I kept it within this patch.
>>>
>>> Please split it out.  Also ...
>>>
>>>>
>>>> The adjusted testcase gcc.dg/uninit-15.c indicates that due
>>>> optimization we loose
>>>> in this case variables declaration.  But this might be to be expected.
>>>>
>>>> In vectorization we have a regression in gcc.dg/vect/vect-cond-3.c
>>>> test-case.  It's caused
>>>> by always having boolean-type on conditions.  So vectorizer sees
>>>> different types, which
>>>> aren't handled by vectorizer right now.  Maybe this issue could be
>>>> special-cased for
>>>> boolean-types in tree-vect-loop, by making operand for used condition
>>>> equal to vector-type.
>>>> But this is a subject for a different patch and not addressed by this series.
>>>>
>>>> There is a regressions in tree-ssa/vrp47.c, and the fix is addressed
>>>> by the 3rd patch of this
>>>> series.
>>>>
>>>> Bootstrapped and regression tested for all standard-languages (plus
>>>> Ada and Obj-C++) on host x86_64-pc-linux-gnu.
>>>>
>>>> Ok for apply?
>>>>
>>>> Regards,
>>>> Kai
>>>>
>>>>
>>>> ChangeLog
>>>>
>>>> 2011-07-07  Kai Tietz  <ktietz@redhat.com>
>>>>
>>>>        * fold-const.c (fold_unary_loc): Preserve
>>>>        non-boolean-typed casts.
>>>>        * gimplify.c (gimple_boolify): Handle boolification
>>>>        of comparisons.
>>>>        (gimplify_expr): Boolifiy non aggregate-typed
>>>>        comparisons.
>>>>        * tree-cfg.c (verify_gimple_comparison): Check result
>>>>        type of comparison expression.
>>>>        * tree-ssa.c (useless_type_conversion_p): Preserve incompatible
>>>>        casts from/to boolean,
>>>>        * tree-ssa-forwprop.c (combine_cond_expr_cond): Add simplification
>>>>        support for one-bit-precision typed X for cases X != 0 and X == 0.
>>>>        (forward_propagate_comparison): Adjust test of condition
>>>>        result.
>>>>
>>>>
>>>>        * gcc.dg/tree-ssa/builtin-expect-5.c: Adjusted.
>>>>        * gcc.dg/tree-ssa/pr21031.c: Likewise.
>>>>        * gcc.dg/tree-ssa/pr30978.c: Likewise.
>>>>        * gcc.dg/tree-ssa/ssa-fre-6.c: Likewise.
>>>>        * gcc.dg/binop-xor1.c: Mark it as expected fail.
>>>>        * gcc.dg/binop-xor3.c: Likewise.
>>>>        * gcc.dg/uninit-15.c: Adjust reported message.
>>>>
>>>> Index: gcc-head/gcc/fold-const.c
>>>> ===================================================================
>>>> --- gcc-head.orig/gcc/fold-const.c
>>>> +++ gcc-head/gcc/fold-const.c
>>>> @@ -7665,11 +7665,11 @@ fold_unary_loc (location_t loc, enum tre
>>>>             non-integral type.
>>>>             Do not fold the result as that would not simplify further, also
>>>>             folding again results in recursions.  */
>>>> -         if (INTEGRAL_TYPE_P (type))
>>>> +         if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>            return build2_loc (loc, TREE_CODE (op0), type,
>>>>                               TREE_OPERAND (op0, 0),
>>>>                               TREE_OPERAND (op0, 1));
>>>> -         else
>>>> +         else if (!INTEGRAL_TYPE_P (type))
>>>>            return build3_loc (loc, COND_EXPR, type, op0,
>>>>                               fold_convert (type, boolean_true_node),
>>>>                               fold_convert (type, boolean_false_node));
>>>> Index: gcc-head/gcc/gimplify.c
>>>> ===================================================================
>>>> --- gcc-head.orig/gcc/gimplify.c
>>>> +++ gcc-head/gcc/gimplify.c
>>>> @@ -2842,18 +2842,23 @@ gimple_boolify (tree expr)
>>>>
>>>>     case TRUTH_NOT_EXPR:
>>>>       TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
>>>> -      /* FALLTHRU */
>>>>
>>>> -    case EQ_EXPR: case NE_EXPR:
>>>> -    case LE_EXPR: case GE_EXPR: case LT_EXPR: case GT_EXPR:
>>>>       /* These expressions always produce boolean results.  */
>>>> -      TREE_TYPE (expr) = boolean_type_node;
>>>> +      if (TREE_CODE (type) != BOOLEAN_TYPE)
>>>> +       TREE_TYPE (expr) = boolean_type_node;
>>>>       return expr;
>>>>
>>>>     default:
>>>> +      if (COMPARISON_CLASS_P (expr))
>>>> +       {
>>>> +         /* There expressions always prduce boolean results.  */
>>>> +         if (TREE_CODE (type) != BOOLEAN_TYPE)
>>>> +           TREE_TYPE (expr) = boolean_type_node;
>>>> +         return expr;
>>>> +       }
>>>>       /* Other expressions that get here must have boolean values, but
>>>>         might need to be converted to the appropriate mode.  */
>>>> -      if (type == boolean_type_node)
>>>> +      if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>        return expr;
>>>>       return fold_convert_loc (loc, boolean_type_node, expr);
>>>>     }
>>>> @@ -6763,7 +6768,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>            tree org_type = TREE_TYPE (*expr_p);
>>>>
>>>>            *expr_p = gimple_boolify (*expr_p);
>>>> -           if (org_type != boolean_type_node)
>>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>>              {
>>>>                *expr_p = fold_convert (org_type, *expr_p);
>>>
>>> Use fold_convert_loc with saved_location
>>
>> Oh, good catch. Yes, I will adjust that.
>>
>>>>                ret = GS_OK;
>>>> @@ -7208,7 +7213,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>               fold_truth_not_expr) happily uses operand type and doesn't
>>>>               automatically uses boolean_type as result, we need to keep
>>>>               orignal type.  */
>>>> -           if (org_type != boolean_type_node)
>>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>>              {
>>>>                *expr_p = fold_convert (org_type, *expr_p);
>>>
>>> Likewise.  Maybe this fixes the diagnostic regression.
>>>
>>>>                ret = GS_OK;
>>>> @@ -7288,7 +7293,19 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>                  tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>>>
>>>>                  if (!AGGREGATE_TYPE_P (type))
>>>> -                   goto expr_2;
>>>> +                   {
>>>> +                     tree org_type = TREE_TYPE (*expr_p);
>>>> +                     *expr_p = gimple_boolify (*expr_p);
>>>> +                     if (!useless_type_conversion_p (org_type,
>>>> +                                                     TREE_TYPE (*expr_p)))
>>>> +                       {
>>>> +                         *expr_p = fold_convert_loc (saved_location,
>>>> +                                                     org_type, *expr_p);
>>>> +                         ret = GS_OK;
>>>> +                       }
>>>> +                     else
>>>> +                       goto expr_2;
>>>> +                   }
>>>>                  else if (TYPE_MODE (type) != BLKmode)
>>>>                    ret = gimplify_scalar_mode_aggregate_compare (expr_p);
>>>>                  else
>>>> Index: gcc-head/gcc/tree-cfg.c
>>>> ===================================================================
>>>> --- gcc-head.orig/gcc/tree-cfg.c
>>>> +++ gcc-head/gcc/tree-cfg.c
>>>> @@ -3203,7 +3203,9 @@ verify_gimple_comparison (tree type, tre
>>>>        && (!POINTER_TYPE_P (op0_type)
>>>>           || !POINTER_TYPE_P (op1_type)
>>>>           || TYPE_MODE (op0_type) != TYPE_MODE (op1_type)))
>>>> -      || !INTEGRAL_TYPE_P (type))
>>>> +      || !INTEGRAL_TYPE_P (type)
>>>> +      || (TREE_CODE (type) != BOOLEAN_TYPE
>>>> +         && TYPE_PRECISION (type) != 1))
>>>>     {
>>>>       error ("type mismatch in comparison expression");
>>>>       debug_generic_expr (type);
>>>> Index: gcc-head/gcc/tree-ssa.c
>>>> ===================================================================
>>>> --- gcc-head.orig/gcc/tree-ssa.c
>>>> +++ gcc-head/gcc/tree-ssa.c
>>>> @@ -1306,10 +1306,10 @@ useless_type_conversion_p (tree outer_ty
>>>>          || TYPE_PRECISION (inner_type) != TYPE_PRECISION (outer_type))
>>>>        return false;
>>>>
>>>> -      /* Preserve conversions to BOOLEAN_TYPE if it is not of precision
>>>> -         one.  */
>>>> -      if (TREE_CODE (inner_type) != BOOLEAN_TYPE
>>>> -         && TREE_CODE (outer_type) == BOOLEAN_TYPE
>>>> +      /* Preserve conversions to/from BOOLEAN_TYPE if types are not
>>>> +        of precision one.  */
>>>> +      if (((TREE_CODE (inner_type) == BOOLEAN_TYPE)
>>>> +          != (TREE_CODE (outer_type) == BOOLEAN_TYPE))
>>>>          && TYPE_PRECISION (outer_type) != 1)
>>>>        return false;
>>>>
>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>> ===================================================================
>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>> @@ -11,5 +11,5 @@ f (int i, float j)
>>>>
>>>>  /* { dg-final { scan-tree-dump-times { if } 2 "forwprop1"} } */
>>>>  /* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 0\);\n[^\n]*if}
>>>> "forwprop1"} } */
>>>> -/* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 1\);\n[^\n]*if}
>>>> "forwprop1"} } */
>>>> +/* { dg-final { scan-tree-dump-not {builtin_expect[^\n]*,
>>>> 1\);\n[^\n]*if} "forwprop1"} } */
>>>
>>> Hm?  Why that?
>>>
>>>>  /* { dg-final { cleanup-tree-dump "forwprop?" } } */
>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>> ===================================================================
>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>> @@ -16,5 +16,5 @@ foo (int a)
>>>>     return 0;
>>>>  }
>>>>
>>>> -/* { dg-final { scan-tree-dump-times "Replaced" 2 "forwprop1"} } */
>>>> +/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
>>>>  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>> ===================================================================
>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>> @@ -10,5 +10,5 @@ int foo(int a)
>>>>   return e;
>>>>  }
>>>>
>>>> -/* { dg-final { scan-tree-dump "e_. = a_..D. > 0;" "optimized" } } */
>>>> +/* { dg-final { scan-tree-dump " = a_..D. > 0;" "optimized" } } */
>>>>  /* { dg-final { cleanup-tree-dump "optimized" } } */
>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>> ===================================================================
>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>> @@ -2,5 +2,5 @@
>>>>  /* { dg-options "-O -fdump-tree-fre1-details" } */
>>>>
>>>>  int i; int foo(void) { i = 2; int j = i * 2; int k = i + 2; return j == k; }
>>>> -/* { dg-final { scan-tree-dump-times "Replaced " 5 "fre1" } } */
>>>> +/* { dg-final { scan-tree-dump-times "Replaced " 6 "fre1" } } */
>>>>  /* { dg-final { cleanup-tree-dump "fre1" } } */
>>>> Index: gcc-head/gcc/tree-ssa-forwprop.c
>>>> ===================================================================
>>>> --- gcc-head.orig/gcc/tree-ssa-forwprop.c
>>>> +++ gcc-head/gcc/tree-ssa-forwprop.c
>>>> @@ -367,9 +367,61 @@ combine_cond_expr_cond (location_t loc,
>>>>   gcc_assert (TREE_CODE_CLASS (code) == tcc_comparison);
>>>>
>>>>   t = fold_binary_loc (loc, code, type, op0, op1);
>>>> +
>>>> +  if (!t && INTEGRAL_TYPE_P (TREE_TYPE (op1))
>>>> +      && TYPE_PRECISION (TREE_TYPE (op1)) == 1
>>>> +      && (code == EQ_EXPR || code == NE_EXPR))
>>>> +    {
>>>> +      if (TREE_CODE (op1) == INTEGER_CST)
>>>> +        {
>>>> +         if (integer_onep (op1))
>>>> +           {
>>>> +             op1 = fold_convert_loc (loc, TREE_TYPE (op1), integer_zero_node);
>>>> +             code = (code == NE_EXPR ? EQ_EXPR : NE_EXPR);
>>>
>>> So you change truthvalue !=/== 1 to truthvalue ==/!= 0 and then
>>> recurse ... that doesn't make sense to me and is super-ugly.
>>> What's the testcase that made you add all this code?
>>
>> Well, the convert from truthvalue !=/== 1 to !=/== 0 limits the amount
>> of cases to handle. As for truthvalued X the we have then just to
>> handle two cases. X != 0 -> X, and X == 0 -> (X ^ 1).
>> The recursion is someting I saw as existing pattern (for the same
>> thing) in truth-op folding in fold-const.
>>
>> Actual I can remove this optimization here, as it should be convered
>> by VRP already (when VRP handles 1-bit precision bitwise ops proper).
>
> We should have a canonical form for those compares and change
> them accordingly, best in fold_stmt.
>
> Richard.

Hmm, I tried to add this code-pattern to fold_stmt, but for this kind
of branch it seems not to be invoked at all. At least not now without
boolification of compares.  One nit I found for GIMPLE_BINARY, as here
just patterns getting replaced, which have fewer number of ops then
original statement. This check looks a bit bogus.
For getting this normalization right now in a consistant way,
fold-const might be right now the better place to handle this.

Regards,
Kai

Richard Biener July 12, 2011, 9:22 a.m. UTC | #5

On Mon, Jul 11, 2011 at 5:37 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>> On Fri, Jul 8, 2011 at 1:32 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>>>> On Thu, Jul 7, 2011 at 6:07 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>> Hello,
>>>>>
>>>>> This patch - second of series - adds boolification of comparisions in
>>>>> gimplifier.  For this
>>>>> casts from/to boolean are marked as not-useless. And in fold_unary_loc
>>>>> casts to non-boolean integral types are preserved.
>>>>> The hunk in tree-ssa-forwprop.c in combine_cond-expr_cond is not strictly
>>>>> necessary - as long as fold-const handles 1-bit precision bitwise-expression
>>>>> with truth-logic - but it has shown to short-cut some expensier folding. So
>>>>> I kept it within this patch.
>>>>
>>>> Please split it out.  Also ...
>>>>
>>>>>
>>>>> The adjusted testcase gcc.dg/uninit-15.c indicates that due
>>>>> optimization we loose
>>>>> in this case variables declaration.  But this might be to be expected.
>>>>>
>>>>> In vectorization we have a regression in gcc.dg/vect/vect-cond-3.c
>>>>> test-case.  It's caused
>>>>> by always having boolean-type on conditions.  So vectorizer sees
>>>>> different types, which
>>>>> aren't handled by vectorizer right now.  Maybe this issue could be
>>>>> special-cased for
>>>>> boolean-types in tree-vect-loop, by making operand for used condition
>>>>> equal to vector-type.
>>>>> But this is a subject for a different patch and not addressed by this series.
>>>>>
>>>>> There is a regressions in tree-ssa/vrp47.c, and the fix is addressed
>>>>> by the 3rd patch of this
>>>>> series.
>>>>>
>>>>> Bootstrapped and regression tested for all standard-languages (plus
>>>>> Ada and Obj-C++) on host x86_64-pc-linux-gnu.
>>>>>
>>>>> Ok for apply?
>>>>>
>>>>> Regards,
>>>>> Kai
>>>>>
>>>>>
>>>>> ChangeLog
>>>>>
>>>>> 2011-07-07  Kai Tietz  <ktietz@redhat.com>
>>>>>
>>>>>        * fold-const.c (fold_unary_loc): Preserve
>>>>>        non-boolean-typed casts.
>>>>>        * gimplify.c (gimple_boolify): Handle boolification
>>>>>        of comparisons.
>>>>>        (gimplify_expr): Boolifiy non aggregate-typed
>>>>>        comparisons.
>>>>>        * tree-cfg.c (verify_gimple_comparison): Check result
>>>>>        type of comparison expression.
>>>>>        * tree-ssa.c (useless_type_conversion_p): Preserve incompatible
>>>>>        casts from/to boolean,
>>>>>        * tree-ssa-forwprop.c (combine_cond_expr_cond): Add simplification
>>>>>        support for one-bit-precision typed X for cases X != 0 and X == 0.
>>>>>        (forward_propagate_comparison): Adjust test of condition
>>>>>        result.
>>>>>
>>>>>
>>>>>        * gcc.dg/tree-ssa/builtin-expect-5.c: Adjusted.
>>>>>        * gcc.dg/tree-ssa/pr21031.c: Likewise.
>>>>>        * gcc.dg/tree-ssa/pr30978.c: Likewise.
>>>>>        * gcc.dg/tree-ssa/ssa-fre-6.c: Likewise.
>>>>>        * gcc.dg/binop-xor1.c: Mark it as expected fail.
>>>>>        * gcc.dg/binop-xor3.c: Likewise.
>>>>>        * gcc.dg/uninit-15.c: Adjust reported message.
>>>>>
>>>>> Index: gcc-head/gcc/fold-const.c
>>>>> ===================================================================
>>>>> --- gcc-head.orig/gcc/fold-const.c
>>>>> +++ gcc-head/gcc/fold-const.c
>>>>> @@ -7665,11 +7665,11 @@ fold_unary_loc (location_t loc, enum tre
>>>>>             non-integral type.
>>>>>             Do not fold the result as that would not simplify further, also
>>>>>             folding again results in recursions.  */
>>>>> -         if (INTEGRAL_TYPE_P (type))
>>>>> +         if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>            return build2_loc (loc, TREE_CODE (op0), type,
>>>>>                               TREE_OPERAND (op0, 0),
>>>>>                               TREE_OPERAND (op0, 1));
>>>>> -         else
>>>>> +         else if (!INTEGRAL_TYPE_P (type))
>>>>>            return build3_loc (loc, COND_EXPR, type, op0,
>>>>>                               fold_convert (type, boolean_true_node),
>>>>>                               fold_convert (type, boolean_false_node));
>>>>> Index: gcc-head/gcc/gimplify.c
>>>>> ===================================================================
>>>>> --- gcc-head.orig/gcc/gimplify.c
>>>>> +++ gcc-head/gcc/gimplify.c
>>>>> @@ -2842,18 +2842,23 @@ gimple_boolify (tree expr)
>>>>>
>>>>>     case TRUTH_NOT_EXPR:
>>>>>       TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
>>>>> -      /* FALLTHRU */
>>>>>
>>>>> -    case EQ_EXPR: case NE_EXPR:
>>>>> -    case LE_EXPR: case GE_EXPR: case LT_EXPR: case GT_EXPR:
>>>>>       /* These expressions always produce boolean results.  */
>>>>> -      TREE_TYPE (expr) = boolean_type_node;
>>>>> +      if (TREE_CODE (type) != BOOLEAN_TYPE)
>>>>> +       TREE_TYPE (expr) = boolean_type_node;
>>>>>       return expr;
>>>>>
>>>>>     default:
>>>>> +      if (COMPARISON_CLASS_P (expr))
>>>>> +       {
>>>>> +         /* There expressions always prduce boolean results.  */
>>>>> +         if (TREE_CODE (type) != BOOLEAN_TYPE)
>>>>> +           TREE_TYPE (expr) = boolean_type_node;
>>>>> +         return expr;
>>>>> +       }
>>>>>       /* Other expressions that get here must have boolean values, but
>>>>>         might need to be converted to the appropriate mode.  */
>>>>> -      if (type == boolean_type_node)
>>>>> +      if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>        return expr;
>>>>>       return fold_convert_loc (loc, boolean_type_node, expr);
>>>>>     }
>>>>> @@ -6763,7 +6768,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>            tree org_type = TREE_TYPE (*expr_p);
>>>>>
>>>>>            *expr_p = gimple_boolify (*expr_p);
>>>>> -           if (org_type != boolean_type_node)
>>>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>>>              {
>>>>>                *expr_p = fold_convert (org_type, *expr_p);
>>>>
>>>> Use fold_convert_loc with saved_location
>>>
>>> Oh, good catch. Yes, I will adjust that.
>>>
>>>>>                ret = GS_OK;
>>>>> @@ -7208,7 +7213,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>               fold_truth_not_expr) happily uses operand type and doesn't
>>>>>               automatically uses boolean_type as result, we need to keep
>>>>>               orignal type.  */
>>>>> -           if (org_type != boolean_type_node)
>>>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>>>              {
>>>>>                *expr_p = fold_convert (org_type, *expr_p);
>>>>
>>>> Likewise.  Maybe this fixes the diagnostic regression.
>>>>
>>>>>                ret = GS_OK;
>>>>> @@ -7288,7 +7293,19 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>                  tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>>>>
>>>>>                  if (!AGGREGATE_TYPE_P (type))
>>>>> -                   goto expr_2;
>>>>> +                   {
>>>>> +                     tree org_type = TREE_TYPE (*expr_p);
>>>>> +                     *expr_p = gimple_boolify (*expr_p);
>>>>> +                     if (!useless_type_conversion_p (org_type,
>>>>> +                                                     TREE_TYPE (*expr_p)))
>>>>> +                       {
>>>>> +                         *expr_p = fold_convert_loc (saved_location,
>>>>> +                                                     org_type, *expr_p);
>>>>> +                         ret = GS_OK;
>>>>> +                       }
>>>>> +                     else
>>>>> +                       goto expr_2;
>>>>> +                   }
>>>>>                  else if (TYPE_MODE (type) != BLKmode)
>>>>>                    ret = gimplify_scalar_mode_aggregate_compare (expr_p);
>>>>>                  else
>>>>> Index: gcc-head/gcc/tree-cfg.c
>>>>> ===================================================================
>>>>> --- gcc-head.orig/gcc/tree-cfg.c
>>>>> +++ gcc-head/gcc/tree-cfg.c
>>>>> @@ -3203,7 +3203,9 @@ verify_gimple_comparison (tree type, tre
>>>>>        && (!POINTER_TYPE_P (op0_type)
>>>>>           || !POINTER_TYPE_P (op1_type)
>>>>>           || TYPE_MODE (op0_type) != TYPE_MODE (op1_type)))
>>>>> -      || !INTEGRAL_TYPE_P (type))
>>>>> +      || !INTEGRAL_TYPE_P (type)
>>>>> +      || (TREE_CODE (type) != BOOLEAN_TYPE
>>>>> +         && TYPE_PRECISION (type) != 1))
>>>>>     {
>>>>>       error ("type mismatch in comparison expression");
>>>>>       debug_generic_expr (type);
>>>>> Index: gcc-head/gcc/tree-ssa.c
>>>>> ===================================================================
>>>>> --- gcc-head.orig/gcc/tree-ssa.c
>>>>> +++ gcc-head/gcc/tree-ssa.c
>>>>> @@ -1306,10 +1306,10 @@ useless_type_conversion_p (tree outer_ty
>>>>>          || TYPE_PRECISION (inner_type) != TYPE_PRECISION (outer_type))
>>>>>        return false;
>>>>>
>>>>> -      /* Preserve conversions to BOOLEAN_TYPE if it is not of precision
>>>>> -         one.  */
>>>>> -      if (TREE_CODE (inner_type) != BOOLEAN_TYPE
>>>>> -         && TREE_CODE (outer_type) == BOOLEAN_TYPE
>>>>> +      /* Preserve conversions to/from BOOLEAN_TYPE if types are not
>>>>> +        of precision one.  */
>>>>> +      if (((TREE_CODE (inner_type) == BOOLEAN_TYPE)
>>>>> +          != (TREE_CODE (outer_type) == BOOLEAN_TYPE))
>>>>>          && TYPE_PRECISION (outer_type) != 1)
>>>>>        return false;
>>>>>
>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>> ===================================================================
>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>> @@ -11,5 +11,5 @@ f (int i, float j)
>>>>>
>>>>>  /* { dg-final { scan-tree-dump-times { if } 2 "forwprop1"} } */
>>>>>  /* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 0\);\n[^\n]*if}
>>>>> "forwprop1"} } */
>>>>> -/* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 1\);\n[^\n]*if}
>>>>> "forwprop1"} } */
>>>>> +/* { dg-final { scan-tree-dump-not {builtin_expect[^\n]*,
>>>>> 1\);\n[^\n]*if} "forwprop1"} } */
>>>>
>>>> Hm?  Why that?
>>>>
>>>>>  /* { dg-final { cleanup-tree-dump "forwprop?" } } */
>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>> ===================================================================
>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>> @@ -16,5 +16,5 @@ foo (int a)
>>>>>     return 0;
>>>>>  }
>>>>>
>>>>> -/* { dg-final { scan-tree-dump-times "Replaced" 2 "forwprop1"} } */
>>>>> +/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
>>>>>  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>> ===================================================================
>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>> @@ -10,5 +10,5 @@ int foo(int a)
>>>>>   return e;
>>>>>  }
>>>>>
>>>>> -/* { dg-final { scan-tree-dump "e_. = a_..D. > 0;" "optimized" } } */
>>>>> +/* { dg-final { scan-tree-dump " = a_..D. > 0;" "optimized" } } */
>>>>>  /* { dg-final { cleanup-tree-dump "optimized" } } */
>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>> ===================================================================
>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>> @@ -2,5 +2,5 @@
>>>>>  /* { dg-options "-O -fdump-tree-fre1-details" } */
>>>>>
>>>>>  int i; int foo(void) { i = 2; int j = i * 2; int k = i + 2; return j == k; }
>>>>> -/* { dg-final { scan-tree-dump-times "Replaced " 5 "fre1" } } */
>>>>> +/* { dg-final { scan-tree-dump-times "Replaced " 6 "fre1" } } */
>>>>>  /* { dg-final { cleanup-tree-dump "fre1" } } */
>>>>> Index: gcc-head/gcc/tree-ssa-forwprop.c
>>>>> ===================================================================
>>>>> --- gcc-head.orig/gcc/tree-ssa-forwprop.c
>>>>> +++ gcc-head/gcc/tree-ssa-forwprop.c
>>>>> @@ -367,9 +367,61 @@ combine_cond_expr_cond (location_t loc,
>>>>>   gcc_assert (TREE_CODE_CLASS (code) == tcc_comparison);
>>>>>
>>>>>   t = fold_binary_loc (loc, code, type, op0, op1);
>>>>> +
>>>>> +  if (!t && INTEGRAL_TYPE_P (TREE_TYPE (op1))
>>>>> +      && TYPE_PRECISION (TREE_TYPE (op1)) == 1
>>>>> +      && (code == EQ_EXPR || code == NE_EXPR))
>>>>> +    {
>>>>> +      if (TREE_CODE (op1) == INTEGER_CST)
>>>>> +        {
>>>>> +         if (integer_onep (op1))
>>>>> +           {
>>>>> +             op1 = fold_convert_loc (loc, TREE_TYPE (op1), integer_zero_node);
>>>>> +             code = (code == NE_EXPR ? EQ_EXPR : NE_EXPR);
>>>>
>>>> So you change truthvalue !=/== 1 to truthvalue ==/!= 0 and then
>>>> recurse ... that doesn't make sense to me and is super-ugly.
>>>> What's the testcase that made you add all this code?
>>>
>>> Well, the convert from truthvalue !=/== 1 to !=/== 0 limits the amount
>>> of cases to handle. As for truthvalued X the we have then just to
>>> handle two cases. X != 0 -> X, and X == 0 -> (X ^ 1).
>>> The recursion is someting I saw as existing pattern (for the same
>>> thing) in truth-op folding in fold-const.
>>>
>>> Actual I can remove this optimization here, as it should be convered
>>> by VRP already (when VRP handles 1-bit precision bitwise ops proper).
>>
>> We should have a canonical form for those compares and change
>> them accordingly, best in fold_stmt.
>>
>> Richard.
>
> Hmm, I tried to add this code-pattern to fold_stmt, but for this kind
> of branch it seems not to be invoked at all. At least not now without
> boolification of compares.  One nit I found for GIMPLE_BINARY, as here
> just patterns getting replaced, which have fewer number of ops then
> original statement. This check looks a bit bogus.
> For getting this normalization right now in a consistant way,
> fold-const might be right now the better place to handle this.

It gets invoked (well, should get invoked) when anyone changes the
statement.  If it is present in that form from the beginning then we
lack canonicalization in fold and/or gimplification.

I suppose you have a testcase?

Richard.

> Regards,
> Kai
>

Kai Tietz July 12, 2011, 9:48 a.m. UTC | #6

2011/7/12 Richard Guenther <richard.guenther@gmail.com>:
> On Mon, Jul 11, 2011 at 5:37 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>>> On Fri, Jul 8, 2011 at 1:32 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>>>>> On Thu, Jul 7, 2011 at 6:07 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>>> Hello,
>>>>>>
>>>>>> This patch - second of series - adds boolification of comparisions in
>>>>>> gimplifier.  For this
>>>>>> casts from/to boolean are marked as not-useless. And in fold_unary_loc
>>>>>> casts to non-boolean integral types are preserved.
>>>>>> The hunk in tree-ssa-forwprop.c in combine_cond-expr_cond is not strictly
>>>>>> necessary - as long as fold-const handles 1-bit precision bitwise-expression
>>>>>> with truth-logic - but it has shown to short-cut some expensier folding. So
>>>>>> I kept it within this patch.
>>>>>
>>>>> Please split it out.  Also ...
>>>>>
>>>>>>
>>>>>> The adjusted testcase gcc.dg/uninit-15.c indicates that due
>>>>>> optimization we loose
>>>>>> in this case variables declaration.  But this might be to be expected.
>>>>>>
>>>>>> In vectorization we have a regression in gcc.dg/vect/vect-cond-3.c
>>>>>> test-case.  It's caused
>>>>>> by always having boolean-type on conditions.  So vectorizer sees
>>>>>> different types, which
>>>>>> aren't handled by vectorizer right now.  Maybe this issue could be
>>>>>> special-cased for
>>>>>> boolean-types in tree-vect-loop, by making operand for used condition
>>>>>> equal to vector-type.
>>>>>> But this is a subject for a different patch and not addressed by this series.
>>>>>>
>>>>>> There is a regressions in tree-ssa/vrp47.c, and the fix is addressed
>>>>>> by the 3rd patch of this
>>>>>> series.
>>>>>>
>>>>>> Bootstrapped and regression tested for all standard-languages (plus
>>>>>> Ada and Obj-C++) on host x86_64-pc-linux-gnu.
>>>>>>
>>>>>> Ok for apply?
>>>>>>
>>>>>> Regards,
>>>>>> Kai
>>>>>>
>>>>>>
>>>>>> ChangeLog
>>>>>>
>>>>>> 2011-07-07  Kai Tietz  <ktietz@redhat.com>
>>>>>>
>>>>>>        * fold-const.c (fold_unary_loc): Preserve
>>>>>>        non-boolean-typed casts.
>>>>>>        * gimplify.c (gimple_boolify): Handle boolification
>>>>>>        of comparisons.
>>>>>>        (gimplify_expr): Boolifiy non aggregate-typed
>>>>>>        comparisons.
>>>>>>        * tree-cfg.c (verify_gimple_comparison): Check result
>>>>>>        type of comparison expression.
>>>>>>        * tree-ssa.c (useless_type_conversion_p): Preserve incompatible
>>>>>>        casts from/to boolean,
>>>>>>        * tree-ssa-forwprop.c (combine_cond_expr_cond): Add simplification
>>>>>>        support for one-bit-precision typed X for cases X != 0 and X == 0.
>>>>>>        (forward_propagate_comparison): Adjust test of condition
>>>>>>        result.
>>>>>>
>>>>>>
>>>>>>        * gcc.dg/tree-ssa/builtin-expect-5.c: Adjusted.
>>>>>>        * gcc.dg/tree-ssa/pr21031.c: Likewise.
>>>>>>        * gcc.dg/tree-ssa/pr30978.c: Likewise.
>>>>>>        * gcc.dg/tree-ssa/ssa-fre-6.c: Likewise.
>>>>>>        * gcc.dg/binop-xor1.c: Mark it as expected fail.
>>>>>>        * gcc.dg/binop-xor3.c: Likewise.
>>>>>>        * gcc.dg/uninit-15.c: Adjust reported message.
>>>>>>
>>>>>> Index: gcc-head/gcc/fold-const.c
>>>>>> ===================================================================
>>>>>> --- gcc-head.orig/gcc/fold-const.c
>>>>>> +++ gcc-head/gcc/fold-const.c
>>>>>> @@ -7665,11 +7665,11 @@ fold_unary_loc (location_t loc, enum tre
>>>>>>             non-integral type.
>>>>>>             Do not fold the result as that would not simplify further, also
>>>>>>             folding again results in recursions.  */
>>>>>> -         if (INTEGRAL_TYPE_P (type))
>>>>>> +         if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>>            return build2_loc (loc, TREE_CODE (op0), type,
>>>>>>                               TREE_OPERAND (op0, 0),
>>>>>>                               TREE_OPERAND (op0, 1));
>>>>>> -         else
>>>>>> +         else if (!INTEGRAL_TYPE_P (type))
>>>>>>            return build3_loc (loc, COND_EXPR, type, op0,
>>>>>>                               fold_convert (type, boolean_true_node),
>>>>>>                               fold_convert (type, boolean_false_node));
>>>>>> Index: gcc-head/gcc/gimplify.c
>>>>>> ===================================================================
>>>>>> --- gcc-head.orig/gcc/gimplify.c
>>>>>> +++ gcc-head/gcc/gimplify.c
>>>>>> @@ -2842,18 +2842,23 @@ gimple_boolify (tree expr)
>>>>>>
>>>>>>     case TRUTH_NOT_EXPR:
>>>>>>       TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
>>>>>> -      /* FALLTHRU */
>>>>>>
>>>>>> -    case EQ_EXPR: case NE_EXPR:
>>>>>> -    case LE_EXPR: case GE_EXPR: case LT_EXPR: case GT_EXPR:
>>>>>>       /* These expressions always produce boolean results.  */
>>>>>> -      TREE_TYPE (expr) = boolean_type_node;
>>>>>> +      if (TREE_CODE (type) != BOOLEAN_TYPE)
>>>>>> +       TREE_TYPE (expr) = boolean_type_node;
>>>>>>       return expr;
>>>>>>
>>>>>>     default:
>>>>>> +      if (COMPARISON_CLASS_P (expr))
>>>>>> +       {
>>>>>> +         /* There expressions always prduce boolean results.  */
>>>>>> +         if (TREE_CODE (type) != BOOLEAN_TYPE)
>>>>>> +           TREE_TYPE (expr) = boolean_type_node;
>>>>>> +         return expr;
>>>>>> +       }
>>>>>>       /* Other expressions that get here must have boolean values, but
>>>>>>         might need to be converted to the appropriate mode.  */
>>>>>> -      if (type == boolean_type_node)
>>>>>> +      if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>>        return expr;
>>>>>>       return fold_convert_loc (loc, boolean_type_node, expr);
>>>>>>     }
>>>>>> @@ -6763,7 +6768,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>            tree org_type = TREE_TYPE (*expr_p);
>>>>>>
>>>>>>            *expr_p = gimple_boolify (*expr_p);
>>>>>> -           if (org_type != boolean_type_node)
>>>>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>>>>              {
>>>>>>                *expr_p = fold_convert (org_type, *expr_p);
>>>>>
>>>>> Use fold_convert_loc with saved_location
>>>>
>>>> Oh, good catch. Yes, I will adjust that.
>>>>
>>>>>>                ret = GS_OK;
>>>>>> @@ -7208,7 +7213,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>               fold_truth_not_expr) happily uses operand type and doesn't
>>>>>>               automatically uses boolean_type as result, we need to keep
>>>>>>               orignal type.  */
>>>>>> -           if (org_type != boolean_type_node)
>>>>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>>>>              {
>>>>>>                *expr_p = fold_convert (org_type, *expr_p);
>>>>>
>>>>> Likewise.  Maybe this fixes the diagnostic regression.
>>>>>
>>>>>>                ret = GS_OK;
>>>>>> @@ -7288,7 +7293,19 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>                  tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>>>>>
>>>>>>                  if (!AGGREGATE_TYPE_P (type))
>>>>>> -                   goto expr_2;
>>>>>> +                   {
>>>>>> +                     tree org_type = TREE_TYPE (*expr_p);
>>>>>> +                     *expr_p = gimple_boolify (*expr_p);
>>>>>> +                     if (!useless_type_conversion_p (org_type,
>>>>>> +                                                     TREE_TYPE (*expr_p)))
>>>>>> +                       {
>>>>>> +                         *expr_p = fold_convert_loc (saved_location,
>>>>>> +                                                     org_type, *expr_p);
>>>>>> +                         ret = GS_OK;
>>>>>> +                       }
>>>>>> +                     else
>>>>>> +                       goto expr_2;
>>>>>> +                   }
>>>>>>                  else if (TYPE_MODE (type) != BLKmode)
>>>>>>                    ret = gimplify_scalar_mode_aggregate_compare (expr_p);
>>>>>>                  else
>>>>>> Index: gcc-head/gcc/tree-cfg.c
>>>>>> ===================================================================
>>>>>> --- gcc-head.orig/gcc/tree-cfg.c
>>>>>> +++ gcc-head/gcc/tree-cfg.c
>>>>>> @@ -3203,7 +3203,9 @@ verify_gimple_comparison (tree type, tre
>>>>>>        && (!POINTER_TYPE_P (op0_type)
>>>>>>           || !POINTER_TYPE_P (op1_type)
>>>>>>           || TYPE_MODE (op0_type) != TYPE_MODE (op1_type)))
>>>>>> -      || !INTEGRAL_TYPE_P (type))
>>>>>> +      || !INTEGRAL_TYPE_P (type)
>>>>>> +      || (TREE_CODE (type) != BOOLEAN_TYPE
>>>>>> +         && TYPE_PRECISION (type) != 1))
>>>>>>     {
>>>>>>       error ("type mismatch in comparison expression");
>>>>>>       debug_generic_expr (type);
>>>>>> Index: gcc-head/gcc/tree-ssa.c
>>>>>> ===================================================================
>>>>>> --- gcc-head.orig/gcc/tree-ssa.c
>>>>>> +++ gcc-head/gcc/tree-ssa.c
>>>>>> @@ -1306,10 +1306,10 @@ useless_type_conversion_p (tree outer_ty
>>>>>>          || TYPE_PRECISION (inner_type) != TYPE_PRECISION (outer_type))
>>>>>>        return false;
>>>>>>
>>>>>> -      /* Preserve conversions to BOOLEAN_TYPE if it is not of precision
>>>>>> -         one.  */
>>>>>> -      if (TREE_CODE (inner_type) != BOOLEAN_TYPE
>>>>>> -         && TREE_CODE (outer_type) == BOOLEAN_TYPE
>>>>>> +      /* Preserve conversions to/from BOOLEAN_TYPE if types are not
>>>>>> +        of precision one.  */
>>>>>> +      if (((TREE_CODE (inner_type) == BOOLEAN_TYPE)
>>>>>> +          != (TREE_CODE (outer_type) == BOOLEAN_TYPE))
>>>>>>          && TYPE_PRECISION (outer_type) != 1)
>>>>>>        return false;
>>>>>>
>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>>> ===================================================================
>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>>> @@ -11,5 +11,5 @@ f (int i, float j)
>>>>>>
>>>>>>  /* { dg-final { scan-tree-dump-times { if } 2 "forwprop1"} } */
>>>>>>  /* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 0\);\n[^\n]*if}
>>>>>> "forwprop1"} } */
>>>>>> -/* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 1\);\n[^\n]*if}
>>>>>> "forwprop1"} } */
>>>>>> +/* { dg-final { scan-tree-dump-not {builtin_expect[^\n]*,
>>>>>> 1\);\n[^\n]*if} "forwprop1"} } */
>>>>>
>>>>> Hm?  Why that?
>>>>>
>>>>>>  /* { dg-final { cleanup-tree-dump "forwprop?" } } */
>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>>> ===================================================================
>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>>> @@ -16,5 +16,5 @@ foo (int a)
>>>>>>     return 0;
>>>>>>  }
>>>>>>
>>>>>> -/* { dg-final { scan-tree-dump-times "Replaced" 2 "forwprop1"} } */
>>>>>> +/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
>>>>>>  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>>> ===================================================================
>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>>> @@ -10,5 +10,5 @@ int foo(int a)
>>>>>>   return e;
>>>>>>  }
>>>>>>
>>>>>> -/* { dg-final { scan-tree-dump "e_. = a_..D. > 0;" "optimized" } } */
>>>>>> +/* { dg-final { scan-tree-dump " = a_..D. > 0;" "optimized" } } */
>>>>>>  /* { dg-final { cleanup-tree-dump "optimized" } } */
>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>>> ===================================================================
>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>>> @@ -2,5 +2,5 @@
>>>>>>  /* { dg-options "-O -fdump-tree-fre1-details" } */
>>>>>>
>>>>>>  int i; int foo(void) { i = 2; int j = i * 2; int k = i + 2; return j == k; }
>>>>>> -/* { dg-final { scan-tree-dump-times "Replaced " 5 "fre1" } } */
>>>>>> +/* { dg-final { scan-tree-dump-times "Replaced " 6 "fre1" } } */
>>>>>>  /* { dg-final { cleanup-tree-dump "fre1" } } */
>>>>>> Index: gcc-head/gcc/tree-ssa-forwprop.c
>>>>>> ===================================================================
>>>>>> --- gcc-head.orig/gcc/tree-ssa-forwprop.c
>>>>>> +++ gcc-head/gcc/tree-ssa-forwprop.c
>>>>>> @@ -367,9 +367,61 @@ combine_cond_expr_cond (location_t loc,
>>>>>>   gcc_assert (TREE_CODE_CLASS (code) == tcc_comparison);
>>>>>>
>>>>>>   t = fold_binary_loc (loc, code, type, op0, op1);
>>>>>> +
>>>>>> +  if (!t && INTEGRAL_TYPE_P (TREE_TYPE (op1))
>>>>>> +      && TYPE_PRECISION (TREE_TYPE (op1)) == 1
>>>>>> +      && (code == EQ_EXPR || code == NE_EXPR))
>>>>>> +    {
>>>>>> +      if (TREE_CODE (op1) == INTEGER_CST)
>>>>>> +        {
>>>>>> +         if (integer_onep (op1))
>>>>>> +           {
>>>>>> +             op1 = fold_convert_loc (loc, TREE_TYPE (op1), integer_zero_node);
>>>>>> +             code = (code == NE_EXPR ? EQ_EXPR : NE_EXPR);
>>>>>
>>>>> So you change truthvalue !=/== 1 to truthvalue ==/!= 0 and then
>>>>> recurse ... that doesn't make sense to me and is super-ugly.
>>>>> What's the testcase that made you add all this code?
>>>>
>>>> Well, the convert from truthvalue !=/== 1 to !=/== 0 limits the amount
>>>> of cases to handle. As for truthvalued X the we have then just to
>>>> handle two cases. X != 0 -> X, and X == 0 -> (X ^ 1).
>>>> The recursion is someting I saw as existing pattern (for the same
>>>> thing) in truth-op folding in fold-const.
>>>>
>>>> Actual I can remove this optimization here, as it should be convered
>>>> by VRP already (when VRP handles 1-bit precision bitwise ops proper).
>>>
>>> We should have a canonical form for those compares and change
>>> them accordingly, best in fold_stmt.
>>>
>>> Richard.
>>
>> Hmm, I tried to add this code-pattern to fold_stmt, but for this kind
>> of branch it seems not to be invoked at all. At least not now without
>> boolification of compares.  One nit I found for GIMPLE_BINARY, as here
>> just patterns getting replaced, which have fewer number of ops then
>> original statement. This check looks a bit bogus.
>> For getting this normalization right now in a consistant way,
>> fold-const might be right now the better place to handle this.
>
> It gets invoked (well, should get invoked) when anyone changes the
> statement.  If it is present in that form from the beginning then we
> lack canonicalization in fold and/or gimplification.
>
> I suppose you have a testcase?
>
> Richard.
>
>> Regards,
>> Kai
>>

Yes, testcase looks like this:

int foo (_Bool a, _Bool b)
{
  return a != ((b | !b));
}

while reduces to
...
return a != 1;  right now, and not a == 0 (as a is boolean)

Richard Biener July 12, 2011, 10:24 a.m. UTC | #7

On Tue, Jul 12, 2011 at 11:48 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/7/12 Richard Guenther <richard.guenther@gmail.com>:
>> On Mon, Jul 11, 2011 at 5:37 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>>>> On Fri, Jul 8, 2011 at 1:32 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>>>>>> On Thu, Jul 7, 2011 at 6:07 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> This patch - second of series - adds boolification of comparisions in
>>>>>>> gimplifier.  For this
>>>>>>> casts from/to boolean are marked as not-useless. And in fold_unary_loc
>>>>>>> casts to non-boolean integral types are preserved.
>>>>>>> The hunk in tree-ssa-forwprop.c in combine_cond-expr_cond is not strictly
>>>>>>> necessary - as long as fold-const handles 1-bit precision bitwise-expression
>>>>>>> with truth-logic - but it has shown to short-cut some expensier folding. So
>>>>>>> I kept it within this patch.
>>>>>>
>>>>>> Please split it out.  Also ...
>>>>>>
>>>>>>>
>>>>>>> The adjusted testcase gcc.dg/uninit-15.c indicates that due
>>>>>>> optimization we loose
>>>>>>> in this case variables declaration.  But this might be to be expected.
>>>>>>>
>>>>>>> In vectorization we have a regression in gcc.dg/vect/vect-cond-3.c
>>>>>>> test-case.  It's caused
>>>>>>> by always having boolean-type on conditions.  So vectorizer sees
>>>>>>> different types, which
>>>>>>> aren't handled by vectorizer right now.  Maybe this issue could be
>>>>>>> special-cased for
>>>>>>> boolean-types in tree-vect-loop, by making operand for used condition
>>>>>>> equal to vector-type.
>>>>>>> But this is a subject for a different patch and not addressed by this series.
>>>>>>>
>>>>>>> There is a regressions in tree-ssa/vrp47.c, and the fix is addressed
>>>>>>> by the 3rd patch of this
>>>>>>> series.
>>>>>>>
>>>>>>> Bootstrapped and regression tested for all standard-languages (plus
>>>>>>> Ada and Obj-C++) on host x86_64-pc-linux-gnu.
>>>>>>>
>>>>>>> Ok for apply?
>>>>>>>
>>>>>>> Regards,
>>>>>>> Kai
>>>>>>>
>>>>>>>
>>>>>>> ChangeLog
>>>>>>>
>>>>>>> 2011-07-07  Kai Tietz  <ktietz@redhat.com>
>>>>>>>
>>>>>>>        * fold-const.c (fold_unary_loc): Preserve
>>>>>>>        non-boolean-typed casts.
>>>>>>>        * gimplify.c (gimple_boolify): Handle boolification
>>>>>>>        of comparisons.
>>>>>>>        (gimplify_expr): Boolifiy non aggregate-typed
>>>>>>>        comparisons.
>>>>>>>        * tree-cfg.c (verify_gimple_comparison): Check result
>>>>>>>        type of comparison expression.
>>>>>>>        * tree-ssa.c (useless_type_conversion_p): Preserve incompatible
>>>>>>>        casts from/to boolean,
>>>>>>>        * tree-ssa-forwprop.c (combine_cond_expr_cond): Add simplification
>>>>>>>        support for one-bit-precision typed X for cases X != 0 and X == 0.
>>>>>>>        (forward_propagate_comparison): Adjust test of condition
>>>>>>>        result.
>>>>>>>
>>>>>>>
>>>>>>>        * gcc.dg/tree-ssa/builtin-expect-5.c: Adjusted.
>>>>>>>        * gcc.dg/tree-ssa/pr21031.c: Likewise.
>>>>>>>        * gcc.dg/tree-ssa/pr30978.c: Likewise.
>>>>>>>        * gcc.dg/tree-ssa/ssa-fre-6.c: Likewise.
>>>>>>>        * gcc.dg/binop-xor1.c: Mark it as expected fail.
>>>>>>>        * gcc.dg/binop-xor3.c: Likewise.
>>>>>>>        * gcc.dg/uninit-15.c: Adjust reported message.
>>>>>>>
>>>>>>> Index: gcc-head/gcc/fold-const.c
>>>>>>> ===================================================================
>>>>>>> --- gcc-head.orig/gcc/fold-const.c
>>>>>>> +++ gcc-head/gcc/fold-const.c
>>>>>>> @@ -7665,11 +7665,11 @@ fold_unary_loc (location_t loc, enum tre
>>>>>>>             non-integral type.
>>>>>>>             Do not fold the result as that would not simplify further, also
>>>>>>>             folding again results in recursions.  */
>>>>>>> -         if (INTEGRAL_TYPE_P (type))
>>>>>>> +         if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>>>            return build2_loc (loc, TREE_CODE (op0), type,
>>>>>>>                               TREE_OPERAND (op0, 0),
>>>>>>>                               TREE_OPERAND (op0, 1));
>>>>>>> -         else
>>>>>>> +         else if (!INTEGRAL_TYPE_P (type))
>>>>>>>            return build3_loc (loc, COND_EXPR, type, op0,
>>>>>>>                               fold_convert (type, boolean_true_node),
>>>>>>>                               fold_convert (type, boolean_false_node));
>>>>>>> Index: gcc-head/gcc/gimplify.c
>>>>>>> ===================================================================
>>>>>>> --- gcc-head.orig/gcc/gimplify.c
>>>>>>> +++ gcc-head/gcc/gimplify.c
>>>>>>> @@ -2842,18 +2842,23 @@ gimple_boolify (tree expr)
>>>>>>>
>>>>>>>     case TRUTH_NOT_EXPR:
>>>>>>>       TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
>>>>>>> -      /* FALLTHRU */
>>>>>>>
>>>>>>> -    case EQ_EXPR: case NE_EXPR:
>>>>>>> -    case LE_EXPR: case GE_EXPR: case LT_EXPR: case GT_EXPR:
>>>>>>>       /* These expressions always produce boolean results.  */
>>>>>>> -      TREE_TYPE (expr) = boolean_type_node;
>>>>>>> +      if (TREE_CODE (type) != BOOLEAN_TYPE)
>>>>>>> +       TREE_TYPE (expr) = boolean_type_node;
>>>>>>>       return expr;
>>>>>>>
>>>>>>>     default:
>>>>>>> +      if (COMPARISON_CLASS_P (expr))
>>>>>>> +       {
>>>>>>> +         /* There expressions always prduce boolean results.  */
>>>>>>> +         if (TREE_CODE (type) != BOOLEAN_TYPE)
>>>>>>> +           TREE_TYPE (expr) = boolean_type_node;
>>>>>>> +         return expr;
>>>>>>> +       }
>>>>>>>       /* Other expressions that get here must have boolean values, but
>>>>>>>         might need to be converted to the appropriate mode.  */
>>>>>>> -      if (type == boolean_type_node)
>>>>>>> +      if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>>>        return expr;
>>>>>>>       return fold_convert_loc (loc, boolean_type_node, expr);
>>>>>>>     }
>>>>>>> @@ -6763,7 +6768,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>>            tree org_type = TREE_TYPE (*expr_p);
>>>>>>>
>>>>>>>            *expr_p = gimple_boolify (*expr_p);
>>>>>>> -           if (org_type != boolean_type_node)
>>>>>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>>>>>              {
>>>>>>>                *expr_p = fold_convert (org_type, *expr_p);
>>>>>>
>>>>>> Use fold_convert_loc with saved_location
>>>>>
>>>>> Oh, good catch. Yes, I will adjust that.
>>>>>
>>>>>>>                ret = GS_OK;
>>>>>>> @@ -7208,7 +7213,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>>               fold_truth_not_expr) happily uses operand type and doesn't
>>>>>>>               automatically uses boolean_type as result, we need to keep
>>>>>>>               orignal type.  */
>>>>>>> -           if (org_type != boolean_type_node)
>>>>>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>>>>>              {
>>>>>>>                *expr_p = fold_convert (org_type, *expr_p);
>>>>>>
>>>>>> Likewise.  Maybe this fixes the diagnostic regression.
>>>>>>
>>>>>>>                ret = GS_OK;
>>>>>>> @@ -7288,7 +7293,19 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>>                  tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>>>>>>
>>>>>>>                  if (!AGGREGATE_TYPE_P (type))
>>>>>>> -                   goto expr_2;
>>>>>>> +                   {
>>>>>>> +                     tree org_type = TREE_TYPE (*expr_p);
>>>>>>> +                     *expr_p = gimple_boolify (*expr_p);
>>>>>>> +                     if (!useless_type_conversion_p (org_type,
>>>>>>> +                                                     TREE_TYPE (*expr_p)))
>>>>>>> +                       {
>>>>>>> +                         *expr_p = fold_convert_loc (saved_location,
>>>>>>> +                                                     org_type, *expr_p);
>>>>>>> +                         ret = GS_OK;
>>>>>>> +                       }
>>>>>>> +                     else
>>>>>>> +                       goto expr_2;
>>>>>>> +                   }
>>>>>>>                  else if (TYPE_MODE (type) != BLKmode)
>>>>>>>                    ret = gimplify_scalar_mode_aggregate_compare (expr_p);
>>>>>>>                  else
>>>>>>> Index: gcc-head/gcc/tree-cfg.c
>>>>>>> ===================================================================
>>>>>>> --- gcc-head.orig/gcc/tree-cfg.c
>>>>>>> +++ gcc-head/gcc/tree-cfg.c
>>>>>>> @@ -3203,7 +3203,9 @@ verify_gimple_comparison (tree type, tre
>>>>>>>        && (!POINTER_TYPE_P (op0_type)
>>>>>>>           || !POINTER_TYPE_P (op1_type)
>>>>>>>           || TYPE_MODE (op0_type) != TYPE_MODE (op1_type)))
>>>>>>> -      || !INTEGRAL_TYPE_P (type))
>>>>>>> +      || !INTEGRAL_TYPE_P (type)
>>>>>>> +      || (TREE_CODE (type) != BOOLEAN_TYPE
>>>>>>> +         && TYPE_PRECISION (type) != 1))
>>>>>>>     {
>>>>>>>       error ("type mismatch in comparison expression");
>>>>>>>       debug_generic_expr (type);
>>>>>>> Index: gcc-head/gcc/tree-ssa.c
>>>>>>> ===================================================================
>>>>>>> --- gcc-head.orig/gcc/tree-ssa.c
>>>>>>> +++ gcc-head/gcc/tree-ssa.c
>>>>>>> @@ -1306,10 +1306,10 @@ useless_type_conversion_p (tree outer_ty
>>>>>>>          || TYPE_PRECISION (inner_type) != TYPE_PRECISION (outer_type))
>>>>>>>        return false;
>>>>>>>
>>>>>>> -      /* Preserve conversions to BOOLEAN_TYPE if it is not of precision
>>>>>>> -         one.  */
>>>>>>> -      if (TREE_CODE (inner_type) != BOOLEAN_TYPE
>>>>>>> -         && TREE_CODE (outer_type) == BOOLEAN_TYPE
>>>>>>> +      /* Preserve conversions to/from BOOLEAN_TYPE if types are not
>>>>>>> +        of precision one.  */
>>>>>>> +      if (((TREE_CODE (inner_type) == BOOLEAN_TYPE)
>>>>>>> +          != (TREE_CODE (outer_type) == BOOLEAN_TYPE))
>>>>>>>          && TYPE_PRECISION (outer_type) != 1)
>>>>>>>        return false;
>>>>>>>
>>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>>>> ===================================================================
>>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>>>> @@ -11,5 +11,5 @@ f (int i, float j)
>>>>>>>
>>>>>>>  /* { dg-final { scan-tree-dump-times { if } 2 "forwprop1"} } */
>>>>>>>  /* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 0\);\n[^\n]*if}
>>>>>>> "forwprop1"} } */
>>>>>>> -/* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 1\);\n[^\n]*if}
>>>>>>> "forwprop1"} } */
>>>>>>> +/* { dg-final { scan-tree-dump-not {builtin_expect[^\n]*,
>>>>>>> 1\);\n[^\n]*if} "forwprop1"} } */
>>>>>>
>>>>>> Hm?  Why that?
>>>>>>
>>>>>>>  /* { dg-final { cleanup-tree-dump "forwprop?" } } */
>>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>>>> ===================================================================
>>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>>>> @@ -16,5 +16,5 @@ foo (int a)
>>>>>>>     return 0;
>>>>>>>  }
>>>>>>>
>>>>>>> -/* { dg-final { scan-tree-dump-times "Replaced" 2 "forwprop1"} } */
>>>>>>> +/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
>>>>>>>  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
>>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>>>> ===================================================================
>>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>>>> @@ -10,5 +10,5 @@ int foo(int a)
>>>>>>>   return e;
>>>>>>>  }
>>>>>>>
>>>>>>> -/* { dg-final { scan-tree-dump "e_. = a_..D. > 0;" "optimized" } } */
>>>>>>> +/* { dg-final { scan-tree-dump " = a_..D. > 0;" "optimized" } } */
>>>>>>>  /* { dg-final { cleanup-tree-dump "optimized" } } */
>>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>>>> ===================================================================
>>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>>>> @@ -2,5 +2,5 @@
>>>>>>>  /* { dg-options "-O -fdump-tree-fre1-details" } */
>>>>>>>
>>>>>>>  int i; int foo(void) { i = 2; int j = i * 2; int k = i + 2; return j == k; }
>>>>>>> -/* { dg-final { scan-tree-dump-times "Replaced " 5 "fre1" } } */
>>>>>>> +/* { dg-final { scan-tree-dump-times "Replaced " 6 "fre1" } } */
>>>>>>>  /* { dg-final { cleanup-tree-dump "fre1" } } */
>>>>>>> Index: gcc-head/gcc/tree-ssa-forwprop.c
>>>>>>> ===================================================================
>>>>>>> --- gcc-head.orig/gcc/tree-ssa-forwprop.c
>>>>>>> +++ gcc-head/gcc/tree-ssa-forwprop.c
>>>>>>> @@ -367,9 +367,61 @@ combine_cond_expr_cond (location_t loc,
>>>>>>>   gcc_assert (TREE_CODE_CLASS (code) == tcc_comparison);
>>>>>>>
>>>>>>>   t = fold_binary_loc (loc, code, type, op0, op1);
>>>>>>> +
>>>>>>> +  if (!t && INTEGRAL_TYPE_P (TREE_TYPE (op1))
>>>>>>> +      && TYPE_PRECISION (TREE_TYPE (op1)) == 1
>>>>>>> +      && (code == EQ_EXPR || code == NE_EXPR))
>>>>>>> +    {
>>>>>>> +      if (TREE_CODE (op1) == INTEGER_CST)
>>>>>>> +        {
>>>>>>> +         if (integer_onep (op1))
>>>>>>> +           {
>>>>>>> +             op1 = fold_convert_loc (loc, TREE_TYPE (op1), integer_zero_node);
>>>>>>> +             code = (code == NE_EXPR ? EQ_EXPR : NE_EXPR);
>>>>>>
>>>>>> So you change truthvalue !=/== 1 to truthvalue ==/!= 0 and then
>>>>>> recurse ... that doesn't make sense to me and is super-ugly.
>>>>>> What's the testcase that made you add all this code?
>>>>>
>>>>> Well, the convert from truthvalue !=/== 1 to !=/== 0 limits the amount
>>>>> of cases to handle. As for truthvalued X the we have then just to
>>>>> handle two cases. X != 0 -> X, and X == 0 -> (X ^ 1).
>>>>> The recursion is someting I saw as existing pattern (for the same
>>>>> thing) in truth-op folding in fold-const.
>>>>>
>>>>> Actual I can remove this optimization here, as it should be convered
>>>>> by VRP already (when VRP handles 1-bit precision bitwise ops proper).
>>>>
>>>> We should have a canonical form for those compares and change
>>>> them accordingly, best in fold_stmt.
>>>>
>>>> Richard.
>>>
>>> Hmm, I tried to add this code-pattern to fold_stmt, but for this kind
>>> of branch it seems not to be invoked at all. At least not now without
>>> boolification of compares.  One nit I found for GIMPLE_BINARY, as here
>>> just patterns getting replaced, which have fewer number of ops then
>>> original statement. This check looks a bit bogus.
>>> For getting this normalization right now in a consistant way,
>>> fold-const might be right now the better place to handle this.
>>
>> It gets invoked (well, should get invoked) when anyone changes the
>> statement.  If it is present in that form from the beginning then we
>> lack canonicalization in fold and/or gimplification.
>>
>> I suppose you have a testcase?
>>
>> Richard.
>>
>>> Regards,
>>> Kai
>>>
>
> Yes, testcase looks like this:
>
> int foo (_Bool a, _Bool b)
> {
>  return a != ((b | !b));
> }
>
> while reduces to
> ...
> return a != 1;  right now, and not a == 0 (as a is boolean)

fold_stmt should be called when copyprop produces a != 1
via substitute_and_fold:

          /* Replace real uses in the statement.  */
          if (get_value_fn)
            did_replace |= replace_uses_in (stmt, get_value_fn);

          /* If we made a replacement, fold the statement.  */
          if (did_replace)
            fold_stmt (&oldi);

Kai Tietz July 12, 2011, 12:21 p.m. UTC | #8

2011/7/12 Richard Guenther <richard.guenther@gmail.com>:
> On Tue, Jul 12, 2011 at 11:48 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
>> 2011/7/12 Richard Guenther <richard.guenther@gmail.com>:
>>> On Mon, Jul 11, 2011 at 5:37 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>>>>> On Fri, Jul 8, 2011 at 1:32 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>>> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>>>>>>> On Thu, Jul 7, 2011 at 6:07 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> This patch - second of series - adds boolification of comparisions in
>>>>>>>> gimplifier.  For this
>>>>>>>> casts from/to boolean are marked as not-useless. And in fold_unary_loc
>>>>>>>> casts to non-boolean integral types are preserved.
>>>>>>>> The hunk in tree-ssa-forwprop.c in combine_cond-expr_cond is not strictly
>>>>>>>> necessary - as long as fold-const handles 1-bit precision bitwise-expression
>>>>>>>> with truth-logic - but it has shown to short-cut some expensier folding. So
>>>>>>>> I kept it within this patch.
>>>>>>>
>>>>>>> Please split it out.  Also ...
>>>>>>>
>>>>>>>>
>>>>>>>> The adjusted testcase gcc.dg/uninit-15.c indicates that due
>>>>>>>> optimization we loose
>>>>>>>> in this case variables declaration.  But this might be to be expected.
>>>>>>>>
>>>>>>>> In vectorization we have a regression in gcc.dg/vect/vect-cond-3.c
>>>>>>>> test-case.  It's caused
>>>>>>>> by always having boolean-type on conditions.  So vectorizer sees
>>>>>>>> different types, which
>>>>>>>> aren't handled by vectorizer right now.  Maybe this issue could be
>>>>>>>> special-cased for
>>>>>>>> boolean-types in tree-vect-loop, by making operand for used condition
>>>>>>>> equal to vector-type.
>>>>>>>> But this is a subject for a different patch and not addressed by this series.
>>>>>>>>
>>>>>>>> There is a regressions in tree-ssa/vrp47.c, and the fix is addressed
>>>>>>>> by the 3rd patch of this
>>>>>>>> series.
>>>>>>>>
>>>>>>>> Bootstrapped and regression tested for all standard-languages (plus
>>>>>>>> Ada and Obj-C++) on host x86_64-pc-linux-gnu.
>>>>>>>>
>>>>>>>> Ok for apply?
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Kai
>>>>>>>>
>>>>>>>>
>>>>>>>> ChangeLog
>>>>>>>>
>>>>>>>> 2011-07-07  Kai Tietz  <ktietz@redhat.com>
>>>>>>>>
>>>>>>>>        * fold-const.c (fold_unary_loc): Preserve
>>>>>>>>        non-boolean-typed casts.
>>>>>>>>        * gimplify.c (gimple_boolify): Handle boolification
>>>>>>>>        of comparisons.
>>>>>>>>        (gimplify_expr): Boolifiy non aggregate-typed
>>>>>>>>        comparisons.
>>>>>>>>        * tree-cfg.c (verify_gimple_comparison): Check result
>>>>>>>>        type of comparison expression.
>>>>>>>>        * tree-ssa.c (useless_type_conversion_p): Preserve incompatible
>>>>>>>>        casts from/to boolean,
>>>>>>>>        * tree-ssa-forwprop.c (combine_cond_expr_cond): Add simplification
>>>>>>>>        support for one-bit-precision typed X for cases X != 0 and X == 0.
>>>>>>>>        (forward_propagate_comparison): Adjust test of condition
>>>>>>>>        result.
>>>>>>>>
>>>>>>>>
>>>>>>>>        * gcc.dg/tree-ssa/builtin-expect-5.c: Adjusted.
>>>>>>>>        * gcc.dg/tree-ssa/pr21031.c: Likewise.
>>>>>>>>        * gcc.dg/tree-ssa/pr30978.c: Likewise.
>>>>>>>>        * gcc.dg/tree-ssa/ssa-fre-6.c: Likewise.
>>>>>>>>        * gcc.dg/binop-xor1.c: Mark it as expected fail.
>>>>>>>>        * gcc.dg/binop-xor3.c: Likewise.
>>>>>>>>        * gcc.dg/uninit-15.c: Adjust reported message.
>>>>>>>>
>>>>>>>> Index: gcc-head/gcc/fold-const.c
>>>>>>>> ===================================================================
>>>>>>>> --- gcc-head.orig/gcc/fold-const.c
>>>>>>>> +++ gcc-head/gcc/fold-const.c
>>>>>>>> @@ -7665,11 +7665,11 @@ fold_unary_loc (location_t loc, enum tre
>>>>>>>>             non-integral type.
>>>>>>>>             Do not fold the result as that would not simplify further, also
>>>>>>>>             folding again results in recursions.  */
>>>>>>>> -         if (INTEGRAL_TYPE_P (type))
>>>>>>>> +         if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>>>>            return build2_loc (loc, TREE_CODE (op0), type,
>>>>>>>>                               TREE_OPERAND (op0, 0),
>>>>>>>>                               TREE_OPERAND (op0, 1));
>>>>>>>> -         else
>>>>>>>> +         else if (!INTEGRAL_TYPE_P (type))
>>>>>>>>            return build3_loc (loc, COND_EXPR, type, op0,
>>>>>>>>                               fold_convert (type, boolean_true_node),
>>>>>>>>                               fold_convert (type, boolean_false_node));
>>>>>>>> Index: gcc-head/gcc/gimplify.c
>>>>>>>> ===================================================================
>>>>>>>> --- gcc-head.orig/gcc/gimplify.c
>>>>>>>> +++ gcc-head/gcc/gimplify.c
>>>>>>>> @@ -2842,18 +2842,23 @@ gimple_boolify (tree expr)
>>>>>>>>
>>>>>>>>     case TRUTH_NOT_EXPR:
>>>>>>>>       TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
>>>>>>>> -      /* FALLTHRU */
>>>>>>>>
>>>>>>>> -    case EQ_EXPR: case NE_EXPR:
>>>>>>>> -    case LE_EXPR: case GE_EXPR: case LT_EXPR: case GT_EXPR:
>>>>>>>>       /* These expressions always produce boolean results.  */
>>>>>>>> -      TREE_TYPE (expr) = boolean_type_node;
>>>>>>>> +      if (TREE_CODE (type) != BOOLEAN_TYPE)
>>>>>>>> +       TREE_TYPE (expr) = boolean_type_node;
>>>>>>>>       return expr;
>>>>>>>>
>>>>>>>>     default:
>>>>>>>> +      if (COMPARISON_CLASS_P (expr))
>>>>>>>> +       {
>>>>>>>> +         /* There expressions always prduce boolean results.  */
>>>>>>>> +         if (TREE_CODE (type) != BOOLEAN_TYPE)
>>>>>>>> +           TREE_TYPE (expr) = boolean_type_node;
>>>>>>>> +         return expr;
>>>>>>>> +       }
>>>>>>>>       /* Other expressions that get here must have boolean values, but
>>>>>>>>         might need to be converted to the appropriate mode.  */
>>>>>>>> -      if (type == boolean_type_node)
>>>>>>>> +      if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>>>>        return expr;
>>>>>>>>       return fold_convert_loc (loc, boolean_type_node, expr);
>>>>>>>>     }
>>>>>>>> @@ -6763,7 +6768,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>>>            tree org_type = TREE_TYPE (*expr_p);
>>>>>>>>
>>>>>>>>            *expr_p = gimple_boolify (*expr_p);
>>>>>>>> -           if (org_type != boolean_type_node)
>>>>>>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>>>>>>              {
>>>>>>>>                *expr_p = fold_convert (org_type, *expr_p);
>>>>>>>
>>>>>>> Use fold_convert_loc with saved_location
>>>>>>
>>>>>> Oh, good catch. Yes, I will adjust that.
>>>>>>
>>>>>>>>                ret = GS_OK;
>>>>>>>> @@ -7208,7 +7213,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>>>               fold_truth_not_expr) happily uses operand type and doesn't
>>>>>>>>               automatically uses boolean_type as result, we need to keep
>>>>>>>>               orignal type.  */
>>>>>>>> -           if (org_type != boolean_type_node)
>>>>>>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>>>>>>              {
>>>>>>>>                *expr_p = fold_convert (org_type, *expr_p);
>>>>>>>
>>>>>>> Likewise.  Maybe this fixes the diagnostic regression.
>>>>>>>
>>>>>>>>                ret = GS_OK;
>>>>>>>> @@ -7288,7 +7293,19 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>>>                  tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>>>>>>>
>>>>>>>>                  if (!AGGREGATE_TYPE_P (type))
>>>>>>>> -                   goto expr_2;
>>>>>>>> +                   {
>>>>>>>> +                     tree org_type = TREE_TYPE (*expr_p);
>>>>>>>> +                     *expr_p = gimple_boolify (*expr_p);
>>>>>>>> +                     if (!useless_type_conversion_p (org_type,
>>>>>>>> +                                                     TREE_TYPE (*expr_p)))
>>>>>>>> +                       {
>>>>>>>> +                         *expr_p = fold_convert_loc (saved_location,
>>>>>>>> +                                                     org_type, *expr_p);
>>>>>>>> +                         ret = GS_OK;
>>>>>>>> +                       }
>>>>>>>> +                     else
>>>>>>>> +                       goto expr_2;
>>>>>>>> +                   }
>>>>>>>>                  else if (TYPE_MODE (type) != BLKmode)
>>>>>>>>                    ret = gimplify_scalar_mode_aggregate_compare (expr_p);
>>>>>>>>                  else
>>>>>>>> Index: gcc-head/gcc/tree-cfg.c
>>>>>>>> ===================================================================
>>>>>>>> --- gcc-head.orig/gcc/tree-cfg.c
>>>>>>>> +++ gcc-head/gcc/tree-cfg.c
>>>>>>>> @@ -3203,7 +3203,9 @@ verify_gimple_comparison (tree type, tre
>>>>>>>>        && (!POINTER_TYPE_P (op0_type)
>>>>>>>>           || !POINTER_TYPE_P (op1_type)
>>>>>>>>           || TYPE_MODE (op0_type) != TYPE_MODE (op1_type)))
>>>>>>>> -      || !INTEGRAL_TYPE_P (type))
>>>>>>>> +      || !INTEGRAL_TYPE_P (type)
>>>>>>>> +      || (TREE_CODE (type) != BOOLEAN_TYPE
>>>>>>>> +         && TYPE_PRECISION (type) != 1))
>>>>>>>>     {
>>>>>>>>       error ("type mismatch in comparison expression");
>>>>>>>>       debug_generic_expr (type);
>>>>>>>> Index: gcc-head/gcc/tree-ssa.c
>>>>>>>> ===================================================================
>>>>>>>> --- gcc-head.orig/gcc/tree-ssa.c
>>>>>>>> +++ gcc-head/gcc/tree-ssa.c
>>>>>>>> @@ -1306,10 +1306,10 @@ useless_type_conversion_p (tree outer_ty
>>>>>>>>          || TYPE_PRECISION (inner_type) != TYPE_PRECISION (outer_type))
>>>>>>>>        return false;
>>>>>>>>
>>>>>>>> -      /* Preserve conversions to BOOLEAN_TYPE if it is not of precision
>>>>>>>> -         one.  */
>>>>>>>> -      if (TREE_CODE (inner_type) != BOOLEAN_TYPE
>>>>>>>> -         && TREE_CODE (outer_type) == BOOLEAN_TYPE
>>>>>>>> +      /* Preserve conversions to/from BOOLEAN_TYPE if types are not
>>>>>>>> +        of precision one.  */
>>>>>>>> +      if (((TREE_CODE (inner_type) == BOOLEAN_TYPE)
>>>>>>>> +          != (TREE_CODE (outer_type) == BOOLEAN_TYPE))
>>>>>>>>          && TYPE_PRECISION (outer_type) != 1)
>>>>>>>>        return false;
>>>>>>>>
>>>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>>>>> ===================================================================
>>>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>>>>> @@ -11,5 +11,5 @@ f (int i, float j)
>>>>>>>>
>>>>>>>>  /* { dg-final { scan-tree-dump-times { if } 2 "forwprop1"} } */
>>>>>>>>  /* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 0\);\n[^\n]*if}
>>>>>>>> "forwprop1"} } */
>>>>>>>> -/* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 1\);\n[^\n]*if}
>>>>>>>> "forwprop1"} } */
>>>>>>>> +/* { dg-final { scan-tree-dump-not {builtin_expect[^\n]*,
>>>>>>>> 1\);\n[^\n]*if} "forwprop1"} } */
>>>>>>>
>>>>>>> Hm?  Why that?
>>>>>>>
>>>>>>>>  /* { dg-final { cleanup-tree-dump "forwprop?" } } */
>>>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>>>>> ===================================================================
>>>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>>>>> @@ -16,5 +16,5 @@ foo (int a)
>>>>>>>>     return 0;
>>>>>>>>  }
>>>>>>>>
>>>>>>>> -/* { dg-final { scan-tree-dump-times "Replaced" 2 "forwprop1"} } */
>>>>>>>> +/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
>>>>>>>>  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
>>>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>>>>> ===================================================================
>>>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>>>>> @@ -10,5 +10,5 @@ int foo(int a)
>>>>>>>>   return e;
>>>>>>>>  }
>>>>>>>>
>>>>>>>> -/* { dg-final { scan-tree-dump "e_. = a_..D. > 0;" "optimized" } } */
>>>>>>>> +/* { dg-final { scan-tree-dump " = a_..D. > 0;" "optimized" } } */
>>>>>>>>  /* { dg-final { cleanup-tree-dump "optimized" } } */
>>>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>>>>> ===================================================================
>>>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>>>>> @@ -2,5 +2,5 @@
>>>>>>>>  /* { dg-options "-O -fdump-tree-fre1-details" } */
>>>>>>>>
>>>>>>>>  int i; int foo(void) { i = 2; int j = i * 2; int k = i + 2; return j == k; }
>>>>>>>> -/* { dg-final { scan-tree-dump-times "Replaced " 5 "fre1" } } */
>>>>>>>> +/* { dg-final { scan-tree-dump-times "Replaced " 6 "fre1" } } */
>>>>>>>>  /* { dg-final { cleanup-tree-dump "fre1" } } */
>>>>>>>> Index: gcc-head/gcc/tree-ssa-forwprop.c
>>>>>>>> ===================================================================
>>>>>>>> --- gcc-head.orig/gcc/tree-ssa-forwprop.c
>>>>>>>> +++ gcc-head/gcc/tree-ssa-forwprop.c
>>>>>>>> @@ -367,9 +367,61 @@ combine_cond_expr_cond (location_t loc,
>>>>>>>>   gcc_assert (TREE_CODE_CLASS (code) == tcc_comparison);
>>>>>>>>
>>>>>>>>   t = fold_binary_loc (loc, code, type, op0, op1);
>>>>>>>> +
>>>>>>>> +  if (!t && INTEGRAL_TYPE_P (TREE_TYPE (op1))
>>>>>>>> +      && TYPE_PRECISION (TREE_TYPE (op1)) == 1
>>>>>>>> +      && (code == EQ_EXPR || code == NE_EXPR))
>>>>>>>> +    {
>>>>>>>> +      if (TREE_CODE (op1) == INTEGER_CST)
>>>>>>>> +        {
>>>>>>>> +         if (integer_onep (op1))
>>>>>>>> +           {
>>>>>>>> +             op1 = fold_convert_loc (loc, TREE_TYPE (op1), integer_zero_node);
>>>>>>>> +             code = (code == NE_EXPR ? EQ_EXPR : NE_EXPR);
>>>>>>>
>>>>>>> So you change truthvalue !=/== 1 to truthvalue ==/!= 0 and then
>>>>>>> recurse ... that doesn't make sense to me and is super-ugly.
>>>>>>> What's the testcase that made you add all this code?
>>>>>>
>>>>>> Well, the convert from truthvalue !=/== 1 to !=/== 0 limits the amount
>>>>>> of cases to handle. As for truthvalued X the we have then just to
>>>>>> handle two cases. X != 0 -> X, and X == 0 -> (X ^ 1).
>>>>>> The recursion is someting I saw as existing pattern (for the same
>>>>>> thing) in truth-op folding in fold-const.
>>>>>>
>>>>>> Actual I can remove this optimization here, as it should be convered
>>>>>> by VRP already (when VRP handles 1-bit precision bitwise ops proper).
>>>>>
>>>>> We should have a canonical form for those compares and change
>>>>> them accordingly, best in fold_stmt.
>>>>>
>>>>> Richard.
>>>>
>>>> Hmm, I tried to add this code-pattern to fold_stmt, but for this kind
>>>> of branch it seems not to be invoked at all. At least not now without
>>>> boolification of compares.  One nit I found for GIMPLE_BINARY, as here
>>>> just patterns getting replaced, which have fewer number of ops then
>>>> original statement. This check looks a bit bogus.
>>>> For getting this normalization right now in a consistant way,
>>>> fold-const might be right now the better place to handle this.
>>>
>>> It gets invoked (well, should get invoked) when anyone changes the
>>> statement.  If it is present in that form from the beginning then we
>>> lack canonicalization in fold and/or gimplification.
>>>
>>> I suppose you have a testcase?
>>>
>>> Richard.
>>>
>>>> Regards,
>>>> Kai
>>>>
>>
>> Yes, testcase looks like this:
>>
>> int foo (_Bool a, _Bool b)
>> {
>>  return a != ((b | !b));
>> }
>>
>> while reduces to
>> ...
>> return a != 1;  right now, and not a == 0 (as a is boolean)
>
> fold_stmt should be called when copyprop produces a != 1
> via substitute_and_fold:
>
>          /* Replace real uses in the statement.  */
>          if (get_value_fn)
>            did_replace |= replace_uses_in (stmt, get_value_fn);
>
>          /* If we made a replacement, fold the statement.  */
>          if (did_replace)
>            fold_stmt (&oldi);

Well, I added this transformation for test in fold-const s
fold_comparison function and still it doesn't get converted.
I assume this caused by the fact that we do this optimization in
forwprop on argument's of the comparison, so no try to fold statement
happens here at this place.

Kai

Richard Biener July 12, 2011, 2:10 p.m. UTC | #9

On Tue, Jul 12, 2011 at 2:21 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> 2011/7/12 Richard Guenther <richard.guenther@gmail.com>:
>> On Tue, Jul 12, 2011 at 11:48 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>> 2011/7/12 Richard Guenther <richard.guenther@gmail.com>:
>>>> On Mon, Jul 11, 2011 at 5:37 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>>>>>> On Fri, Jul 8, 2011 at 1:32 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>>>> 2011/7/8 Richard Guenther <richard.guenther@gmail.com>:
>>>>>>>> On Thu, Jul 7, 2011 at 6:07 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> This patch - second of series - adds boolification of comparisions in
>>>>>>>>> gimplifier.  For this
>>>>>>>>> casts from/to boolean are marked as not-useless. And in fold_unary_loc
>>>>>>>>> casts to non-boolean integral types are preserved.
>>>>>>>>> The hunk in tree-ssa-forwprop.c in combine_cond-expr_cond is not strictly
>>>>>>>>> necessary - as long as fold-const handles 1-bit precision bitwise-expression
>>>>>>>>> with truth-logic - but it has shown to short-cut some expensier folding. So
>>>>>>>>> I kept it within this patch.
>>>>>>>>
>>>>>>>> Please split it out.  Also ...
>>>>>>>>
>>>>>>>>>
>>>>>>>>> The adjusted testcase gcc.dg/uninit-15.c indicates that due
>>>>>>>>> optimization we loose
>>>>>>>>> in this case variables declaration.  But this might be to be expected.
>>>>>>>>>
>>>>>>>>> In vectorization we have a regression in gcc.dg/vect/vect-cond-3.c
>>>>>>>>> test-case.  It's caused
>>>>>>>>> by always having boolean-type on conditions.  So vectorizer sees
>>>>>>>>> different types, which
>>>>>>>>> aren't handled by vectorizer right now.  Maybe this issue could be
>>>>>>>>> special-cased for
>>>>>>>>> boolean-types in tree-vect-loop, by making operand for used condition
>>>>>>>>> equal to vector-type.
>>>>>>>>> But this is a subject for a different patch and not addressed by this series.
>>>>>>>>>
>>>>>>>>> There is a regressions in tree-ssa/vrp47.c, and the fix is addressed
>>>>>>>>> by the 3rd patch of this
>>>>>>>>> series.
>>>>>>>>>
>>>>>>>>> Bootstrapped and regression tested for all standard-languages (plus
>>>>>>>>> Ada and Obj-C++) on host x86_64-pc-linux-gnu.
>>>>>>>>>
>>>>>>>>> Ok for apply?
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Kai
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ChangeLog
>>>>>>>>>
>>>>>>>>> 2011-07-07  Kai Tietz  <ktietz@redhat.com>
>>>>>>>>>
>>>>>>>>>        * fold-const.c (fold_unary_loc): Preserve
>>>>>>>>>        non-boolean-typed casts.
>>>>>>>>>        * gimplify.c (gimple_boolify): Handle boolification
>>>>>>>>>        of comparisons.
>>>>>>>>>        (gimplify_expr): Boolifiy non aggregate-typed
>>>>>>>>>        comparisons.
>>>>>>>>>        * tree-cfg.c (verify_gimple_comparison): Check result
>>>>>>>>>        type of comparison expression.
>>>>>>>>>        * tree-ssa.c (useless_type_conversion_p): Preserve incompatible
>>>>>>>>>        casts from/to boolean,
>>>>>>>>>        * tree-ssa-forwprop.c (combine_cond_expr_cond): Add simplification
>>>>>>>>>        support for one-bit-precision typed X for cases X != 0 and X == 0.
>>>>>>>>>        (forward_propagate_comparison): Adjust test of condition
>>>>>>>>>        result.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>        * gcc.dg/tree-ssa/builtin-expect-5.c: Adjusted.
>>>>>>>>>        * gcc.dg/tree-ssa/pr21031.c: Likewise.
>>>>>>>>>        * gcc.dg/tree-ssa/pr30978.c: Likewise.
>>>>>>>>>        * gcc.dg/tree-ssa/ssa-fre-6.c: Likewise.
>>>>>>>>>        * gcc.dg/binop-xor1.c: Mark it as expected fail.
>>>>>>>>>        * gcc.dg/binop-xor3.c: Likewise.
>>>>>>>>>        * gcc.dg/uninit-15.c: Adjust reported message.
>>>>>>>>>
>>>>>>>>> Index: gcc-head/gcc/fold-const.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- gcc-head.orig/gcc/fold-const.c
>>>>>>>>> +++ gcc-head/gcc/fold-const.c
>>>>>>>>> @@ -7665,11 +7665,11 @@ fold_unary_loc (location_t loc, enum tre
>>>>>>>>>             non-integral type.
>>>>>>>>>             Do not fold the result as that would not simplify further, also
>>>>>>>>>             folding again results in recursions.  */
>>>>>>>>> -         if (INTEGRAL_TYPE_P (type))
>>>>>>>>> +         if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>>>>>            return build2_loc (loc, TREE_CODE (op0), type,
>>>>>>>>>                               TREE_OPERAND (op0, 0),
>>>>>>>>>                               TREE_OPERAND (op0, 1));
>>>>>>>>> -         else
>>>>>>>>> +         else if (!INTEGRAL_TYPE_P (type))
>>>>>>>>>            return build3_loc (loc, COND_EXPR, type, op0,
>>>>>>>>>                               fold_convert (type, boolean_true_node),
>>>>>>>>>                               fold_convert (type, boolean_false_node));
>>>>>>>>> Index: gcc-head/gcc/gimplify.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- gcc-head.orig/gcc/gimplify.c
>>>>>>>>> +++ gcc-head/gcc/gimplify.c
>>>>>>>>> @@ -2842,18 +2842,23 @@ gimple_boolify (tree expr)
>>>>>>>>>
>>>>>>>>>     case TRUTH_NOT_EXPR:
>>>>>>>>>       TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
>>>>>>>>> -      /* FALLTHRU */
>>>>>>>>>
>>>>>>>>> -    case EQ_EXPR: case NE_EXPR:
>>>>>>>>> -    case LE_EXPR: case GE_EXPR: case LT_EXPR: case GT_EXPR:
>>>>>>>>>       /* These expressions always produce boolean results.  */
>>>>>>>>> -      TREE_TYPE (expr) = boolean_type_node;
>>>>>>>>> +      if (TREE_CODE (type) != BOOLEAN_TYPE)
>>>>>>>>> +       TREE_TYPE (expr) = boolean_type_node;
>>>>>>>>>       return expr;
>>>>>>>>>
>>>>>>>>>     default:
>>>>>>>>> +      if (COMPARISON_CLASS_P (expr))
>>>>>>>>> +       {
>>>>>>>>> +         /* There expressions always prduce boolean results.  */
>>>>>>>>> +         if (TREE_CODE (type) != BOOLEAN_TYPE)
>>>>>>>>> +           TREE_TYPE (expr) = boolean_type_node;
>>>>>>>>> +         return expr;
>>>>>>>>> +       }
>>>>>>>>>       /* Other expressions that get here must have boolean values, but
>>>>>>>>>         might need to be converted to the appropriate mode.  */
>>>>>>>>> -      if (type == boolean_type_node)
>>>>>>>>> +      if (TREE_CODE (type) == BOOLEAN_TYPE)
>>>>>>>>>        return expr;
>>>>>>>>>       return fold_convert_loc (loc, boolean_type_node, expr);
>>>>>>>>>     }
>>>>>>>>> @@ -6763,7 +6768,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>>>>            tree org_type = TREE_TYPE (*expr_p);
>>>>>>>>>
>>>>>>>>>            *expr_p = gimple_boolify (*expr_p);
>>>>>>>>> -           if (org_type != boolean_type_node)
>>>>>>>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>>>>>>>              {
>>>>>>>>>                *expr_p = fold_convert (org_type, *expr_p);
>>>>>>>>
>>>>>>>> Use fold_convert_loc with saved_location
>>>>>>>
>>>>>>> Oh, good catch. Yes, I will adjust that.
>>>>>>>
>>>>>>>>>                ret = GS_OK;
>>>>>>>>> @@ -7208,7 +7213,7 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>>>>               fold_truth_not_expr) happily uses operand type and doesn't
>>>>>>>>>               automatically uses boolean_type as result, we need to keep
>>>>>>>>>               orignal type.  */
>>>>>>>>> -           if (org_type != boolean_type_node)
>>>>>>>>> +           if (!useless_type_conversion_p (org_type, TREE_TYPE (*expr_p)))
>>>>>>>>>              {
>>>>>>>>>                *expr_p = fold_convert (org_type, *expr_p);
>>>>>>>>
>>>>>>>> Likewise.  Maybe this fixes the diagnostic regression.
>>>>>>>>
>>>>>>>>>                ret = GS_OK;
>>>>>>>>> @@ -7288,7 +7293,19 @@ gimplify_expr (tree *expr_p, gimple_seq
>>>>>>>>>                  tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 1));
>>>>>>>>>
>>>>>>>>>                  if (!AGGREGATE_TYPE_P (type))
>>>>>>>>> -                   goto expr_2;
>>>>>>>>> +                   {
>>>>>>>>> +                     tree org_type = TREE_TYPE (*expr_p);
>>>>>>>>> +                     *expr_p = gimple_boolify (*expr_p);
>>>>>>>>> +                     if (!useless_type_conversion_p (org_type,
>>>>>>>>> +                                                     TREE_TYPE (*expr_p)))
>>>>>>>>> +                       {
>>>>>>>>> +                         *expr_p = fold_convert_loc (saved_location,
>>>>>>>>> +                                                     org_type, *expr_p);
>>>>>>>>> +                         ret = GS_OK;
>>>>>>>>> +                       }
>>>>>>>>> +                     else
>>>>>>>>> +                       goto expr_2;
>>>>>>>>> +                   }
>>>>>>>>>                  else if (TYPE_MODE (type) != BLKmode)
>>>>>>>>>                    ret = gimplify_scalar_mode_aggregate_compare (expr_p);
>>>>>>>>>                  else
>>>>>>>>> Index: gcc-head/gcc/tree-cfg.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- gcc-head.orig/gcc/tree-cfg.c
>>>>>>>>> +++ gcc-head/gcc/tree-cfg.c
>>>>>>>>> @@ -3203,7 +3203,9 @@ verify_gimple_comparison (tree type, tre
>>>>>>>>>        && (!POINTER_TYPE_P (op0_type)
>>>>>>>>>           || !POINTER_TYPE_P (op1_type)
>>>>>>>>>           || TYPE_MODE (op0_type) != TYPE_MODE (op1_type)))
>>>>>>>>> -      || !INTEGRAL_TYPE_P (type))
>>>>>>>>> +      || !INTEGRAL_TYPE_P (type)
>>>>>>>>> +      || (TREE_CODE (type) != BOOLEAN_TYPE
>>>>>>>>> +         && TYPE_PRECISION (type) != 1))
>>>>>>>>>     {
>>>>>>>>>       error ("type mismatch in comparison expression");
>>>>>>>>>       debug_generic_expr (type);
>>>>>>>>> Index: gcc-head/gcc/tree-ssa.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- gcc-head.orig/gcc/tree-ssa.c
>>>>>>>>> +++ gcc-head/gcc/tree-ssa.c
>>>>>>>>> @@ -1306,10 +1306,10 @@ useless_type_conversion_p (tree outer_ty
>>>>>>>>>          || TYPE_PRECISION (inner_type) != TYPE_PRECISION (outer_type))
>>>>>>>>>        return false;
>>>>>>>>>
>>>>>>>>> -      /* Preserve conversions to BOOLEAN_TYPE if it is not of precision
>>>>>>>>> -         one.  */
>>>>>>>>> -      if (TREE_CODE (inner_type) != BOOLEAN_TYPE
>>>>>>>>> -         && TREE_CODE (outer_type) == BOOLEAN_TYPE
>>>>>>>>> +      /* Preserve conversions to/from BOOLEAN_TYPE if types are not
>>>>>>>>> +        of precision one.  */
>>>>>>>>> +      if (((TREE_CODE (inner_type) == BOOLEAN_TYPE)
>>>>>>>>> +          != (TREE_CODE (outer_type) == BOOLEAN_TYPE))
>>>>>>>>>          && TYPE_PRECISION (outer_type) != 1)
>>>>>>>>>        return false;
>>>>>>>>>
>>>>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/builtin-expect-5.c
>>>>>>>>> @@ -11,5 +11,5 @@ f (int i, float j)
>>>>>>>>>
>>>>>>>>>  /* { dg-final { scan-tree-dump-times { if } 2 "forwprop1"} } */
>>>>>>>>>  /* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 0\);\n[^\n]*if}
>>>>>>>>> "forwprop1"} } */
>>>>>>>>> -/* { dg-final { scan-tree-dump {builtin_expect[^\n]*, 1\);\n[^\n]*if}
>>>>>>>>> "forwprop1"} } */
>>>>>>>>> +/* { dg-final { scan-tree-dump-not {builtin_expect[^\n]*,
>>>>>>>>> 1\);\n[^\n]*if} "forwprop1"} } */
>>>>>>>>
>>>>>>>> Hm?  Why that?
>>>>>>>>
>>>>>>>>>  /* { dg-final { cleanup-tree-dump "forwprop?" } } */
>>>>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
>>>>>>>>> @@ -16,5 +16,5 @@ foo (int a)
>>>>>>>>>     return 0;
>>>>>>>>>  }
>>>>>>>>>
>>>>>>>>> -/* { dg-final { scan-tree-dump-times "Replaced" 2 "forwprop1"} } */
>>>>>>>>> +/* { dg-final { scan-tree-dump-times "Replaced" 1 "forwprop1"} } */
>>>>>>>>>  /* { dg-final { cleanup-tree-dump "forwprop1" } } */
>>>>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/pr30978.c
>>>>>>>>> @@ -10,5 +10,5 @@ int foo(int a)
>>>>>>>>>   return e;
>>>>>>>>>  }
>>>>>>>>>
>>>>>>>>> -/* { dg-final { scan-tree-dump "e_. = a_..D. > 0;" "optimized" } } */
>>>>>>>>> +/* { dg-final { scan-tree-dump " = a_..D. > 0;" "optimized" } } */
>>>>>>>>>  /* { dg-final { cleanup-tree-dump "optimized" } } */
>>>>>>>>> Index: gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- gcc-head.orig/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>>>>>> +++ gcc-head/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-6.c
>>>>>>>>> @@ -2,5 +2,5 @@
>>>>>>>>>  /* { dg-options "-O -fdump-tree-fre1-details" } */
>>>>>>>>>
>>>>>>>>>  int i; int foo(void) { i = 2; int j = i * 2; int k = i + 2; return j == k; }
>>>>>>>>> -/* { dg-final { scan-tree-dump-times "Replaced " 5 "fre1" } } */
>>>>>>>>> +/* { dg-final { scan-tree-dump-times "Replaced " 6 "fre1" } } */
>>>>>>>>>  /* { dg-final { cleanup-tree-dump "fre1" } } */
>>>>>>>>> Index: gcc-head/gcc/tree-ssa-forwprop.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- gcc-head.orig/gcc/tree-ssa-forwprop.c
>>>>>>>>> +++ gcc-head/gcc/tree-ssa-forwprop.c
>>>>>>>>> @@ -367,9 +367,61 @@ combine_cond_expr_cond (location_t loc,
>>>>>>>>>   gcc_assert (TREE_CODE_CLASS (code) == tcc_comparison);
>>>>>>>>>
>>>>>>>>>   t = fold_binary_loc (loc, code, type, op0, op1);
>>>>>>>>> +
>>>>>>>>> +  if (!t && INTEGRAL_TYPE_P (TREE_TYPE (op1))
>>>>>>>>> +      && TYPE_PRECISION (TREE_TYPE (op1)) == 1
>>>>>>>>> +      && (code == EQ_EXPR || code == NE_EXPR))
>>>>>>>>> +    {
>>>>>>>>> +      if (TREE_CODE (op1) == INTEGER_CST)
>>>>>>>>> +        {
>>>>>>>>> +         if (integer_onep (op1))
>>>>>>>>> +           {
>>>>>>>>> +             op1 = fold_convert_loc (loc, TREE_TYPE (op1), integer_zero_node);
>>>>>>>>> +             code = (code == NE_EXPR ? EQ_EXPR : NE_EXPR);
>>>>>>>>
>>>>>>>> So you change truthvalue !=/== 1 to truthvalue ==/!= 0 and then
>>>>>>>> recurse ... that doesn't make sense to me and is super-ugly.
>>>>>>>> What's the testcase that made you add all this code?
>>>>>>>
>>>>>>> Well, the convert from truthvalue !=/== 1 to !=/== 0 limits the amount
>>>>>>> of cases to handle. As for truthvalued X the we have then just to
>>>>>>> handle two cases. X != 0 -> X, and X == 0 -> (X ^ 1).
>>>>>>> The recursion is someting I saw as existing pattern (for the same
>>>>>>> thing) in truth-op folding in fold-const.
>>>>>>>
>>>>>>> Actual I can remove this optimization here, as it should be convered
>>>>>>> by VRP already (when VRP handles 1-bit precision bitwise ops proper).
>>>>>>
>>>>>> We should have a canonical form for those compares and change
>>>>>> them accordingly, best in fold_stmt.
>>>>>>
>>>>>> Richard.
>>>>>
>>>>> Hmm, I tried to add this code-pattern to fold_stmt, but for this kind
>>>>> of branch it seems not to be invoked at all. At least not now without
>>>>> boolification of compares.  One nit I found for GIMPLE_BINARY, as here
>>>>> just patterns getting replaced, which have fewer number of ops then
>>>>> original statement. This check looks a bit bogus.
>>>>> For getting this normalization right now in a consistant way,
>>>>> fold-const might be right now the better place to handle this.
>>>>
>>>> It gets invoked (well, should get invoked) when anyone changes the
>>>> statement.  If it is present in that form from the beginning then we
>>>> lack canonicalization in fold and/or gimplification.
>>>>
>>>> I suppose you have a testcase?
>>>>
>>>> Richard.
>>>>
>>>>> Regards,
>>>>> Kai
>>>>>
>>>
>>> Yes, testcase looks like this:
>>>
>>> int foo (_Bool a, _Bool b)
>>> {
>>>  return a != ((b | !b));
>>> }
>>>
>>> while reduces to
>>> ...
>>> return a != 1;  right now, and not a == 0 (as a is boolean)
>>
>> fold_stmt should be called when copyprop produces a != 1
>> via substitute_and_fold:
>>
>>          /* Replace real uses in the statement.  */
>>          if (get_value_fn)
>>            did_replace |= replace_uses_in (stmt, get_value_fn);
>>
>>          /* If we made a replacement, fold the statement.  */
>>          if (did_replace)
>>            fold_stmt (&oldi);
>
> Well, I added this transformation for test in fold-const s
> fold_comparison function and still it doesn't get converted.
> I assume this caused by the fact that we do this optimization in
> forwprop on argument's of the comparison, so no try to fold statement
> happens here at this place.

The non-canonical comparison appears after copyprop for me.

Richard.

> Kai
>

[tree-optimization] : [2 of 3]: Boolify compares & more

Commit Message

Comments

Patch