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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #9)
> Hi,
> this patch solves the testcase.
> The first hunk gets rid of the redundant NOP_EXPR converting TRUTH_EXPR from
> INT to BOOL.  The second improves gimplifier to not introduce unnecesary
> control flow.
> 
> If this approach seems to make sense, I will extend it to other booleans and
> prepare less hackish patch.
> 
> Honza
> 
> Index: fold-const.c
> ===================================================================
> --- fold-const.c      (revision 206346)
> +++ fold-const.c      (working copy)
> @@ -1977,6 +1977,8 @@ fold_convert_loc (location_t loc, tree t
>  
>      case INTEGER_TYPE: case ENUMERAL_TYPE: case BOOLEAN_TYPE:
>      case OFFSET_TYPE:
> +      if (TREE_CODE (arg) == TRUTH_ORIF_EXPR)
> +     return fold_build2_loc (loc, TRUTH_ORIF_EXPR, type, TREE_OPERAND (arg, 
> 0),
> TREE_OPERAND (arg, 1));
>        if (TREE_CODE (arg) == INTEGER_CST)
>       {
>         tem = fold_convert_const (NOP_EXPR, type, arg);

This looks bogus.  See where we do this folding and instead handle it
there (I suspect gimple_boolify?)

> Index: gimplify.c
> ===================================================================
> --- gimplify.c        (revision 206346)
> +++ gimplify.c        (working copy)
> @@ -2931,14 +2931,37 @@ gimplify_cond_expr (tree *expr_p, gimple
>         result = build_simple_mem_ref_loc (loc, tmp);
>       }
>  
> -      /* Build the new then clause, `tmp = then_;'.  But don't build the
> -      assignment if the value is void; in C++ it can be if it's a throw.  */
> -      if (!VOID_TYPE_P (TREE_TYPE (then_)))
> -     TREE_OPERAND (expr, 1) = build2 (MODIFY_EXPR, type, tmp, then_);
> +      /* Convert (A||B) ? true : false
> +      as A ? tmp = true : tmp = B != 0.  */

So does it end up working for if (a || b || c)?  Otherwise this looks
sensible to me.  (A&&B) ? true : false -> A ? tmp = B != 0 : tmp = false
then?

> -      /* Similarly, build the new else clause, `tmp = else_;'.  */
> -      if (!VOID_TYPE_P (TREE_TYPE (else_)))
> -     TREE_OPERAND (expr, 2) = build2 (MODIFY_EXPR, type, tmp, else_);
> +      if (integer_zerop (else_) && integer_onep (then_)
> +       && TREE_CODE (TREE_OPERAND (expr, 0)) == TRUTH_ORIF_EXPR)
> +     {
> +       tree pred = TREE_OPERAND (expr, 0);
> +       location_t loc = EXPR_LOCATION (*expr_p);
> +       TREE_OPERAND (expr, 0) = TREE_OPERAND (pred, 0);
> +       /* Build the new then clause, `tmp = then_;'.  But don't build the
> +          assignment if the value is void; in C++ it can be if it's a throw. 
>  */
> +       TREE_OPERAND (expr, 1) = build2 (MODIFY_EXPR, type, tmp,
> +                                        fold_convert (type, 
> boolean_true_node));
> +
> +       /* Similarly, build the new else clause, `tmp = else_;'.  */
> +       TREE_OPERAND (expr, 2) = build2 (MODIFY_EXPR, type, tmp,
> +                                        fold_build2_loc (loc, NE_EXPR, type, 
> TREE_OPERAND (pred, 1),
> +                                                         fold_convert 
> (TREE_TYPE (TREE_OPERAND (pred, 1)),
> +                                                                       
> integer_zero_node)));
> +     }
> +      else
> +     {
> +       /* Build the new then clause, `tmp = then_;'.  But don't build the
> +          assignment if the value is void; in C++ it can be if it's a throw. 
>  */
> +       if (!VOID_TYPE_P (TREE_TYPE (then_)))
> +         TREE_OPERAND (expr, 1) = build2 (MODIFY_EXPR, type, tmp, then_);
> +
> +       /* Similarly, build the new else clause, `tmp = else_;'.  */
> +       if (!VOID_TYPE_P (TREE_TYPE (else_)))
> +         TREE_OPERAND (expr, 2) = build2 (MODIFY_EXPR, type, tmp, else_);
> +     }
>  
>        TREE_TYPE (expr) = void_type_node;
>        recalculate_side_effects (expr);

Reply via email to