diff mbox series

make range_int_cst_p work with any numeric range (VR_ANTI_RANGE, etc)

Message ID 1da55be1-7511-d303-c6d2-e49cfb0ae5b3@redhat.com
State New
Headers show
Series make range_int_cst_p work with any numeric range (VR_ANTI_RANGE, etc) | expand

Commit Message

Aldy Hernandez Nov. 5, 2019, 1:15 p.m. UTC 
The function range_int_cst_p only works with VR_RANGE's at the moment. 
This is silly because VR_ANTI_RANGE and even VR_VARYING can contain 
numeric bounds.  I have fixed this oversight and have made the function 
return the bounds in MIN/MAX.  This simplifies a lot of code, because 
there is no longer a need to special case VR_VARYING and VR_ANTI_RANGE, 
as well as pick at the individual range components outside of the API.

The patch has the pleasant side-effect of bringing more things into the 
API fold.  Basically, any access to either value_range::min(), max(), or 
kind(), is suspect and a big hint that the code should be rewritten to 
use the API (contains_p, varying_p, zero_p, etc).

One of the primary culprits of API noncompliance is the sprintf and 
strlen warning code.  Mind you, not due to negligence on the author, but 
because we had no value-range API when Martin added the passes.  I 
realize it's nobody's responsibility to fix older value-range code, and 
I'll probably end up doing it myself (next cycle??), but I could 
definitely use a hand from the experts, as it's intricate and delicate code.

Speak of which, in converting dump_strlen_info() to use the new 
range_int_cst_p, I noticed a lot of the code disappeared if we used the 
API.  Martin, if you'd prefer not to dump varying, undefined, etc, let 
me know and we can gate that call to vr.dump().  I took the liberty 
because it was simple, clean, and hidden away in an internal debugging 
helper.

OK for trunk?

Comments

Richard Biener Nov. 5, 2019, 2:27 p.m. UTC | #1 
On Tue, Nov 5, 2019 at 2:15 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> The function range_int_cst_p only works with VR_RANGE's at the moment.
> This is silly because VR_ANTI_RANGE and even VR_VARYING can contain
> numeric bounds.  I have fixed this oversight and have made the function
> return the bounds in MIN/MAX.  This simplifies a lot of code, because
> there is no longer a need to special case VR_VARYING and VR_ANTI_RANGE,
> as well as pick at the individual range components outside of the API.
>
> The patch has the pleasant side-effect of bringing more things into the
> API fold.  Basically, any access to either value_range::min(), max(), or
> kind(), is suspect and a big hint that the code should be rewritten to
> use the API (contains_p, varying_p, zero_p, etc).
>
> One of the primary culprits of API noncompliance is the sprintf and
> strlen warning code.  Mind you, not due to negligence on the author, but
> because we had no value-range API when Martin added the passes.  I
> realize it's nobody's responsibility to fix older value-range code, and
> I'll probably end up doing it myself (next cycle??), but I could
> definitely use a hand from the experts, as it's intricate and delicate code.
>
> Speak of which, in converting dump_strlen_info() to use the new
> range_int_cst_p, I noticed a lot of the code disappeared if we used the
> API.  Martin, if you'd prefer not to dump varying, undefined, etc, let
> me know and we can gate that call to vr.dump().  I took the liberty
> because it was simple, clean, and hidden away in an internal debugging
> helper.
>
> OK for trunk?

No.  It's a semantic change, no?  Don't we for VR_ANTI_RANGE
always get [-INF, +INF] back then?  Likewise for VARYING?
What do we get for UNDEFINED?  I think callers are not prepared
for this and expect it to return true for "useful" ranges only.

If you want this, use a new name, like get_range_bounds ().
Also not sure why min/max need to be INTEGER_CST, why
not _always_ return something (that is, the fucntion should never
need to return false).

The patch doesn't look like an improvement, it just adds to confusion.

