Re: [PATCH] Fix reassoc ICE (PR tree-optimization/69802)

[Richard Biener <rguenther at suse dot de>]

On Mon, 15 Feb 2016, Jakub Jelinek wrote:

> Hi!
> 
> The following patch fixes an ICE where one of the range tests
> is SSA_NAME_DEF_STMT of a bool/_Bool or unsigned : 1 bitfield.
> In that case, we don't know where to put the adjusted range test.
> The patch for this uncommon case gives up, unless the range test
> can be the SSA_NAME_DEF_STMT itself, and in that case makes sure we
> DTRT.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Richard.

> 2016-02-15  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/69802
> 	* tree-ssa-reassoc.c (update_range_test): If op is
> 	SSA_NAME_IS_DEFAULT_DEF, give up unless tem is a positive
> 	op == 1 test of precision 1 integral op, otherwise handle
> 	that case as op itself.  Fix up formatting.
> 	(optimize_range_tests_to_bit_test, optimize_range_tests): Fix
> 	up formatting.
> 
> 	* gcc.dg/pr69802.c: New test.
> 
> --- gcc/tree-ssa-reassoc.c.jj	2016-02-12 10:23:51.000000000 +0100
> +++ gcc/tree-ssa-reassoc.c	2016-02-15 14:25:54.996572238 +0100
> @@ -2046,19 +2046,41 @@ update_range_test (struct range_entry *r
>  {
>    operand_entry *oe = (*ops)[range->idx];
>    tree op = oe->op;
> -  gimple *stmt = op ? SSA_NAME_DEF_STMT (op) :
> -    last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
> +  gimple *stmt = op ? SSA_NAME_DEF_STMT (op)
> +		    : last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
>    location_t loc = gimple_location (stmt);
>    tree optype = op ? TREE_TYPE (op) : boolean_type_node;
>    tree tem = build_range_check (loc, optype, unshare_expr (exp),
>  				in_p, low, high);
>    enum warn_strict_overflow_code wc = WARN_STRICT_OVERFLOW_COMPARISON;
>    gimple_stmt_iterator gsi;
> -  unsigned int i;
> +  unsigned int i, uid;
>  
>    if (tem == NULL_TREE)
>      return false;
>  
> +  /* If op is default def SSA_NAME, there is no place to insert the
> +     new comparison.  Give up, unless we can use OP itself as the
> +     range test.  */
> +  if (op && SSA_NAME_IS_DEFAULT_DEF (op))
> +    {
> +      if (op == range->exp
> +	  && ((TYPE_PRECISION (optype) == 1 && TYPE_UNSIGNED (optype))
> +	      || TREE_CODE (optype) == BOOLEAN_TYPE)
> +	  && (op == tem
> +	      || (TREE_CODE (tem) == EQ_EXPR
> +		  && TREE_OPERAND (tem, 0) == op
> +		  && integer_onep (TREE_OPERAND (tem, 1))))
> +	  && opcode != BIT_IOR_EXPR
> +	  && (opcode != ERROR_MARK || oe->rank != BIT_IOR_EXPR))
> +	{
> +	  stmt = NULL;
> +	  tem = op;
> +	}
> +      else
> +	return false;
> +    }
> +
>    if (strict_overflow_p && issue_strict_overflow_warning (wc))
>      warning_at (loc, OPT_Wstrict_overflow,
>  		"assuming signed overflow does not occur "
> @@ -2096,12 +2118,22 @@ update_range_test (struct range_entry *r
>      tem = invert_truthvalue_loc (loc, tem);
>  
>    tem = fold_convert_loc (loc, optype, tem);
> -  gsi = gsi_for_stmt (stmt);
> -  unsigned int uid = gimple_uid (stmt);
> +  if (stmt)
> +    {
> +      gsi = gsi_for_stmt (stmt);
> +      uid = gimple_uid (stmt);
> +    }
> +  else
> +    {
> +      gsi = gsi_none ();
> +      uid = 0;
> +    }
> +  if (stmt == NULL)
> +    gcc_checking_assert (tem == op);
>    /* In rare cases range->exp can be equal to lhs of stmt.
>       In that case we have to insert after the stmt rather then before
>       it.  If stmt is a PHI, insert it at the start of the basic block.  */
> -  if (op != range->exp)
> +  else if (op != range->exp)
>      {
>        gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
>        tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, true,
> @@ -2489,7 +2521,7 @@ optimize_range_tests_to_bit_test (enum t
>  	  operand_entry *oe = (*ops)[ranges[i].idx];
>  	  tree op = oe->op;
>  	  gimple *stmt = op ? SSA_NAME_DEF_STMT (op)
> -			   : last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
> +			    : last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
>  	  location_t loc = gimple_location (stmt);
>  	  tree optype = op ? TREE_TYPE (op) : boolean_type_node;
>  
> @@ -2553,7 +2585,7 @@ optimize_range_tests_to_bit_test (enum t
>  	  gcc_assert (TREE_CODE (exp) == SSA_NAME);
>  	  gimple_set_visited (SSA_NAME_DEF_STMT (exp), true);
>  	  gimple *g = gimple_build_assign (make_ssa_name (optype),
> -					  BIT_IOR_EXPR, tem, exp);
> +					   BIT_IOR_EXPR, tem, exp);
>  	  gimple_set_location (g, loc);
>  	  gimple_seq_add_stmt_without_update (&seq, g);
>  	  exp = gimple_assign_lhs (g);
> @@ -2599,8 +2631,9 @@ optimize_range_tests (enum tree_code opc
>        oe = (*ops)[i];
>        ranges[i].idx = i;
>        init_range_entry (ranges + i, oe->op,
> -			oe->op ? NULL :
> -			  last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id)));
> +			oe->op
> +			? NULL
> +			: last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id)));
>        /* For | invert it now, we will invert it again before emitting
>  	 the optimized expression.  */
>        if (opcode == BIT_IOR_EXPR
> --- gcc/testsuite/gcc.dg/pr69802.c.jj	2016-02-15 14:06:22.716491146 +0100
> +++ gcc/testsuite/gcc.dg/pr69802.c	2016-02-15 14:05:58.000000000 +0100
> @@ -0,0 +1,23 @@
> +/* PR tree-optimization/69802 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wall" } */
> +
> +struct S { unsigned f : 1; };
> +int a, d;
> +
> +int
> +foo (void)
> +{
> +  unsigned b = 0;
> +  struct S c;
> +  d = ((1 && b) < c.f) & c.f;	/* { dg-warning "is used uninitialized" } */
> +  return a;
> +}
> +
> +int
> +bar (_Bool c)
> +{
> +  unsigned b = 0;
> +  d = ((1 && b) < c) & c;
> +  return a;
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)