Re: PING: Fwd: [PATCH 2/2] Decouple adjust_range_from_scev from vr_values and value_range_equiv.

[Aldy Hernandez <aldyh@redhat.com>]

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



On 8/17/20 5:59 PM, Andrew MacLeod wrote:
> On 8/17/20 6:04 AM, Aldy Hernandez wrote:
>>
>>
>> On 8/14/20 7:16 PM, Andrew MacLeod wrote:
>>> On 8/14/20 12:05 PM, Aldy Hernandez wrote:
>>>> I made some minor changes to the function comments.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>>     * vr-values.c (check_for_binary_op_overflow): Change type of store
>>>>     to range_query.
>>>>     (vr_values::adjust_range_with_scev): Abstract most of the code...
>>>>     (range_of_var_in_loop): ...here.  Remove value_range_equiv uses.
>>>>     (simplify_using_ranges::simplify_using_ranges): Change type of 
>>>> store
>>>>     to range_query.
>>>>     * vr-values.h (class range_query): New.
>>>>     (class simplify_using_ranges): Use range_query.
>>>>     (class vr_values): Add OVERRIDE to get_value_range.
>>>>     (range_of_var_in_loop): New.
>>>> ---
>>>>  gcc/vr-values.c | 150 ++++++++++++++++++++++--------------------------
>>>>  gcc/vr-values.h |  23 ++++++--
>>>>  2 files changed, 88 insertions(+), 85 deletions(-)
>>>>
>>>> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
>>>> index 9002d87c14b..5b7bae3bfb7 100644
>>>> --- a/gcc/vr-values.c
>>>> +++ b/gcc/vr-values.c
>>>> @@ -1004,7 +1004,7 @@ vr_values::extract_range_from_comparison 
>>>> (value_range_equiv *vr,
>>>>     overflow.  */
>>>>
>>>>  static bool
>>>> -check_for_binary_op_overflow (vr_values *store,
>>>> +check_for_binary_op_overflow (range_query *store,
>>>>                    enum tree_code subcode, tree type,
>>>>                    tree op0, tree op1, bool *ovf)
>>>>  {
>>>> @@ -1737,22 +1737,18 @@ compare_range_with_value (enum tree_code 
>>>> comp, const value_range *vr,
>>>>
>>>>    gcc_unreachable ();
>>>>  }
>>>> -/* Given a range VR, a LOOP and a variable VAR, determine whether it
>>>> -   would be profitable to adjust VR using scalar evolution information
>>>> -   for VAR.  If so, update VR with the new limits.  */
>>>> +
>>>> +/* Given a VAR in STMT within LOOP, determine the range of the
>>>> +   variable and store it in VR.  If no range can be determined, the
>>>> +   resulting range will be set to VARYING.  */
>>>>
>>>>  void
>>>> -vr_values::adjust_range_with_scev (value_range_equiv *vr, class 
>>>> loop *loop,
>>>> -                   gimple *stmt, tree var)
>>>> +range_of_var_in_loop (irange *vr, range_query *query,
>>>> +              class loop *loop, gimple *stmt, tree var)
>>>>  {
>>>> -  tree init, step, chrec, tmin, tmax, min, max, type, tem;
>>>> +  tree init, step, chrec, tmin, tmax, min, max, type;
>>>>    enum ev_direction dir;
>>>>
>>>> -  /* TODO.  Don't adjust anti-ranges.  An anti-range may provide
>>>> -     better opportunities than a regular range, but I'm not sure.  */
>>>> -  if (vr->kind () == VR_ANTI_RANGE)
>>>> -    return;
>>>> -
>>>
>>> IIUC, you've switched to using the new API, so the bounds calls will 
>>> basically turn and ANTI range into a varying , making [lbound,ubound] 
>>> will be [MIN, MAX] ?
>>> so its effectively a no-op, except we will not punt on getting a 
>>> range when VR is an anti range anymore.. so that goodness...
>>
>> Yes.
>>
>>>
>>>
>>>> chrec = instantiate_parameters (loop, analyze_scalar_evolution 
>>>> (loop, var));
>>>>
>>>>    /* Like in PR19590, scev can return a constant function. */
>>>> @@ -1763,16 +1759,17 @@ vr_values::adjust_range_with_scev 
>>>> (value_range_equiv *vr, class loop *loop,
>>>>      }
>>>>
>>>>    if (TREE_CODE (chrec) != POLYNOMIAL_CHREC)
>>>> -    return;
>>>> +    {
>>>> +      vr->set_varying (TREE_TYPE (var));
>>>> +      return;
>>>> +    }
>>>
>>> Im seeing a lot of this pattern...
>>> Maybe we should set vr to varying upon entry to the function as the 
>>> default return value.. then we can just return like it did before in 
>>> all those places.
>>>
>>> Better yet, since this routine doesn't "update" anymore and simply 
>>> returns a range, maybe it could instead return a boolean if it finds 
>>> a range rather than the current behaviour...
>>> then those simply become
>>>
>>> +    return false;
>>>
>>> We won't have to intersect at the caller if we don't need to, and its 
>>> useful information at other points to know a range was calculated 
>>> without having to see if varying_p () came back from the call.
>>> ie, we'd the usage pattern would then be
>>>
>>> value_range_equiv r;
>>> if (range_of_var_in_loop (&r, this, loop, stmt, var))
>>>     vr->intersect (&r);
>>>
>>> This is the pattern we use throughout the ranger.
>>
>> Done.
>>
>>>
>>>
>>>>
>>>>    init = initial_condition_in_loop_num (chrec, loop->num);
>>>> -  tem = op_with_constant_singleton_value_range (init);
>>>> -  if (tem)
>>>> -    init = tem;
>>>> +  if (TREE_CODE (init) == SSA_NAME)
>>>> +    query->get_value_range (init, stmt)->singleton_p (&init);
>>>>    step = evolution_part_in_loop_num (chrec, loop->num);
>>>> -  tem = op_with_constant_singleton_value_range (step);
>>>> -  if (tem)
>>>> -    step = tem;
>>>> +  if (TREE_CODE (step) == SSA_NAME)
>>>> +    query->get_value_range (step, stmt)->singleton_p (&step);
>>>
>>> If I read this correctly, we get values for init and step... and if 
>>> they are SSA_NAMES, then we query ranges, otherwise use what we got 
>>> back.. So that would seem to be the same behaviour as before then..
>>> Perhaps a comment is warranted? I had to read it a few times :-)
>>
>> Indeed.  I am trying to do too much in one line.  I've added a comment.
>>
>>>
>>>
>>>>
>>>>    /* If STEP is symbolic, we can't know whether INIT will be the
>>>>       minimum or maximum value in the range.  Also, unless INIT is
>>>> @@ -1781,7 +1778,10 @@ vr_values::adjust_range_with_scev 
>>>> (value_range_equiv *vr, class loop *loop,
>>>>    if (step == NULL_TREE
>>>>        || !is_gimple_min_invariant (step)
>>>>        || !valid_value_p (init))
>>>> -    return;
>>>> +    {
>>>> +      vr->set_varying (TREE_TYPE (var));
>>>> +      return;
>>>> +    }
>>>>
>>>>    dir = scev_direction (chrec);
>>>>    if (/* Do not adjust ranges if we do not know whether the iv 
>>>> increases
>>>> @@ -1790,7 +1790,10 @@ vr_values::adjust_range_with_scev 
>>>> (value_range_equiv *vr, class loop *loop,
>>>>        /* ... or if it may wrap.  */
>>>>        || scev_probably_wraps_p (NULL_TREE, init, step, stmt,
>>>>                  get_chrec_loop (chrec), true))
>>>> -    return;
>>>> +    {
>>>> +      vr->set_varying (TREE_TYPE (var));
>>>> +      return;
>>>> +    }
>>>>
>>>>    type = TREE_TYPE (var);
>>>>    if (POINTER_TYPE_P (type) || !TYPE_MIN_VALUE (type))
>>>> @@ -1807,7 +1810,7 @@ vr_values::adjust_range_with_scev 
>>>> (value_range_equiv *vr, class loop *loop,
>>>>    if (TREE_CODE (step) == INTEGER_CST
>>>>        && is_gimple_val (init)
>>>>        && (TREE_CODE (init) != SSA_NAME
>>>> -      || get_value_range (init, stmt)->kind () == VR_RANGE))
>>>> +      || query->get_value_range (init, stmt)->kind () == VR_RANGE))
>>>>      {
>>>>        widest_int nit;
>>>>
>>>> @@ -1830,21 +1833,32 @@ vr_values::adjust_range_with_scev 
>>>> (value_range_equiv *vr, class loop *loop,
>>>>            && (sgn == UNSIGNED
>>>>            || wi::gts_p (wtmp, 0) == wi::gts_p (wi::to_wide (step), 
>>>> 0)))
>>>>          {
>>>> -          value_range_equiv maxvr;
>>>> -          tem = wide_int_to_tree (TREE_TYPE (init), wtmp);
>>>> -          extract_range_from_binary_expr (&maxvr, PLUS_EXPR,
>>>> -                          TREE_TYPE (init), init, tem);
>>>> +          value_range maxvr, vr0, vr1;
>>>> +          if (TREE_CODE (init) == SSA_NAME)
>>>> +        vr0 = *(query->get_value_range (init, stmt));
>>>> +          else if (is_gimple_min_invariant (init))
>>>> +        vr0.set (init);
>>>> +          else
>>>> +        vr0.set_varying (TREE_TYPE (init));
>>>> +          tree tem = wide_int_to_tree (TREE_TYPE (init), wtmp);
>>>> +          vr1.set (tem, tem);
>>>> +          range_fold_binary_expr (&maxvr, PLUS_EXPR,
>>>> +                      TREE_TYPE (init), &vr0, &vr1);
>>>> +
>>>>            /* Likewise if the addition did.  */
>>>>            if (maxvr.kind () == VR_RANGE)
>>>>          {
>>>>            value_range initvr;
>>>>
>>>>            if (TREE_CODE (init) == SSA_NAME)
>>>> -            initvr = *(get_value_range (init, stmt));
>>>> +            initvr = *(query->get_value_range (init, stmt));
>>>>            else if (is_gimple_min_invariant (init))
>>>>              initvr.set (init);
>>>>            else
>>>> -            return;
>>>> +            {
>>>> +              vr->set_varying (TREE_TYPE (var));
>>>> +              return;
>>>> +            }
>>>>
>>>>            /* Check if init + nit * step overflows.  Though we checked
>>>>               scev {init, step}_loop doesn't wrap, it is not enough
>>>> @@ -1854,7 +1868,10 @@ vr_values::adjust_range_with_scev 
>>>> (value_range_equiv *vr, class loop *loop,
>>>>                 && compare_values (maxvr.min (), initvr.min ()) != -1)
>>>>                || (dir == EV_DIR_GROWS
>>>>                && compare_values (maxvr.max (), initvr.max ()) != 1))
>>>> -            return;
>>>> +            {
>>>> +              vr->set_varying (TREE_TYPE (var));
>>>> +              return;
>>>> +            }
>>>>
>>>>            tmin = maxvr.min ();
>>>>            tmax = maxvr.max ();
>>>> @@ -1863,56 +1880,12 @@ vr_values::adjust_range_with_scev 
>>>> (value_range_equiv *vr, class loop *loop,
>>>>      }
>>>>      }
>>>>
>>>> -  if (vr->varying_p () || vr->undefined_p ())
>>>> -    {
>>>> -      min = tmin;
>>>> -      max = tmax;
>>>> -
>>>> -      /* For VARYING or UNDEFINED ranges, just about anything we get
>>>> -     from scalar evolutions should be better.  */
>>>> -
>>>> -      if (dir == EV_DIR_DECREASES)
>>>> -    max = init;
>>>> -      else
>>>> -    min = init;
>>>> -    }
>>>> -  else if (vr->kind () == VR_RANGE)
>>>> -    {
>>>> -      min = vr->min ();
>>>> -      max = vr->max ();
>>>> -
>>>> -      if (dir == EV_DIR_DECREASES)
>>>> -    {
>>>> -      /* INIT is the maximum value.  If INIT is lower than VR->MAX ()
>>>> -         but no smaller than VR->MIN (), set VR->MAX () to INIT.  */
>>>> -      if (compare_values (init, max) == -1)
>>>> -        max = init;
>>>> -
>>>> -      /* According to the loop information, the variable does not
>>>> -         overflow.  */
>>>> -      if (compare_values (min, tmin) == -1)
>>>> -        min = tmin;
>>>> -
>>>> -    }
>>>> -      else
>>>> -    {
>>>> -      /* If INIT is bigger than VR->MIN (), set VR->MIN () to 
>>>> INIT.  */
>>>> -      if (compare_values (init, min) == 1)
>>>> -        min = init;
>>>> -
>>>> -      if (compare_values (tmax, max) == -1)
>>>> -        max = tmax;
>>>> -    }
>>>> -    }
>>>> +  min = tmin;
>>>> +  max = tmax;
>>>> +  if (dir == EV_DIR_DECREASES)
>>>> +    max = init;
>>>>    else
>>>> -    return;
>>>> -
>>>> -  /* If we just created an invalid range with the minimum
>>>> -     greater than the maximum, we fail conservatively.
>>>> -     This should happen only in unreachable
>>>> -     parts of code, or for invalid programs.  */
>>>> -  if (compare_values (min, max) == 1)
>>>> -    return;
>>>> +    min = init;
>>>>
>>>>    /* Even for valid range info, sometimes overflow flag will leak in.
>>>>       As GIMPLE IL should have no constants with TREE_OVERFLOW set, we
>>>> @@ -1922,7 +1895,24 @@ vr_values::adjust_range_with_scev 
>>>> (value_range_equiv *vr, class loop *loop,
>>>>    if (TREE_OVERFLOW_P (max))
>>>>      max = drop_tree_overflow (max);
>>>>
>>>> -  vr->update (min, max);
>>>> +  gcc_checking_assert (compare_values (min, max) != 1);
>>>> +  if (TREE_CODE (min) == INTEGER_CST && TREE_CODE (max) == 
>>>> INTEGER_CST)
>>>> +    vr->set (min, max);
>>>> +  else
>>>> +    vr->set_varying (TREE_TYPE (var));
>>>> +}
>>>
>>> if min OR max is an integer (not both as in here),  shouldn't we be 
>>> setting the bounds we do know?
>>> ie  [min, +INF] or [0/-INF, max] ?
>>> or is that an behaviour change?
>>
>> It is definitely a behavior change.  However, I did try it just in 
>> case, but it caused a stage gengtype error, which I'm quite 
>> disinterested in chasing down.
>>
>> OK?
>>
>>
> hum. is it a behaviour change?  It might be for multiranges, but it 
> seems like min or max could be symbolics in legacy mode... and we'd lose 
> that information...
> 
> why can't we just    set (min, max) all the time like it did before?

Ah crap.  You're right.  Loop info may give us [SYM, 10] which must play 
well with a VR of say [5, 8].

I changed the interface to take in a *MIN and *MAX and set those in a 
function which is now called bounds_of_var_in_loop() since it's 
returning a pair of bounds, not a range.

I also moved the tweaking what was being done in said function into 
adjust_range_with_scev, which should now handle symbolics as we were 
doing before.

And as a bonus I tested all this by making sure that my new code gives 
the same result as the old code (see "FIXME: Temporary testing 
construct" in patch).  Bootstraps without failing the sanity check. 
Tests currently running.

For the ranger code, we can just normalize MIN/MAX into constants (or 
resolve any symbolics on-demand), and then just intersect.  For the 
record, any symbolics are guaranteed to be either SYM or SYM +- INT (per 
the valid_value_p() predicate in bounds_of_var_in_loop).  So they will 
be easy to parse.

I added a future enhancement comment for anti-ranges in the 
adjust_range_with_scev method if anyone is so inclined.  The previous 
code bailed on anti-ranges in preference of the VR passed down.  I have 
kept this functionality to avoid changing current behavior (and failing 
the sanity test).  And yes, we are steering away from anti-ranges and 
kind() as a whole, but adjust_range_with_scev() is meant to work with 
legacy ranges.

OK pending tests (provided I remove the double checking blocks :)).

Aldy

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

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index fe51a6faeb8..2538629b45d 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -1006,7 +1006,7 @@ vr_values::extract_range_from_comparison (value_range_equiv *vr,
    overflow.  */
 
 static bool
-check_for_binary_op_overflow (vr_values *store,
+check_for_binary_op_overflow (range_query *store,
 			      enum tree_code subcode, tree type,
 			      tree op0, tree op1, bool *ovf)
 {
@@ -1736,6 +1736,156 @@ compare_range_with_value (enum tree_code comp, const value_range *vr,
 
   gcc_unreachable ();
 }
+
+/* Given a VAR in STMT within LOOP, determine the bounds of the
+   variable and store it in MIN/MAX and return TRUE.  If no bounds
+   could be determined, return FALSE.  */
+
+bool
+bounds_of_var_in_loop (tree *min, tree *max, range_query *query,
+		       class loop *loop, gimple *stmt, tree var)
+{
+  tree init, step, chrec, tmin, tmax, type = TREE_TYPE (var);
+  enum ev_direction dir;
+
+  chrec = instantiate_parameters (loop, analyze_scalar_evolution (loop, var));
+
+  /* Like in PR19590, scev can return a constant function.  */
+  if (is_gimple_min_invariant (chrec))
+    {
+      *min = *max = chrec;
+      return true;
+    }
+
+  if (TREE_CODE (chrec) != POLYNOMIAL_CHREC)
+    return false;
+
+  init = initial_condition_in_loop_num (chrec, loop->num);
+  step = evolution_part_in_loop_num (chrec, loop->num);
+
+  /* If INIT is an SSA with a singleton range, set INIT to said
+     singleton, otherwise leave INIT alone.  */
+  if (TREE_CODE (init) == SSA_NAME)
+    query->get_value_range (init, stmt)->singleton_p (&init);
+  /* Likewise for step.  */
+  if (TREE_CODE (step) == SSA_NAME)
+    query->get_value_range (step, stmt)->singleton_p (&step);
+
+  /* If STEP is symbolic, we can't know whether INIT will be the
+     minimum or maximum value in the range.  Also, unless INIT is
+     a simple expression, compare_values and possibly other functions
+     in tree-vrp won't be able to handle it.  */
+  if (step == NULL_TREE
+      || !is_gimple_min_invariant (step)
+      || !valid_value_p (init))
+    return false;
+
+  dir = scev_direction (chrec);
+  if (/* Do not adjust ranges if we do not know whether the iv increases
+	 or decreases,  ... */
+      dir == EV_DIR_UNKNOWN
+      /* ... or if it may wrap.  */
+      || scev_probably_wraps_p (NULL_TREE, init, step, stmt,
+				get_chrec_loop (chrec), true))
+    return false;
+
+  if (POINTER_TYPE_P (type) || !TYPE_MIN_VALUE (type))
+    tmin = lower_bound_in_type (type, type);
+  else
+    tmin = TYPE_MIN_VALUE (type);
+  if (POINTER_TYPE_P (type) || !TYPE_MAX_VALUE (type))
+    tmax = upper_bound_in_type (type, type);
+  else
+    tmax = TYPE_MAX_VALUE (type);
+
+  /* Try to use estimated number of iterations for the loop to constrain the
+     final value in the evolution.  */
+  if (TREE_CODE (step) == INTEGER_CST
+      && is_gimple_val (init)
+      && (TREE_CODE (init) != SSA_NAME
+	  || query->get_value_range (init, stmt)->kind () == VR_RANGE))
+    {
+      widest_int nit;
+
+      /* We are only entering here for loop header PHI nodes, so using
+	 the number of latch executions is the correct thing to use.  */
+      if (max_loop_iterations (loop, &nit))
+	{
+	  signop sgn = TYPE_SIGN (TREE_TYPE (step));
+	  wi::overflow_type overflow;
+
+	  widest_int wtmp = wi::mul (wi::to_widest (step), nit, sgn,
+				     &overflow);
+	  /* If the multiplication overflowed we can't do a meaningful
+	     adjustment.  Likewise if the result doesn't fit in the type
+	     of the induction variable.  For a signed type we have to
+	     check whether the result has the expected signedness which
+	     is that of the step as number of iterations is unsigned.  */
+	  if (!overflow
+	      && wi::fits_to_tree_p (wtmp, TREE_TYPE (init))
+	      && (sgn == UNSIGNED
+		  || wi::gts_p (wtmp, 0) == wi::gts_p (wi::to_wide (step), 0)))
+	    {
+	      value_range maxvr, vr0, vr1;
+	      if (TREE_CODE (init) == SSA_NAME)
+		vr0 = *(query->get_value_range (init, stmt));
+	      else if (is_gimple_min_invariant (init))
+		vr0.set (init);
+	      else
+		vr0.set_varying (TREE_TYPE (init));
+	      tree tem = wide_int_to_tree (TREE_TYPE (init), wtmp);
+	      vr1.set (tem, tem);
+	      range_fold_binary_expr (&maxvr, PLUS_EXPR,
+				      TREE_TYPE (init), &vr0, &vr1);
+
+	      /* Likewise if the addition did.  */
+	      if (maxvr.kind () == VR_RANGE)
+		{
+		  value_range initvr;
+
+		  if (TREE_CODE (init) == SSA_NAME)
+		    initvr = *(query->get_value_range (init, stmt));
+		  else if (is_gimple_min_invariant (init))
+		    initvr.set (init);
+		  else
+		    return false;
+
+		  /* Check if init + nit * step overflows.  Though we checked
+		     scev {init, step}_loop doesn't wrap, it is not enough
+		     because the loop may exit immediately.  Overflow could
+		     happen in the plus expression in this case.  */
+		  if ((dir == EV_DIR_DECREASES
+		       && compare_values (maxvr.min (), initvr.min ()) != -1)
+		      || (dir == EV_DIR_GROWS
+			  && compare_values (maxvr.max (), initvr.max ()) != 1))
+		    return false;
+
+		  tmin = maxvr.min ();
+		  tmax = maxvr.max ();
+		}
+	    }
+	}
+    }
+
+  *min = tmin;
+  *max = tmax;
+  if (dir == EV_DIR_DECREASES)
+    *max = init;
+  else
+    *min = init;
+
+  /* Even for valid range info, sometimes overflow flag will leak in.
+     As GIMPLE IL should have no constants with TREE_OVERFLOW set, we
+     drop them.  */
+  if (TREE_OVERFLOW_P (*min))
+    *min = drop_tree_overflow (*min);
+  if (TREE_OVERFLOW_P (*max))
+    *max = drop_tree_overflow (*max);
+
+  gcc_checking_assert (compare_values (*min, *max) != 1);
+  return true;
+}
+
 /* Given a range VR, a LOOP and a variable VAR, determine whether it
    would be profitable to adjust VR using scalar evolution information
    for VAR.  If so, update VR with the new limits.  */
@@ -1743,6 +1893,57 @@ compare_range_with_value (enum tree_code comp, const value_range *vr,
 void
 vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
 				   gimple *stmt, tree var)
+{
+  // FIXME: Temporary testing construct.
+  value_range_equiv old;
+  old.deep_copy (vr);
+
+  tree min, max;
+  if (bounds_of_var_in_loop (&min, &max, this, loop, stmt, var))
+    {
+      if (vr->undefined_p () || vr->varying_p ())
+	{
+	  /* For VARYING or UNDEFINED ranges, just about anything we get
+	     from scalar evolutions should be better.  */
+	  vr->update (min, max);
+	}
+      else if (vr->kind () == VR_RANGE)
+	{
+	  /* Start with the input range... */
+	  tree vrmin = vr->min ();
+	  tree vrmax = vr->max ();
+
+	  /* ...and narrow it down with what we got from SCEV.  */
+	  if (compare_values (min, vrmin) == 1)
+	    vrmin = min;
+	  if (compare_values (max, vrmax) == -1)
+	    vrmax = max;
+
+	  vr->update (vrmin, vrmax);
+	}
+      else if (vr->kind () == VR_ANTI_RANGE)
+	{
+	  /* ?? As an enhancement, if VR, MIN, and MAX are constants, one
+	     could just intersect VR with a range of [MIN,MAX].  */
+	}
+    }
+
+  // FIXME: Temporary testing construct.
+  old_adjust_range_with_scev (&old, loop, stmt, var);
+  if (!vr->equal_p (old, false))
+    {
+      fprintf (stderr, "old range: ");
+      old.dump ();
+      fprintf (stderr, "\nnew range: ");
+      vr->dump ();
+      gcc_unreachable ();
+      fprintf (stderr, "\n");
+    }
+}
+
+void
+vr_values::old_adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
+				   gimple *stmt, tree var)
 {
   tree init, step, chrec, tmin, tmax, min, max, type, tem;
   enum ev_direction dir;
@@ -1806,7 +2007,7 @@ vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
   if (TREE_CODE (step) == INTEGER_CST
       && is_gimple_val (init)
       && (TREE_CODE (init) != SSA_NAME
-	  || get_value_range (init, stmt)->kind () == VR_RANGE))
+	  || get_value_range (init)->kind () == VR_RANGE))
     {
       widest_int nit;
 
@@ -1839,7 +2040,7 @@ vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
 		  value_range initvr;
 
 		  if (TREE_CODE (init) == SSA_NAME)
-		    initvr = *(get_value_range (init, stmt));
+		    initvr = *(get_value_range (init));
 		  else if (is_gimple_min_invariant (init))
 		    initvr.set (init);
 		  else
@@ -1924,6 +2125,7 @@ vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
   vr->update (min, max);
 }
 
+
 /* Dump value ranges of all SSA_NAMEs to FILE.  */
 
 void
@@ -4216,7 +4418,7 @@ simplify_using_ranges::two_valued_val_range_p (tree var, tree *a, tree *b)
   return false;
 }
 
-simplify_using_ranges::simplify_using_ranges (vr_values *store)
+simplify_using_ranges::simplify_using_ranges (range_query *store)
   : store (store)
 {
   to_remove_edges = vNULL;
diff --git a/gcc/vr-values.h b/gcc/vr-values.h
index 330b4605e39..1a14df2357b 100644
--- a/gcc/vr-values.h
+++ b/gcc/vr-values.h
@@ -22,16 +22,26 @@ along with GCC; see the file COPYING3.  If not see
 
 #include "value-range-equiv.h"
 
+// Abstract class to return a range for a given SSA.
+
+class range_query
+{
+public:
+  virtual const value_range_equiv *get_value_range (const_tree,
+						    gimple * = NULL) = 0;
+  virtual ~range_query () { }
+};
+
 // Class to simplify a statement using range information.
 //
 // The constructor takes a full vr_values, but all it needs is
 // get_value_range() from it.  This class could be made to work with
 // any range repository.
 
-class simplify_using_ranges
+class simplify_using_ranges : public range_query
 {
 public:
-  simplify_using_ranges (class vr_values *);
+  simplify_using_ranges (class range_query *);
   ~simplify_using_ranges ();
   bool simplify (gimple_stmt_iterator *);
 
@@ -44,7 +54,7 @@ public:
 
 private:
   const value_range_equiv *get_value_range (const_tree op,
-					    gimple *stmt = NULL);
+					    gimple *stmt = NULL) OVERRIDE;
   bool simplify_truth_ops_using_ranges (gimple_stmt_iterator *, gimple *);
   bool simplify_div_or_mod_using_ranges (gimple_stmt_iterator *, gimple *);
   bool simplify_abs_using_ranges (gimple_stmt_iterator *, gimple *);
@@ -79,7 +89,7 @@ private:
 
   vec<edge> to_remove_edges;
   vec<switch_update> to_update_switch_stmts;
-  class vr_values *store;
+  class range_query *store;
 };
 
 /* The VR_VALUES class holds the current view of range information
@@ -96,7 +106,7 @@ private:
    gets attached to an SSA_NAME.  It's unclear how useful that global
    information will be in a world where we can compute context sensitive
    range information fast or perform on-demand queries.  */
-class vr_values
+class vr_values : public range_query
 {
  public:
   vr_values (void);
@@ -112,6 +122,8 @@ class vr_values
   tree op_with_constant_singleton_value_range (tree);
   void adjust_range_with_scev (value_range_equiv *, class loop *,
 			       gimple *, tree);
+  // FIXME: Temporary testing construct.
+  void old_adjust_range_with_scev (value_range_equiv *, class loop *, gimple *, tree);
   void dump_all_value_ranges (FILE *);
 
   void extract_range_for_var_from_comparison_expr (tree, enum tree_code,
@@ -177,4 +189,7 @@ extern tree get_output_for_vrp (gimple *);
 // FIXME: Move this to tree-vrp.c.
 void simplify_cond_using_ranges_2 (class vr_values *, gcond *);
 
+extern bool bounds_of_var_in_loop (tree *min, tree *max, range_query *,
+				   class loop *loop, gimple *stmt, tree var);
+
 #endif /* GCC_VR_VALUES_H */