Richard.
Andrew MacLeod Nov. 5, 2019, 3:20 p.m. UTC | #2 
On 11/5/19 9:27 AM, Richard Biener wrote:
> On Tue, Nov 5, 2019 at 2:15 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>> The function range_int_cst_p only works with VR_RANGE's at the moment.
>> This is silly because VR_ANTI_RANGE and even VR_VARYING can contain
>> numeric bounds.  I have fixed this oversight and have made the function
>> return the bounds in MIN/MAX.  This simplifies a lot of code, because
>> there is no longer a need to special case VR_VARYING and VR_ANTI_RANGE,
>> as well as pick at the individual range components outside of the API.
>>
>> The patch has the pleasant side-effect of bringing more things into the
>> API fold.  Basically, any access to either value_range::min(), max(), or
>> kind(), is suspect and a big hint that the code should be rewritten to
>> use the API (contains_p, varying_p, zero_p, etc).
>>
>> One of the primary culprits of API noncompliance is the sprintf and
>> strlen warning code.  Mind you, not due to negligence on the author, but
>> because we had no value-range API when Martin added the passes.  I
>> realize it's nobody's responsibility to fix older value-range code, and
>> I'll probably end up doing it myself (next cycle??), but I could
>> definitely use a hand from the experts, as it's intricate and delicate code.
>>
>> Speak of which, in converting dump_strlen_info() to use the new
>> range_int_cst_p, I noticed a lot of the code disappeared if we used the
>> API.  Martin, if you'd prefer not to dump varying, undefined, etc, let
>> me know and we can gate that call to vr.dump().  I took the liberty
>> because it was simple, clean, and hidden away in an internal debugging
>> helper.
>>
>> OK for trunk?
> No.  It's a semantic change, no?  Don't we for VR_ANTI_RANGE
> always get [-INF, +INF] back then?  Likewise for VARYING?
> What do we get for UNDEFINED?  I think callers are not prepared
> for this and expect it to return true for "useful" ranges only.
>
> If you want this, use a new name, like get_range_bounds ().
> Also not sure why min/max need to be INTEGER_CST, why
> not _always_ return something (that is, the fucntion should never
> need to return false).
>
> The patch doesn't look like an improvement, it just adds to confusion.
>
> Richard.
which  is the direction I was going to go too. In fact  I do not think 
this functionality is needed at all.

The functionality you are partially implementing here is basically just 
asking for lbound() and ubound ().. that already looks through all these 
cases.

Andrew
Aldy Hernandez Nov. 5, 2019, 3:28 p.m. UTC | #3 
On 11/5/19 3:27 PM, Richard Biener wrote:
> On Tue, Nov 5, 2019 at 2:15 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>>
>> The function range_int_cst_p only works with VR_RANGE's at the moment.
>> This is silly because VR_ANTI_RANGE and even VR_VARYING can contain
>> numeric bounds.  I have fixed this oversight and have made the function
>> return the bounds in MIN/MAX.  This simplifies a lot of code, because
>> there is no longer a need to special case VR_VARYING and VR_ANTI_RANGE,
>> as well as pick at the individual range components outside of the API.
>>
>> The patch has the pleasant side-effect of bringing more things into the
>> API fold.  Basically, any access to either value_range::min(), max(), or
>> kind(), is suspect and a big hint that the code should be rewritten to
>> use the API (contains_p, varying_p, zero_p, etc).
>>
>> One of the primary culprits of API noncompliance is the sprintf and
>> strlen warning code.  Mind you, not due to negligence on the author, but
>> because we had no value-range API when Martin added the passes.  I
>> realize it's nobody's responsibility to fix older value-range code, and
>> I'll probably end up doing it myself (next cycle??), but I could
>> definitely use a hand from the experts, as it's intricate and delicate code.
>>
>> Speak of which, in converting dump_strlen_info() to use the new
>> range_int_cst_p, I noticed a lot of the code disappeared if we used the
>> API.  Martin, if you'd prefer not to dump varying, undefined, etc, let
>> me know and we can gate that call to vr.dump().  I took the liberty
>> because it was simple, clean, and hidden away in an internal debugging
>> helper.
>>
>> OK for trunk?
> 
> No.  It's a semantic change, no?  Don't we for VR_ANTI_RANGE
> always get [-INF, +INF] back then?  Likewise for VARYING?
> What do we get for UNDEFINED?  I think callers are not prepared
> for this and expect it to return true for "useful" ranges only.
> 
> If you want this, use a new name, like get_range_bounds ().
> Also not sure why min/max need to be INTEGER_CST, why
> not _always_ return something (that is, the fucntion should never
> need to return false).
> 
> The patch doesn't look like an improvement, it just adds to confusion.

