Re: VRP: abstract out wide int CONVERT_EXPR_P code

[Aldy Hernandez <aldyh@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2218 bytes --]



On 08/28/2018 05:27 AM, Richard Biener wrote:
> On Mon, Aug 27, 2018 at 2:24 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>>
>> Howdy!
>>
>> Phew, I think this is the last abstraction.  This handles the unary
>> CONVERT_EXPR_P code.
>>
>> It's the usual story-- normalize the symbolics to [-MIN,+MAX] and handle
>> everything generically.
>>
>> Normalizing the symbolics brought about some nice surprises.  We now
>> handle a few things we were punting on before, which I've documented in
>> the patch, but can remove if so desired.  I wrote them mainly for myself:
>>
>> /* NOTES: Previously we were returning VARYING for all symbolics, but
>>      we can do better by treating them as [-MIN, +MAX].  For
>>      example, converting [SYM, SYM] from INT to LONG UNSIGNED,
>>      we can return: ~[0x8000000, 0xffffffff7fffffff].
>>
>>      We were also failing to convert ~[0,0] from char* to unsigned,
>>      instead choosing to return VR_VARYING.  Now we return ~[0,0].  */
>>
>> Tested on x86-64 by the usual bootstrap and regtest gymnastics,
>> including --enable-languages=all, because my past sins are still
>> haunting me.
>>
>> OK?
> 
> The new wide_int_range_convert_tree looks odd given it returns
> tree's.  I'd have expected an API that does the conversion resulting
> in a wide_int range and the VRP code adapting to that by converting
> the result to trees.

Rewritten as suggested.

A few notes.

First.  I am not using widest_int as was done previously.  We were 
passing 0/false to the overflow args in force_fit_type so the call was 
just degrading into wide_int_to_tree() anyhow.  Am I missing some 
subtlety here, and must we use widest_int and then force_fit_type on the 
caller?

Second.  I need extract_range_into_wide_ints to work with anti ranges of 
constants.  It seems like only the unary handling of CONVERT_EXPR here 
uses it that way, so I believe it should go into one patch.  If you 
believe strongly otherwise, I could split out the 4 lines into a 
separate patch, but I'd prefer not to.

Finally, I could use vr0_min.get_precision() instead of inner_precision, 
but I think it's more regular and readable the way I have it.

Tested on all languages on x86-64 Linux.

OK for trunk?

