[PATCH] Make VRP optimize useless conversions

[PATCH] Make VRP optimize useless conversions

[Richard Guenther]

The following patch teaches VRP to disregard the intermediate
conversion in a sequence (T1)(T2)val if that sequence is
value-preserving for val.  There are possibly some more
cases that could be handled when a sign-change is involved
but the following is a first safe step.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

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

	* tree-vrp.c (simplify_conversion_using_ranges): New function.
	(simplify_stmt_using_ranges): Call it.

	* gcc.dg/tree-ssa/vrp58.c: New testcase.

Index: gcc/tree-vrp.c
===================================================================
*** gcc/tree-vrp.c	(revision 175962)
--- gcc/tree-vrp.c	(working copy)
*************** simplify_switch_using_ranges (gimple stm
*** 7342,7347 ****
--- 7342,7378 ----
    return false;
  }
  
+ /* Simplify an integral conversion from an SSA name in STMT.  */
+ 
+ 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;
+ 
+   /* 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;
+   gimple_assign_set_rhs1 (stmt, rhs1);
+   update_stmt (stmt);
+   return true;
+ }
+ 
  /* Simplify STMT using ranges if possible.  */
  
  static bool
*************** simplify_stmt_using_ranges (gimple_stmt_
*** 7351,7356 ****
--- 7382,7388 ----
    if (is_gimple_assign (stmt))
      {
        enum tree_code rhs_code = gimple_assign_rhs_code (stmt);
+       tree rhs1 = gimple_assign_rhs1 (stmt);
  
        switch (rhs_code)
  	{
*************** simplify_stmt_using_ranges (gimple_stmt_
*** 7364,7370 ****
  	     or identity if the RHS is zero or one, and the LHS are known
  	     to be boolean values.  Transform all TRUTH_*_EXPR into
               BIT_*_EXPR if both arguments are known to be boolean values.  */
! 	  if (INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (stmt))))
  	    return simplify_truth_ops_using_ranges (gsi, stmt);
  	  break;
  
--- 7396,7402 ----
  	     or identity if the RHS is zero or one, and the LHS are known
  	     to be boolean values.  Transform all TRUTH_*_EXPR into
               BIT_*_EXPR if both arguments are known to be boolean values.  */
! 	  if (INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
  	    return simplify_truth_ops_using_ranges (gsi, stmt);
  	  break;
  
*************** simplify_stmt_using_ranges (gimple_stmt_
*** 7373,7387 ****
  	 than zero and the second operand is an exact power of two.  */
  	case TRUNC_DIV_EXPR:
  	case TRUNC_MOD_EXPR:
! 	  if (INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (stmt)))
  	      && integer_pow2p (gimple_assign_rhs2 (stmt)))
  	    return simplify_div_or_mod_using_ranges (stmt);
  	  break;
  
        /* Transform ABS (X) into X or -X as appropriate.  */
  	case ABS_EXPR:
! 	  if (TREE_CODE (gimple_assign_rhs1 (stmt)) == SSA_NAME
! 	      && INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (stmt))))
  	    return simplify_abs_using_ranges (stmt);
  	  break;
  
--- 7405,7419 ----
  	 than zero and the second operand is an exact power of two.  */
  	case TRUNC_DIV_EXPR:
  	case TRUNC_MOD_EXPR:
! 	  if (INTEGRAL_TYPE_P (TREE_TYPE (rhs1))
  	      && integer_pow2p (gimple_assign_rhs2 (stmt)))
  	    return simplify_div_or_mod_using_ranges (stmt);
  	  break;
  
        /* Transform ABS (X) into X or -X as appropriate.  */
  	case ABS_EXPR:
! 	  if (TREE_CODE (rhs1) == SSA_NAME
! 	      && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
  	    return simplify_abs_using_ranges (stmt);
  	  break;
  
*************** simplify_stmt_using_ranges (gimple_stmt_
*** 7390,7399 ****
  	  /* Optimize away BIT_AND_EXPR and BIT_IOR_EXPR
  	     if all the bits being cleared are already cleared or
  	     all the bits being set are already set.  */
! 	  if (INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (stmt))))
  	    return simplify_bit_ops_using_ranges (gsi, stmt);
  	  break;
  
  	default:
  	  break;
  	}
--- 7422,7437 ----
  	  /* Optimize away BIT_AND_EXPR and BIT_IOR_EXPR
  	     if all the bits being cleared are already cleared or
  	     all the bits being set are already set.  */