Perhaps you're right, and as Andrew points out, the API already provides 
{lower,upper}_bound().  I'll shelf this for now, and clean up calls to 
min(), max(), and kind() at a later time.

Thanks.
Aldy
diff mbox series

Patch

commit cba4b59ef2e0e6821d63cfa959d201f22534eb69
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Tue Nov 5 10:54:22 2019 +0100

    Make range_int_cst_p work with any numeric range, not just VR_RANGE.
    This includes VR_ANTI_RANGE as well as VR_VARYING, as they can also
    have numeric bounds.  Add two new arguments to return the MIN/MAX
    bounds.
    
    Remove the now redundant range_has_numeric_bounds_p.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 562b69d1aab..5f12d166176 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -48,6 +48,30 @@ 
 	* fold-const.c (operand_compare::hash_operand): Remove
 	FIELD_DECL handling.
 
+2019-11-05  Aldy Hernandez  <aldyh@redhat.com>
+
+	* gimple-ssa-sprintf.c (get_int_range): Call range_int_cst_p with
+	min/max arguments.
+	(format_integer): Same.
+	(handle_printf_call): Same.
+	* tree-ssa-strlen.c (compare_nonzero_chars): Same.
+	(dump_strlen_info): Same.
+	(get_range_strlen_dynamic): Same.
+	(count_nonzero_bytes): Same.
+	* tree-vrp.c (range_has_numeric_bounds_p): Remove.
+	(extract_range_from_pointer_plus_expr): Call range_int_cst_p with
+	min/max arguments.
+	(value_range_base::normalize_addresses): Use range_int_cst_p
+	instead of removed range_has_numeric_bounds_p.
+	(range_int_cst_p): New MIN/MAX arguments.
+	* tree-vrp.h (range_int_cst_p): Add two new arguments.
+	* vr-values.c (r_values::check_for_binary_op_overflow): Call
+	range_int_cst_p with min/max arguments.
+	(vr_values::simplify_div_or_mod_using_ranges): Same.
+	(vr_set_zero_nonzero_bits): Same.
+	(range_fits_type_p): Use value_range_base::supports_type_p instead
+	of open-coding the test.
+
 2019-11-05  Aldy Hernandez  <aldyh@redhat.com>
 
 	* tree-vrp.h (vrp_bitmap_equal_p): Remove.
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index b548bbd95e3..0029cfc258c 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -1023,18 +1023,12 @@  get_int_range (tree arg, HOST_WIDE_INT *pmin, HOST_WIDE_INT *pmax,
 	  /* Try to determine the range of values of the integer argument.  */
 	  const value_range *vr
 	    = CONST_CAST (class vr_values *, vr_values)->get_value_range (arg);
+	  tree min, max;
 
-	  if (range_int_cst_p (vr))
+	  if (!vr->varying_p () && range_int_cst_p (vr, &min, &max))
 	    {
-	      HOST_WIDE_INT type_min
-		= (TYPE_UNSIGNED (argtype)
-		   ? tree_to_uhwi (TYPE_MIN_VALUE (argtype))
-		   : tree_to_shwi (TYPE_MIN_VALUE (argtype)));
-
-	      HOST_WIDE_INT type_max = tree_to_uhwi (TYPE_MAX_VALUE (argtype));
-
-	      *pmin = TREE_INT_CST_LOW (vr->min ());
-	      *pmax = TREE_INT_CST_LOW (vr->max ());
+	      *pmin = TREE_INT_CST_LOW (min);
+	      *pmax = TREE_INT_CST_LOW (max);
 
 	      if (*pmin < *pmax)
 		{
@@ -1044,8 +1038,12 @@  get_int_range (tree arg, HOST_WIDE_INT *pmin, HOST_WIDE_INT *pmax,
 		     and its upper bound is in excess of TYPE_MAX.  In
 		     that (invalid) case disregard the range and use that
 		     of the expected type instead.  */
+		  HOST_WIDE_INT type_min
+		    = (TYPE_UNSIGNED (argtype)
+		       ? tree_to_uhwi (TYPE_MIN_VALUE (argtype))
+		       : tree_to_shwi (TYPE_MIN_VALUE (argtype)));
+		  HOST_WIDE_INT type_max = tree_to_uhwi (TYPE_MAX_VALUE (argtype));
 		  knownrange = type_min < *pmin || *pmax < type_max;
-
 		  unknown = false;
 		}
 	    }
@@ -1326,11 +1324,8 @@  format_integer (const directive &dir, tree arg, const vr_values *vr_values)
       const value_range *vr
 	= CONST_CAST (class vr_values *, vr_values)->get_value_range (arg);
 
-      if (range_int_cst_p (vr))
+      if (range_int_cst_p (vr, &argmin, &argmax))
 	{
-	  argmin = vr->min ();
-	  argmax = vr->max ();
-
 	  /* Set KNOWNRANGE if the argument is in a known subrange
 	     of the directive's type and neither width nor precision
 	     is unknown.  (KNOWNRANGE may be reset below).  */
@@ -1342,11 +1337,7 @@  format_integer (const directive &dir, tree arg, const vr_values *vr_values)
 	  res.argmin = argmin;
 	  res.argmax = argmax;
 	}
-      else if (vr->kind () == VR_ANTI_RANGE)
-	{
-	  /* Handle anti-ranges if/when bug 71690 is resolved.  */
-	}
-      else if (vr->varying_p () || vr->undefined_p ())
+      else if (vr->undefined_p ())
 	{
 	  /* The argument here may be the result of promoting the actual
 	     argument to int.  Try to determine the type of the actual
@@ -4089,11 +4080,12 @@  handle_printf_call (gimple_stmt_iterator *gsi, const vr_values *vr_values)
 	     of them at level 2.  */
 	  const value_range *vr
 	    = CONST_CAST (class vr_values *, vr_values)->get_value_range (size);
+	  tree min, max;
 
-	  if (range_int_cst_p (vr))
+	  if (range_int_cst_p (vr, &min, &max))
 	    {
-	      unsigned HOST_WIDE_INT minsize = TREE_INT_CST_LOW (vr->min ());
-	      unsigned HOST_WIDE_INT maxsize = TREE_INT_CST_LOW (vr->max ());
+	      unsigned HOST_WIDE_INT minsize = TREE_INT_CST_LOW (min);
+	      unsigned HOST_WIDE_INT maxsize = TREE_INT_CST_LOW (max);
 	      dstsize = warn_level < 2 ? maxsize : minsize;
 
 	      if (minsize > target_int_max ())
@@ -4107,13 +4099,6 @@  handle_printf_call (gimple_stmt_iterator *gsi, const vr_values *vr_values)
 	      if (maxsize > target_int_max ())
 		posunder4k = false;
 	    }
-	  else if (vr->varying_p ())
-	    {
-	      /* POSIX requires snprintf to fail if DSTSIZE is greater
-		 than INT_MAX.  Since SIZE's range is unknown, avoid
-		 folding.  */
-	      posunder4k = false;
-	    }
 
 	  /* The destination size is not constant.  If the function is
 	     bounded (e.g., snprintf) a lower bound of zero doesn't
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 50cc442a61f..25a075d3dc5 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -228,17 +228,16 @@  compare_nonzero_chars (strinfo *si, unsigned HOST_WIDE_INT off,
   const value_range *vr
     = (CONST_CAST (class vr_values *, rvals)
        ->get_value_range (si->nonzero_chars));
-
-  value_range_kind rng = vr->kind ();
-  if (rng != VR_RANGE || !range_int_cst_p (vr))
+  tree min, max;
+  if (!range_int_cst_p (vr, &min, &max))
     return -1;
 
   /* If the offset is less than the minimum length or if the bounds
      of the length range are equal return the result of the comparison
      same as in the constant case.  Otherwise return a conservative
      result.  */
-  int cmpmin = compare_tree_int (vr->min (), off);
-  if (cmpmin > 0 || tree_int_cst_equal (vr->min (), vr->max ()))
+  int cmpmin = compare_tree_int (min, off);
+  if (cmpmin > 0 || tree_int_cst_equal (min, max))
     return cmpmin;
 
   return -1;
@@ -795,32 +794,19 @@  dump_strlen_info (FILE *fp, gimple *stmt, const vr_values *rvals)
 	      print_generic_expr (fp, si->nonzero_chars);
 	      if (TREE_CODE (si->nonzero_chars) == SSA_NAME)
 		{
-		  value_range_kind rng = VR_UNDEFINED;
-		  wide_int min, max;
+		  value_range_base vr;
 		  if (rvals)
 		    {
-		      const value_range *vr
+		      const value_range *vr_p
 			= CONST_CAST (class vr_values *, rvals)
 			->get_value_range (si->nonzero_chars);
-		      rng = vr->kind ();
-		      if (range_int_cst_p (vr))
-			{
-			  min = wi::to_wide (vr->min ());
-			  max = wi::to_wide (vr->max ());
-			}
-		      else
-			rng = VR_UNDEFINED;
+		      if (vr_p)
+			vr = *vr_p;
 		    }
 		  else
-		    rng = get_range_info (si->nonzero_chars, &min, &max);
+		    get_range_info (si->nonzero_chars, vr);
 
-		  if (rng == VR_RANGE || rng == VR_ANTI_RANGE)
-		    {
-		      fprintf (fp, " %s[%llu, %llu]",
-			       rng == VR_RANGE ? "" : "~",
-			       (long long) min.to_uhwi (),
-			       (long long) max.to_uhwi ());
-		    }
+		  vr.dump (fp);
 		}
 	      fprintf (fp, " , refcount = %i", si->refcount);
 	      if (si->stmt)
@@ -994,13 +980,7 @@  get_range_strlen_dynamic (tree src, c_strlen_data *pdata, bitmap *visited,
 	      const value_range *vr
 		= CONST_CAST (class vr_values *, rvals)
 		->get_value_range (si->nonzero_chars);
-	      if (vr->kind () == VR_RANGE
-		  && range_int_cst_p (vr))
-		{
-		  pdata->minlen = vr->min ();
-		  pdata->maxlen = vr->max ();
-		}
-	      else
+	      if (!range_int_cst_p (vr, &pdata->minlen, &pdata->maxlen))
 		pdata->minlen = build_zero_cst (size_type_node);
 	    }
 	  else
@@ -1034,13 +1014,8 @@  get_range_strlen_dynamic (tree src, c_strlen_data *pdata, bitmap *visited,
 	  const value_range *vr
 	    = CONST_CAST (class vr_values *, rvals)
 	    ->get_value_range (si->nonzero_chars);
-	  if (vr->kind () == VR_RANGE
-	      && range_int_cst_p (vr))
-	    {
-	      pdata->minlen = vr->min ();
-	      pdata->maxlen = vr->max ();
-	      pdata->maxbound = pdata->maxlen;
-	    }
+	  if (range_int_cst_p (vr, &pdata->minlen, &pdata->maxlen))
+	    pdata->maxbound = pdata->maxlen;
 	  else
 	    {
 	      pdata->minlen = build_zero_cst (size_type_node);
@@ -4043,12 +4018,14 @@  count_nonzero_bytes (tree exp, unsigned HOST_WIDE_INT offset,
 	  const value_range *vr
 	    = CONST_CAST (class vr_values *, rvals)
 	    ->get_value_range (si->nonzero_chars);
-	  if (vr->kind () != VR_RANGE
-	      || !range_int_cst_p (vr))
+	  tree min, max;
+	  if (range_int_cst_p (vr, &min, &max))
+	    {
+	      minlen = tree_to_uhwi (min);
+	      maxlen = tree_to_uhwi (max);
+	    }
+	  else
 	    return false;
-
-	 minlen = tree_to_uhwi (vr->min ());
-	 maxlen = tree_to_uhwi (vr->max ());
 	}
       else
 	return false;
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index e1d5c7cb98c..b7d7e5e6646 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -922,21 +922,28 @@  vrp_operand_equal_p (const_tree val1, const_tree val2)
   return true;
 }
 
-static bool
-range_has_numeric_bounds_p (const value_range_base *vr)
-{
-  return (vr->min ()
-	  && TREE_CODE (vr->min ()) == INTEGER_CST
-	  && TREE_CODE (vr->max ()) == INTEGER_CST);
-}
+/* Return true if range is an integer constant, though not necessarily
+   a singleton.  This can be true for a VR_RANGE, VR_ANTI_RANGE, or
+   VR_VARYING, as all of them can contain an integer constant range.
 
-/* Return true if max and min of VR are INTEGER_CST.  It's not necessary
-   a singleton.  */
+   If true, and MIN/MAX are non-zero, set them to the lower and upper
+   bounds of the range respectively.  */
 
 bool
-range_int_cst_p (const value_range_base *vr)
+range_int_cst_p (const value_range_base *vr, tree *min, tree *max)
 {
-  return (vr->kind () == VR_RANGE && range_has_numeric_bounds_p (vr));
+  if (vr->min ()
+      && TREE_CODE (vr->min ()) == INTEGER_CST
+      && TREE_CODE (vr->max ()) == INTEGER_CST)
+    {
+      tree type = vr->type ();
+      if (min)
+	*min = wide_int_to_tree (type, vr->lower_bound ());
+      if (max)
+	*max = wide_int_to_tree (type, vr->upper_bound ());
+      return true;
+    }
+  return false;
 }
 
 /* Return the single symbol (an SSA_NAME) contained in T if any, or NULL_TREE
@@ -1511,12 +1518,12 @@  extract_range_from_pointer_plus_expr (value_range_base *vr,
      doesn't either.  As the second operand is sizetype (unsigned),
      consider all ranges where the MSB could be set as possible
      subtractions where the result might be NULL.  */
-  if ((!range_includes_zero_p (vr0)
-       || !range_includes_zero_p (vr1))
+  tree vr1min, vr1max;
+  if ((!range_includes_zero_p (vr0) || !range_includes_zero_p (vr1))
       && !TYPE_OVERFLOW_WRAPS (expr_type)
       && (flag_delete_null_pointer_checks
-	  || (range_int_cst_p (vr1)
-	      && !tree_int_cst_sign_bit (vr1->max ()))))
+	  || (range_int_cst_p (vr1, &vr1min, &vr1max)
+	      && !tree_int_cst_sign_bit (vr1max))))
     vr->set_nonzero (expr_type);
   else if (vr0->zero_p () && vr1->zero_p ())
     vr->set_zero (expr_type);
@@ -6054,7 +6061,7 @@  value_range_base::normalize_addresses () const
   if (undefined_p ())
     return *this;
 
-  if (!POINTER_TYPE_P (type ()) || range_has_numeric_bounds_p (this))
+  if (!POINTER_TYPE_P (type ()) || range_int_cst_p (this))
     return *this;
 
   if (!range_includes_zero_p (this))
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
index 0bf33caba85..b0d5ae3e877 100644
--- a/gcc/tree-vrp.h
+++ b/gcc/tree-vrp.h
@@ -279,7 +279,8 @@  extern void register_edge_assert_for (tree, edge, enum tree_code,
 extern bool stmt_interesting_for_vrp (gimple *);
 extern bool infer_value_range (gimple *, tree, tree_code *, tree *);
 
-extern bool range_int_cst_p (const value_range_base *);
+extern bool range_int_cst_p (const value_range_base *,
+			     tree * = NULL, tree * = NULL);
 
 extern int compare_values (tree, tree);
 extern int compare_values_warnv (tree, tree, bool *);
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index d1713bf4e0e..ceda5cbdc9f 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -1022,16 +1022,15 @@  vr_values::check_for_binary_op_overflow (enum tree_code subcode, tree type,
   else
     vr1.set_varying (TREE_TYPE (op1));
 
-  tree vr0min = vr0.min (), vr0max = vr0.max ();
-  tree vr1min = vr1.min (), vr1max = vr1.max ();
-  if (!range_int_cst_p (&vr0)
+  tree vr0min, vr0max, vr1min, vr1max;
+  if (!range_int_cst_p (&vr0, &vr0min, &vr0max)
       || TREE_OVERFLOW (vr0min)
       || TREE_OVERFLOW (vr0max))
     {
       vr0min = vrp_val_min (TREE_TYPE (op0));
       vr0max = vrp_val_max (TREE_TYPE (op0));
     }
-  if (!range_int_cst_p (&vr1)
+  if (!range_int_cst_p (&vr1, &vr1min, &vr1max)
       || TREE_OVERFLOW (vr1min)
       || TREE_OVERFLOW (vr1max))
     {
@@ -3138,19 +3137,14 @@  vr_values::simplify_div_or_mod_using_ranges (gimple_stmt_iterator *gsi,
   else
     {
       vr = get_value_range (op0);
-      if (range_int_cst_p (vr))
-	{
-	  op0min = vr->min ();
-	  op0max = vr->max ();
-	}
+      range_int_cst_p (vr, &op0min, &op0max);
     }
 
   if (rhs_code == TRUNC_MOD_EXPR
       && TREE_CODE (op1) == SSA_NAME)
     {
       const value_range *vr1 = get_value_range (op1);
-      if (range_int_cst_p (vr1))
-	op1min = vr1->min ();
+      range_int_cst_p (vr1, &op1min);
     }
   if (rhs_code == TRUNC_MOD_EXPR
       && TREE_CODE (op1min) == INTEGER_CST
@@ -3352,11 +3346,11 @@  vr_set_zero_nonzero_bits (const tree expr_type,
 			  wide_int *may_be_nonzero,
 			  wide_int *must_be_nonzero)
 {
-  if (range_int_cst_p (vr))
+  tree min, max;
+  if (range_int_cst_p (vr, &min, &max))
     {
       wi_set_zero_nonzero_bits (expr_type,
-				wi::to_wide (vr->min ()),
-				wi::to_wide (vr->max ()),
+				wi::to_wide (min), wi::to_wide (max),
 				*may_be_nonzero, *must_be_nonzero);
       return true;
     }
@@ -3525,8 +3519,7 @@  range_fits_type_p (const value_range *vr,
 
   /* We can only handle integral and pointer types.  */
   src_type = vr->type ();
-  if (!INTEGRAL_TYPE_P (src_type)
-      && !POINTER_TYPE_P (src_type))
+  if (!value_range_base::supports_type_p (src_type))
     return false;
 
   /* An extension is fine unless VR is SIGNED and dest_sgn is UNSIGNED,
@@ -3539,7 +3532,8 @@  range_fits_type_p (const value_range *vr,
     return true;
 
   /* Now we can only handle ranges with constant bounds.  */
-  if (!range_int_cst_p (vr))
+  tree min, max;
+  if (!range_int_cst_p (vr, &min, &max))
     return false;
 
   /* For sign changes, the MSB of the wide_int has to be clear.
@@ -3547,17 +3541,17 @@  range_fits_type_p (const value_range *vr,
      a signed wide_int, while a negative value cannot be represented
      by an unsigned wide_int.  */
   if (src_sgn != dest_sgn
-      && (wi::lts_p (wi::to_wide (vr->min ()), 0)
-	  || wi::lts_p (wi::to_wide (vr->max ()), 0)))
+      && (wi::lts_p (wi::to_wide (min), 0)
+	  || wi::lts_p (wi::to_wide (max), 0)))
     return false;
 
   /* Then we can perform the conversion on both ends and compare
      the result for equality.  */
-  tem = wi::ext (wi::to_widest (vr->min ()), dest_precision, dest_sgn);
-  if (tem != wi::to_widest (vr->min ()))
+  tem = wi::ext (wi::to_widest (min), dest_precision, dest_sgn);
+  if (tem != wi::to_widest (min))
     return false;
-  tem = wi::ext (wi::to_widest (vr->max ()), dest_precision, dest_sgn);
-  if (tem != wi::to_widest (vr->max ()))
+  tem = wi::ext (wi::to_widest (max), dest_precision, dest_sgn);
+  if (tem != wi::to_widest (max))
     return false;
 
   return true;