diff mbox

Make VRP optimize useless conversions

Message ID alpine.LNX.2.00.1107081530160.810@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener July 8, 2011, 1:33 p.m. UTC 
On Fri, 8 Jul 2011, Michael Matz wrote:

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

Ok, indeed conservative is different for what VRP does and for what
a transformation must assess.  So the following patch makes
a conservative attempt at checking the transformation (which of
course non-surprisingly matches what the VRP part does).

So, more like the following?

Bootstrap & regtest in progress.

Thanks,
Richard.

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

	* tree-vrp.c (simplify_conversion_using_ranges): Manually
	translate the source value-range through the conversion chain.
diff mbox

Patch

Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 176030)
+++ gcc/tree-vrp.c	(working copy)
@@ -7347,30 +7347,56 @@  simplify_switch_using_ranges (gimple stm
 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;
+  tree innerop, middleop, finaltype;
+  gimple def_stmt;
+  value_range_t *innervr;
+  double_int innermin, innermax, middlemin, middlemax;
 
-  /* 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;
+  finaltype = TREE_TYPE (gimple_assign_lhs (stmt));
+  middleop = gimple_assign_rhs1 (stmt);
+  def_stmt = SSA_NAME_DEF_STMT (middleop);
   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)
+  innerop = gimple_assign_rhs1 (def_stmt);
+  if (TREE_CODE (innerop) != SSA_NAME)
+    return false;
+
+  /* Do not allow changing a zero- to a sign-extension or vice versa.  */
+  if (TYPE_UNSIGNED (finaltype)
+      != TYPE_UNSIGNED (TREE_TYPE (middleop)))
     return false;
-  inner = get_value_range (rhs1);
-  if (inner->type != VR_RANGE)
+
+  /* Get the value-range of the inner operand.  */
+  innervr = get_value_range (innerop);
+  if (innervr->type != VR_RANGE
+      || TREE_CODE (innervr->min) != INTEGER_CST
+      || TREE_CODE (innervr->max) != INTEGER_CST)
     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))
+
+  /* Simulate the conversion chain to check if the result is equal if
+     the middle conversion is removed.  */
+  innermin = tree_to_double_int (innervr->min);
+  innermax = tree_to_double_int (innervr->max);
+  middlemin = double_int_ext (innermin, TYPE_PRECISION (TREE_TYPE (middleop)),
+			      TYPE_UNSIGNED (TREE_TYPE (middleop)));
+  middlemax = double_int_ext (innermax, TYPE_PRECISION (TREE_TYPE (middleop)),
+			      TYPE_UNSIGNED (TREE_TYPE (middleop)));
+  if (!double_int_equal_p (double_int_ext (middlemin,
+					   TYPE_PRECISION (finaltype),
+					   TYPE_UNSIGNED (finaltype)),
+			   double_int_ext (innermin,
+					   TYPE_PRECISION (finaltype),
+					   TYPE_UNSIGNED (finaltype)))
+      || !double_int_equal_p (double_int_ext (middlemax,
+					      TYPE_PRECISION (finaltype),
+					      TYPE_UNSIGNED (finaltype)),
+			      double_int_ext (innermax,
+					      TYPE_PRECISION (finaltype),
+					      TYPE_UNSIGNED (finaltype))))
     return false;
-  gimple_assign_set_rhs1 (stmt, rhs1);
+
+  gimple_assign_set_rhs1 (stmt, innerop);
   update_stmt (stmt);
   return true;
 }