[-- Attachment #2: curr.patch --]
[-- Type: text/x-patch, Size: 6640 bytes --]

gcc/

	* wide-int-range.cc (wide_int_range_convert): New.
	* wide-int-range.h (wide_int_range_convert): New.
	* tree-vrp.c (extract_range_from_unary_expr): Abstract wide int
	code into wide_int_range_convert.
	(extract_range_into_wide_ints): Do not munge anti range constants
	into the entire domain.  Just return the range back.

diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index f20730a85ba..a78ed45c713 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -995,7 +995,7 @@ ranges_from_anti_range (value_range *ar,
 /* Extract the components of a value range into a pair of wide ints in
    [WMIN, WMAX].
 
-   If the value range is anything but a VR_RANGE of constants, the
+   If the value range is anything but a VR_*RANGE of constants, the
    resulting wide ints are set to [-MIN, +MAX] for the type.  */
 
 static void inline
@@ -1003,7 +1003,10 @@ extract_range_into_wide_ints (value_range *vr,
 			      signop sign, unsigned prec,
 			      wide_int &wmin, wide_int &wmax)
 {
-  if (range_int_cst_p (vr))
+  if ((vr->type == VR_RANGE
+       || vr->type == VR_ANTI_RANGE)
+      && TREE_CODE (vr->min) == INTEGER_CST
+      && TREE_CODE (vr->max) == INTEGER_CST)
     {
       wmin = wi::to_wide (vr->min);
       wmax = wi::to_wide (vr->max);
@@ -1894,44 +1897,41 @@ extract_range_from_unary_expr (value_range *vr,
 	  return;
 	}
 
-      /* If VR0 is varying and we increase the type precision, assume
-	 a full range for the following transformation.  */
-      if (vr0.type == VR_VARYING
-	  && INTEGRAL_TYPE_P (inner_type)
-	  && TYPE_PRECISION (inner_type) < TYPE_PRECISION (outer_type))
-	{
-	  vr0.type = VR_RANGE;
-	  vr0.min = TYPE_MIN_VALUE (inner_type);
-	  vr0.max = TYPE_MAX_VALUE (inner_type);
-	}
-
-      /* If VR0 is a constant range or anti-range and the conversion is
-	 not truncating we can convert the min and max values and
-	 canonicalize the resulting range.  Otherwise we can do the
-	 conversion if the size of the range is less than what the
-	 precision of the target type can represent and the range is
-	 not an anti-range.  */
-      if ((vr0.type == VR_RANGE
-	   || vr0.type == VR_ANTI_RANGE)
+      /* We normalize everything to a VR_RANGE, but for constant
+	 anti-ranges we must handle them by leaving the final result
+	 as an anti range.  This allows us to convert things like
+	 ~[0,5] seamlessly.  */
+      value_range_type vr_type = VR_RANGE;
+      if (vr0.type == VR_ANTI_RANGE
 	  && TREE_CODE (vr0.min) == INTEGER_CST
-	  && TREE_CODE (vr0.max) == INTEGER_CST
-	  && (TYPE_PRECISION (outer_type) >= TYPE_PRECISION (inner_type)
-	      || (vr0.type == VR_RANGE
-		  && integer_zerop (int_const_binop (RSHIFT_EXPR,
-		       int_const_binop (MINUS_EXPR, vr0.max, vr0.min),
-		         size_int (TYPE_PRECISION (outer_type)))))))
+	  && TREE_CODE (vr0.max) == INTEGER_CST)
+	vr_type = VR_ANTI_RANGE;
+
+      /* NOTES: Previously we were returning VARYING for all symbolics, but
+	 we can do better by treating them as [-MIN, +MAX].  For
+	 example, converting [SYM, SYM] from INT to LONG UNSIGNED,
+	 we can return: ~[0x8000000, 0xffffffff7fffffff].
+
+	 We were also failing to convert ~[0,0] from char* to unsigned,
+	 instead choosing to return VR_VARYING.  Now we return ~[0,0].  */
+      wide_int vr0_min, vr0_max, wmin, wmax;
+      signop inner_sign = TYPE_SIGN (inner_type);
+      signop outer_sign = TYPE_SIGN (outer_type);
+      unsigned inner_prec = TYPE_PRECISION (inner_type);
+      unsigned outer_prec = TYPE_PRECISION (outer_type);
+      extract_range_into_wide_ints (&vr0, inner_sign, inner_prec,
+				    vr0_min, vr0_max);
+      if (wide_int_range_convert (wmin, wmax,
+				  inner_sign, inner_prec,
+				  outer_sign, outer_prec,
+				  vr0_min, vr0_max))
 	{
-	  tree new_min, new_max;
-	  new_min = force_fit_type (outer_type, wi::to_widest (vr0.min),
-				    0, false);
-	  new_max = force_fit_type (outer_type, wi::to_widest (vr0.max),
-				    0, false);
-	  set_and_canonicalize_value_range (vr, vr0.type,
-					    new_min, new_max, NULL);
-	  return;
+	  tree min = wide_int_to_tree (outer_type, wmin);
+	  tree max = wide_int_to_tree (outer_type, wmax);
+	  set_and_canonicalize_value_range (vr, vr_type, min, max, NULL);
 	}
-
-      set_value_range_to_varying (vr);
+      else
+	set_value_range_to_varying (vr);
       return;
     }
   else if (code == ABS_EXPR)
diff --git a/gcc/wide-int-range.cc b/gcc/wide-int-range.cc
index 3cdcede04cd..5f257966819 100644
--- a/gcc/wide-int-range.cc
+++ b/gcc/wide-int-range.cc
@@ -665,6 +665,39 @@ wide_int_range_abs (wide_int &min, wide_int &max,
   return true;
 }
 
+/* Convert range in [VR0_MIN, VR0_MAX] with INNER_SIGN and INNER_PREC,
+   to a range in [MIN, MAX] with OUTER_SIGN and OUTER_PREC.
+
+   Return TRUE if we were able to successfully calculate the new range.
+
+   Caller is responsible for canonicalizing the resulting range.  */
+
+bool
+wide_int_range_convert (wide_int &min, wide_int &max,
+			signop inner_sign,
+			unsigned inner_prec,
+			signop outer_sign,
+			unsigned outer_prec,
+			const wide_int &vr0_min,
+			const wide_int &vr0_max)
+{
+  /* If the conversion is not truncating we can convert the min and
+     max values and canonicalize the resulting range.  Otherwise we
+     can do the conversion if the size of the range is less than what
+     the precision of the target type can represent.  */
+  if (outer_prec >= inner_prec
+      || wi::rshift (wi::sub (vr0_max, vr0_min),
+		     wi::uhwi (outer_prec, inner_prec),
+		     inner_sign) == 0)
+    {
+      min = wide_int::from (vr0_min, outer_prec, inner_sign);
+      max = wide_int::from (vr0_max, outer_prec, inner_sign);
+      return (!wi::eq_p (min, wi::min_value (outer_prec, outer_sign))
+	      || !wi::eq_p (max, wi::max_value (outer_prec, outer_sign)));
+    }
+  return false;
+}
+
 /* Calculate a division operation on two ranges and store the result in
    [WMIN, WMAX] U [EXTRA_MIN, EXTRA_MAX].
 
diff --git a/gcc/wide-int-range.h b/gcc/wide-int-range.h
index 427ef34c6b4..83574b7cac1 100644
--- a/gcc/wide-int-range.h
+++ b/gcc/wide-int-range.h
@@ -99,6 +99,13 @@ extern bool wide_int_range_abs (wide_int &min, wide_int &max,
 				const wide_int &vr0_min,
 				const wide_int &vr0_max,
 				bool overflow_undefined);
+extern bool wide_int_range_convert (wide_int &min, wide_int &max,
+				    signop inner_sign,
+				    unsigned inner_prec,
+				    signop outer_sign,
+				    unsigned outer_prec,
+				    const wide_int &vr0_min,
+				    const wide_int &vr0_max);
 extern bool wide_int_range_div (wide_int &wmin, wide_int &wmax,
 				enum tree_code code,
 				signop sign, unsigned prec,