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 <rguent...@suse.de>
* tree-vrp.c (simplify_conversion_using_ranges): Also check
the intermediate value-range.
Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c (revision 176030)
+++ gcc/tree-vrp.c (working copy)
@@ -7348,14 +7348,22 @@ static bool
simplify_conversion_using_ranges (gimple stmt)
{
tree rhs1 = gimple_assign_rhs1 (stmt);
- gimple def_stmt = SSA_NAME_DEF_STMT (rhs1);
- value_range_t *final, *inner;
+ gimple def_stmt;
+ value_range_t *final, *intermediate, *inner;
- /* Obtain final and inner value-ranges for a conversion
+ /* Obtain final, intermediate 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;
+ intermediate = get_value_range (rhs1);
+ if (intermediate->type != VR_RANGE)
+ return false;
+ if (!tree_int_cst_equal (final->min, intermediate->min)
+ || !tree_int_cst_equal (final->max, intermediate->max))
+ return false;
+
+ def_stmt = SSA_NAME_DEF_STMT (rhs1);
if (!is_gimple_assign (def_stmt)
|| !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt)))
return false;
@@ -7365,11 +7373,12 @@ simplify_conversion_using_ranges (gimple
inner = get_value_range (rhs1);
if (inner->type != VR_RANGE)
return false;
- /* If the value-range is preserved by the conversion sequence strip
- the intermediate conversion. */
if (!tree_int_cst_equal (final->min, inner->min)
|| !tree_int_cst_equal (final->max, inner->max))
return false;
+
+ /* The value-range is preserved by the conversion sequence; strip
+ the intermediate conversion. */
gimple_assign_set_rhs1 (stmt, rhs1);
update_stmt (stmt);
return true;