! 	  if (INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
  	    return simplify_bit_ops_using_ranges (gsi, stmt);
  	  break;
  
+ 	CASE_CONVERT:
+ 	  if (TREE_CODE (rhs1) == SSA_NAME
+ 	      && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
+ 	    return simplify_conversion_using_ranges (stmt);
+ 	  break;
+ 
  	default:
  	  break;
  	}
Index: gcc/testsuite/gcc.dg/tree-ssa/vrp58.c
===================================================================
*** gcc/testsuite/gcc.dg/tree-ssa/vrp58.c	(revision 0)
--- gcc/testsuite/gcc.dg/tree-ssa/vrp58.c	(revision 0)
***************
*** 0 ****
--- 1,12 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -fdump-tree-vrp1-details" } */
+ 
+ long long
+ foo (long long a, signed char b, signed char c)
+ {
+   int bc = b * c;
+   return a + (short)bc;
+ }
+ 
+ /* { dg-final { scan-tree-dump "Folded into" "vrp1" } } */
+ /* { dg-final { cleanup-tree-dump "vrp1" } } */

Re: [PATCH] Make VRP optimize useless conversions

[Michael Matz]

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.

Re: [PATCH] Make VRP optimize useless conversions

[Richard Guenther]

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.

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;

Re: [PATCH] Make VRP optimize useless conversions

[Michael Matz]

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.

Re: [PATCH] Make VRP optimize useless conversions

[Richard Guenther]

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.

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;
 }

Re: [PATCH] Make VRP optimize useless conversions

[Richard Guenther]

On Fri, 8 Jul 2011, Richard Guenther wrote:

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

The following actually works.

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

Can you double-check it?

Thanks,
Richard.

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

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

Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 176030)
+++ gcc/tree-vrp.c	(working copy)
@@ -7347,30 +7347,55 @@ 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;
-  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 the middle values do not represent a proper range fail.  */
+  if (double_int_cmp (middlemin, middlemax,
+		      TYPE_UNSIGNED (TREE_TYPE (middleop))) > 0)
     return false;
-  gimple_assign_set_rhs1 (stmt, rhs1);
+  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, innerop);
   update_stmt (stmt);
   return true;
 }

Re: [PATCH] Make VRP optimize useless conversions

[Michael Matz]

Hi,

On Mon, 11 Jul 2011, Richard Guenther wrote:

> The following actually works.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> 
> Can you double-check it?

Seems sensible.  Given this:

  short s;
  int i;
  for (s = 0; s <= 127; s++)
    i += (signed char)(unsigned char)s;
  return i;

(or similar), does it remove the conversions to signed and unsigned char 
now?  And does it _not_ remove them if the upper bound is 128, or the 
lower bound is -1 ?

Similar (now with extensions):
  signed char c;
  unsigned u;
  for (c = 1; c < 127; c++)
    u += (unsigned)(int)c;

The conversion to int is not necessary; but it is when the lower bound 
is -1.


Ciao,
Michael.

Build failure (Re: [PATCH] Make VRP optimize useless conversions)

[Ulrich Weigand]

Richard Guenther wrote:

> 2011-07-11  Richard Guenther  <rguenther@suse.de>
> 
> 	* tree-vrp.c (simplify_conversion_using_ranges): Manually
> 	translate the source value-range through the conversion chain.

This causes a build failure in cachemgr.c on spu-elf.  A slightly
modified simplified test case also fails on i386-linux:

void *
test (unsigned long long x, unsigned long long y)
{
  return (void *) (unsigned int) (x / y);
}

compiled with -O2 results in:

test.i: In function 'test':
test.i:3:1: error: invalid types in nop conversion
void *
long long unsigned int
D.1962_5 = (void *) D.1963_3;

test.i:3:1: internal compiler error: verify_gimple failed

Any thoughts?

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: Build failure (Re: [PATCH] Make VRP optimize useless conversions)

[Richard Guenther]

On Tue, 12 Jul 2011, Ulrich Weigand wrote:

> Richard Guenther wrote:
> 
> > 2011-07-11  Richard Guenther  <rguenther@suse.de>
> > 
> > 	* tree-vrp.c (simplify_conversion_using_ranges): Manually
> > 	translate the source value-range through the conversion chain.
> 
> This causes a build failure in cachemgr.c on spu-elf.  A slightly
> modified simplified test case also fails on i386-linux:
> 
> void *
> test (unsigned long long x, unsigned long long y)
> {
>   return (void *) (unsigned int) (x / y);
> }
> 
> compiled with -O2 results in:
> 
> test.i: In function 'test':
> test.i:3:1: error: invalid types in nop conversion
> void *
> long long unsigned int
> D.1962_5 = (void *) D.1963_3;
> 
> test.i:3:1: internal compiler error: verify_gimple failed
> 
> Any thoughts?

Fix in testing.

