Re: [PATCH] Make VRP optimize useless conversions

[Michael Matz <matz at suse dot de>]

Hi,

On Thu, 7 Jul 2011, Richard Guenther wrote:

> +   tree rhs1 = gimple_assign_rhs1 (stmt);
> +   gimple def_stmt = SSA_NAME_DEF_STMT (rhs1);
> +   value_range_t *final, *inner;
> + 
> +   /* Obtain final and inner value-ranges for a conversion
> +      sequence (final-type)(intermediate-type)inner-type.  */
> +   final = get_value_range (gimple_assign_lhs (stmt));
> +   if (final->type != VR_RANGE)
> +     return false;
> +   if (!is_gimple_assign (def_stmt)
> +       || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt)))
> +     return false;
> +   rhs1 = gimple_assign_rhs1 (def_stmt);
> +   if (TREE_CODE (rhs1) != SSA_NAME)
> +     return false;
> +   inner = get_value_range (rhs1);
> +   if (inner->type != VR_RANGE)
> +     return false;
> +   if (!tree_int_cst_equal (final->min, inner->min)
> +       || !tree_int_cst_equal (final->max, inner->max))
> +     return false;

I think that's a bit too conservative.  Granted in current VRP it might 
work, but think about an intermediate truncation plus widening:

  short s;
  short d = (short)(signed char)s;

It wouldn't be wrong for VRP to assign d the range [-16384,16383], 
suboptimal but correct.  That would trigger your function in removing the 
truncation, and _that_ would be incorrect.  The bounds of VRP aren't 
reliably tight.  You probably want to recheck if the intermediate 
conversion isn't truncating the known input range of rhs1.


Ciao,
Michael.