[Bug tree-optimization/59660] We fail to optimize common boolean checks pre-inlining

[rguenth at gcc dot gnu.org]

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