Richard.

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

	* tree-vrp.c (simplify_conversion_using_ranges): Make sure
	the final type is integral.

	* gcc.dg/torture/20110713-1.c: New testcase.

Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 176224)
+++ gcc/tree-vrp.c	(working copy)
@@ -7353,6 +7353,8 @@ simplify_conversion_using_ranges (gimple
   double_int innermin, innermax, middlemin, middlemax;
 
   finaltype = TREE_TYPE (gimple_assign_lhs (stmt));
+  if (!INTEGRAL_TYPE_P (finaltype))
+    return false;
   middleop = gimple_assign_rhs1 (stmt);
   def_stmt = SSA_NAME_DEF_STMT (middleop);
   if (!is_gimple_assign (def_stmt)
Index: gcc/testsuite/gcc.dg/torture/20110713-1.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/20110713-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/20110713-1.c	(revision 0)
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target ilp32 } */
+
+void *
+test (unsigned long long x, unsigned long long y)
+{
+    return (void *) (unsigned int) (x / y);
+}

Re: Build failure (Re: [PATCH] Make VRP optimize useless conversions)

[H.J. Lu]

On Wed, Jul 13, 2011 at 1:28 AM, Richard Guenther <rguenther@suse.de> wrote:
> On Tue, 12 Jul 2011, Ulrich Weigand wrote:
>
>> Richard Guenther wrote:
>>
>> > 2011-07-11  Richard Guenther  <rguenther@suse.de>
>> >
>> >     * tree-vrp.c (simplify_conversion_using_ranges): Manually
>> >     translate the source value-range through the conversion chain.
>>
>> This causes a build failure in cachemgr.c on spu-elf.  A slightly
>> modified simplified test case also fails on i386-linux:
>>
>> void *
>> test (unsigned long long x, unsigned long long y)
>> {
>>   return (void *) (unsigned int) (x / y);
>> }
>>
>> compiled with -O2 results in:
>>
>> test.i: In function 'test':
>> test.i:3:1: error: invalid types in nop conversion
>> void *
>> long long unsigned int
>> D.1962_5 = (void *) D.1963_3;
>>
>> test.i:3:1: internal compiler error: verify_gimple failed
>>
>> Any thoughts?
>
> Fix in testing.
>
> Richard.
>
> 2011-07-13  Richard Guenther  <rguenther@suse.de>
>
>        * tree-vrp.c (simplify_conversion_using_ranges): Make sure
>        the final type is integral.
>
>        * gcc.dg/torture/20110713-1.c: New testcase.
>
> Index: gcc/tree-vrp.c
> ===================================================================
> --- gcc/tree-vrp.c      (revision 176224)
> +++ gcc/tree-vrp.c      (working copy)
> @@ -7353,6 +7353,8 @@ simplify_conversion_using_ranges (gimple
>   double_int innermin, innermax, middlemin, middlemax;
>
>   finaltype = TREE_TYPE (gimple_assign_lhs (stmt));
> +  if (!INTEGRAL_TYPE_P (finaltype))
> +    return false;
>   middleop = gimple_assign_rhs1 (stmt);
>   def_stmt = SSA_NAME_DEF_STMT (middleop);
>   if (!is_gimple_assign (def_stmt)
> Index: gcc/testsuite/gcc.dg/torture/20110713-1.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/torture/20110713-1.c   (revision 0)
> +++ gcc/testsuite/gcc.dg/torture/20110713-1.c   (revision 0)
> @@ -0,0 +1,8 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target ilp32 } */
> +
> +void *
> +test (unsigned long long x, unsigned long long y)
> +{
> +    return (void *) (unsigned int) (x / y);
> +}
>

This also fixed:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49731


-- 
H.J.

Re: Build failure (Re: [PATCH] Make VRP optimize useless conversions)

[Ulrich Weigand]

Richard Guenther wrote:

> 2011-07-13  Richard Guenther  <rguenther@suse.de>
> 
> 	* tree-vrp.c (simplify_conversion_using_ranges): Make sure
> 	the final type is integral.

This fixes the spu-elf build failure.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Make VRP optimize useless conversions

[H.J. Lu]

On Mon, Jul 11, 2011 at 5:12 AM, Richard Guenther <rguenther@suse.de> wrote:
> On Fri, 8 Jul 2011, Richard Guenther wrote:
>
>> 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?
>
> The following actually works.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> Can you double-check it?
>
> Thanks,
> Richard.
>
> 2011-07-11  Richard Guenther  <rguenther@suse.de>
>
>        * tree-vrp.c (simplify_conversion_using_ranges): Manually
>        translate the source value-range through the conversion chain.
>

This may have caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

-- 
H.J.