Make VRP optimize useless conversions

On Thu, 7 Jul 2011, Michael Matz wrote:

> 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.

It should be indeed safe with the current handling of conversions,
but better be safe.  So, like the following?

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Thanks,
Richard.

2011-07-08  Richard Guenther  <rguenther@suse.de>

	* tree-vrp.c (simplify_conversion_using_ranges): Also check
	the intermediate value-range.

Hi,

On Fri, 8 Jul 2011, Richard Guenther wrote:

> It should be indeed safe with the current handling of conversions, but 
> better be safe.  So, like the following?

No.  The point is that you can't compare the bounds that VRP computes with 
each other when the outcome affects correctness.  Think about a very 
trivial and stupid VRP, that assigns the range [WIDEST_INT_MIN .. 
WIDEST_UINT_MAX] to each and every SSA name without looking at types and 
operations at all (assuming that this reflects the largest int type on the 
target).  It's useless but correct.  Of course we wouldn't implement such 
useless range discovery, but similar situations can arise when some VRP 
algorithms give up for certain reasons, or computation of tight bounds 
merely isn't implemented for some operations.

Your routines need to work also in the presence of such imprecise ranges.

Hence, the check that the intermediate conversion is useless needs to take 
into account the input value range (that's conservatively correct), and 
the precision and signedness of the target type (if it can represent all 
value of the input range the conversion was useless).  It must not look at 
the suspected value range of the destination, precisely because it is 
conservative only.


Ciao,
Michael.