[PATCH 0/2] decouple adjust_range_from_scev from vr_values

[PATCH 0/2] decouple adjust_range_from_scev from vr_values

[Aldy Hernandez]

The goal here is to disassociate adjust_range_from_scev from vr_values,
and value_range_equiv while we're at it.

We've already done something similar with simplify_using_ranges, where we
take in a "store" which is a class providing get_value_range().  Initially
we set it up to take a vr_values, but the ultimate purpose was to have
it work with either *vrp or the ranger.  As such, I have abstracted
out the get_value_range() method into its own abstract class from
which vr_values inherits, and provides said method.

As I did for the substitute_and_fold_engine, I future proofed
get_value_range so it takes a gimple statement.  This provides context for
the SSA being queried.  I purposely did not provide a default statement
of NULL, as I want each caller to pass a statement if available.

This patchset is divided in two: one patch to provide the additional
argument to get_value_range and one to do the ripping apart in
adjust_range_from_scev.  I will discuss the SCEV part in the relevant
patch.

Aldy

[PATCH 1/2] Add statement context to get_value_range.

[Aldy Hernandez]

This is in line with the statement context that we have for get_value()
in the substitute_and_fold_engine class.
---
 gcc/vr-values.c | 64 ++++++++++++++++++++++++++-----------------------
 gcc/vr-values.h | 14 +++++------
 2 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 511342f2f13..9002d87c14b 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -147,7 +147,8 @@ vr_values::get_lattice_entry (const_tree var)
    return NULL.  Otherwise create an empty range if none existed for VAR.  */
 
 const value_range_equiv *
-vr_values::get_value_range (const_tree var)
+vr_values::get_value_range (const_tree var,
+			    gimple *stmt ATTRIBUTE_UNUSED)
 {
   /* If we have no recorded ranges, then return NULL.  */
   if (!vr_value)
@@ -450,7 +451,7 @@ simplify_using_ranges::op_with_boolean_value_range_p (tree op)
 
   /* ?? Errr, this should probably check for [0,0] and [1,1] as well
      as [0,1].  */
-  const value_range *vr = get_value_range (op);
+  const value_range *vr = get_value_range (op, NULL);
   return *vr == value_range (build_zero_cst (TREE_TYPE (op)),
 			     build_one_cst (TREE_TYPE (op)));
 }
@@ -972,12 +973,13 @@ vr_values::extract_range_from_cond_expr (value_range_equiv *vr, gassign *stmt)
 
 void
 vr_values::extract_range_from_comparison (value_range_equiv *vr,
+					  gimple *stmt,
 					  enum tree_code code,
 					  tree type, tree op0, tree op1)
 {
   bool sop;
   tree val
-    = simplifier.vrp_evaluate_conditional_warnv_with_ops (code, op0, op1,
+    = simplifier.vrp_evaluate_conditional_warnv_with_ops (stmt, code, op0, op1,
 							  false, &sop, NULL);
   if (val)
     {
@@ -1008,14 +1010,14 @@ check_for_binary_op_overflow (vr_values *store,
 {
   value_range vr0, vr1;
   if (TREE_CODE (op0) == SSA_NAME)
-    vr0 = *store->get_value_range (op0);
+    vr0 = *store->get_value_range (op0, NULL);
   else if (TREE_CODE (op0) == INTEGER_CST)
     vr0.set (op0);
   else
     vr0.set_varying (TREE_TYPE (op0));
 
   if (TREE_CODE (op1) == SSA_NAME)
-    vr1 = *store->get_value_range (op1);
+    vr1 = *store->get_value_range (op1, NULL);
   else if (TREE_CODE (op1) == INTEGER_CST)
     vr1.set (op1);
   else
@@ -1472,7 +1474,7 @@ vr_values::extract_range_from_assignment (value_range_equiv *vr, gassign *stmt)
   else if (code == COND_EXPR)
     extract_range_from_cond_expr (vr, stmt);
   else if (TREE_CODE_CLASS (code) == tcc_comparison)
-    extract_range_from_comparison (vr, gimple_assign_rhs_code (stmt),
+    extract_range_from_comparison (vr, stmt, gimple_assign_rhs_code (stmt),
 				   gimple_expr_type (stmt),
 				   gimple_assign_rhs1 (stmt),
 				   gimple_assign_rhs2 (stmt));
@@ -1805,7 +1807,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)->kind () == VR_RANGE))
+	  || get_value_range (init, stmt)->kind () == VR_RANGE))
     {
       widest_int nit;
 
@@ -1838,7 +1840,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));
+		    initvr = *(get_value_range (init, stmt));
 		  else if (is_gimple_min_invariant (init))
 		    initvr.set (init);
 		  else
@@ -2090,7 +2092,7 @@ const value_range_equiv *
 simplify_using_ranges::get_vr_for_comparison (int i, value_range_equiv *tem)
 {
   /* Shallow-copy equiv bitmap.  */
-  const value_range_equiv *vr = get_value_range (ssa_name (i));
+  const value_range_equiv *vr = get_value_range (ssa_name (i), NULL);
 
   /* If name N_i does not have a valid range, use N_i as its own
      range.  This allows us to compare against names that may
@@ -2115,7 +2117,7 @@ simplify_using_ranges::compare_name_with_value
 				 bool *strict_overflow_p, bool use_equiv_p)
 {
   /* Get the set of equivalences for VAR.  */
-  bitmap e = get_value_range (var)->equiv ();
+  bitmap e = get_value_range (var, NULL)->equiv ();
 
   /* Start at -1.  Set it to 0 if we do a comparison without relying
      on overflow, or 1 if all comparisons rely on overflow.  */
@@ -2195,8 +2197,8 @@ simplify_using_ranges::compare_names (enum tree_code comp, tree n1, tree n2,
 {
   /* Compare the ranges of every name equivalent to N1 against the
      ranges of every name equivalent to N2.  */
-  bitmap e1 = get_value_range (n1)->equiv ();
-  bitmap e2 = get_value_range (n2)->equiv ();
+  bitmap e1 = get_value_range (n1, NULL)->equiv ();
+  bitmap e2 = get_value_range (n2, NULL)->equiv ();
 
   /* Use the fake bitmaps if e1 or e2 are not available.  */
   static bitmap s_e1 = NULL, s_e2 = NULL;
@@ -2308,8 +2310,8 @@ simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops_using_ranges
     (enum tree_code code, tree op0, tree op1, bool * strict_overflow_p)
 {
   const value_range_equiv *vr0, *vr1;
-  vr0 = (TREE_CODE (op0) == SSA_NAME) ? get_value_range (op0) : NULL;
-  vr1 = (TREE_CODE (op1) == SSA_NAME) ? get_value_range (op1) : NULL;
+  vr0 = (TREE_CODE (op0) == SSA_NAME) ? get_value_range (op0, NULL) : NULL;
+  vr1 = (TREE_CODE (op1) == SSA_NAME) ? get_value_range (op1, NULL) : NULL;
 
   tree res = NULL_TREE;
   if (vr0 && vr1)
@@ -2326,7 +2328,8 @@ simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops_using_ranges
 
 tree
 simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops
-						(enum tree_code code,
+						(gimple *stmt,
+						 enum tree_code code,
 						 tree op0, tree op1,
 						 bool use_equiv_p,
 						 bool *strict_overflow_p,
@@ -2387,7 +2390,7 @@ simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops
 	    }
 	  else
 	    gcc_unreachable ();
-	  const value_range_equiv *vr0 = get_value_range (op0);
+	  const value_range_equiv *vr0 = get_value_range (op0, stmt);
 	  /* If vro, the range for OP0 to pass the overflow test, has
 	     no intersection with *vr0, OP0's known range, then the
 	     overflow test can't pass, so return the node for false.
@@ -2449,8 +2452,8 @@ simplify_using_ranges::vrp_evaluate_conditional (tree_code code, tree op0,
     return NULL_TREE;
 
   sop = false;
-  ret = vrp_evaluate_conditional_warnv_with_ops (code, op0, op1, true, &sop,
-  						 &only_ranges);
+  ret = vrp_evaluate_conditional_warnv_with_ops (stmt, code, op0, op1, true,
+						 &sop, &only_ranges);
 
   if (ret && sop)
     {
@@ -2493,7 +2496,7 @@ simplify_using_ranges::vrp_evaluate_conditional (tree_code code, tree op0,
 	 always fold regardless of the value of OP0.  If -Wtype-limits
 	 was specified, emit a warning.  */
       tree type = TREE_TYPE (op0);
-      const value_range_equiv *vr0 = get_value_range (op0);
+      const value_range_equiv *vr0 = get_value_range (op0, stmt);
 
       if (vr0->varying_p ()
 	  && INTEGRAL_TYPE_P (type)
@@ -2544,7 +2547,7 @@ simplify_using_ranges::vrp_visit_cond_stmt (gcond *stmt, edge *taken_edge_p)
 	  fprintf (dump_file, "\t");
 	  print_generic_expr (dump_file, use);
 	  fprintf (dump_file, ": ");
-	  dump_value_range (dump_file, get_value_range (use));
+	  dump_value_range (dump_file, get_value_range (use, stmt));
 	}
 
       fprintf (dump_file, "\n");
@@ -2594,7 +2597,8 @@ simplify_using_ranges::vrp_visit_cond_stmt (gcond *stmt, edge *taken_edge_p)
      4 more predicates folded in SPEC.  */
 
   bool sop;
-  val = vrp_evaluate_conditional_warnv_with_ops (gimple_cond_code (stmt),
+  val = vrp_evaluate_conditional_warnv_with_ops (stmt,
+						 gimple_cond_code (stmt),
 						 gimple_cond_lhs (stmt),
 						 gimple_cond_rhs (stmt),
 						 false, &sop, NULL);
@@ -3119,7 +3123,7 @@ simplify_using_ranges::simplify_div_or_mod_using_ranges
     }
   else
     {
-      vr = get_value_range (op0);
+      vr = get_value_range (op0, stmt);
       if (range_int_cst_p (vr))
 	{
 	  op0min = vr->min ();
@@ -3130,7 +3134,7 @@ simplify_using_ranges::simplify_div_or_mod_using_ranges
   if (rhs_code == TRUNC_MOD_EXPR
       && TREE_CODE (op1) == SSA_NAME)
     {
-      const value_range_equiv *vr1 = get_value_range (op1);
+      const value_range_equiv *vr1 = get_value_range (op1, stmt);
       if (range_int_cst_p (vr1))
 	op1min = vr1->min ();
     }
@@ -3279,7 +3283,7 @@ simplify_using_ranges::simplify_abs_using_ranges (gimple_stmt_iterator *gsi,
 						  gimple *stmt)
 {
   tree op = gimple_assign_rhs1 (stmt);
-  const value_range *vr = get_value_range (op);
+  const value_range *vr = get_value_range (op, stmt);
 
   if (vr)
     {
@@ -3369,14 +3373,14 @@ simplify_using_ranges::simplify_bit_ops_using_ranges
   wide_int mask;
 
   if (TREE_CODE (op0) == SSA_NAME)
-    vr0 = *(get_value_range (op0));
+    vr0 = *(get_value_range (op0, stmt));
   else if (is_gimple_min_invariant (op0))
     vr0.set (op0);
   else
     return false;
 
   if (TREE_CODE (op1) == SSA_NAME)
-    vr1 = *(get_value_range (op1));
+    vr1 = *(get_value_range (op1, stmt));
   else if (is_gimple_min_invariant (op1))
     vr1.set (op1);
   else
@@ -3595,7 +3599,7 @@ simplify_using_ranges::simplify_cond_using_ranges_1 (gcond *stmt)
       && INTEGRAL_TYPE_P (TREE_TYPE (op0))
       && is_gimple_min_invariant (op1))
     {
-      const value_range *vr = get_value_range (op0);
+      const value_range *vr = get_value_range (op0, stmt);
 
       /* If we have range information for OP0, then we might be
 	 able to simplify this conditional. */
@@ -3739,7 +3743,7 @@ simplify_using_ranges::simplify_switch_using_ranges (gswitch *stmt)
 
   if (TREE_CODE (op) == SSA_NAME)
     {
-      vr = get_value_range (op);
+      vr = get_value_range (op, stmt);
 
       /* We can only handle integer ranges.  */
       if (vr->varying_p ()
@@ -4032,7 +4036,7 @@ simplify_using_ranges::simplify_float_conversion_using_ranges
 					 gimple *stmt)
 {
   tree rhs1 = gimple_assign_rhs1 (stmt);
-  const value_range *vr = get_value_range (rhs1);
+  const value_range *vr = get_value_range (rhs1, stmt);
   scalar_float_mode fltmode
     = SCALAR_FLOAT_TYPE_MODE (TREE_TYPE (gimple_assign_lhs (stmt)));
   scalar_int_mode mode;
@@ -4196,7 +4200,7 @@ simplify_using_ranges::simplify_internal_call_using_ranges
 bool
 simplify_using_ranges::two_valued_val_range_p (tree var, tree *a, tree *b)
 {
-  value_range vr = *get_value_range (var);
+  value_range vr = *get_value_range (var, NULL);
   vr.normalize_symbolics ();
   if (vr.varying_p () || vr.undefined_p ())
     return false;
diff --git a/gcc/vr-values.h b/gcc/vr-values.h
index 62a20218c6d..3fbea9bd69f 100644
--- a/gcc/vr-values.h
+++ b/gcc/vr-values.h
@@ -38,12 +38,12 @@ public:
   // ?? These should be cleaned, merged, and made private.
   tree vrp_evaluate_conditional (tree_code, tree, tree, gimple *);
   void vrp_visit_cond_stmt (gcond *, edge *);
-  tree vrp_evaluate_conditional_warnv_with_ops (enum tree_code,
+  tree vrp_evaluate_conditional_warnv_with_ops (gimple *stmt, enum tree_code,
 						tree, tree, bool,
 						bool *, bool *);
 
 private:
-  const value_range_equiv *get_value_range (const_tree op);
+  const value_range_equiv *get_value_range (const_tree op, gimple *stmt);
   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 *);
@@ -101,7 +101,7 @@ class vr_values
   vr_values (void);
   ~vr_values (void);
 
-  const value_range_equiv *get_value_range (const_tree);
+  const value_range_equiv *get_value_range (const_tree, gimple * = NULL);
   void set_vr_value (tree, value_range_equiv *);
   value_range_equiv *swap_vr_value (tree, value_range_equiv *);
 
@@ -140,8 +140,8 @@ class vr_values
   void extract_range_from_unary_expr (value_range_equiv *, enum tree_code,
 				      tree, tree);
   void extract_range_from_cond_expr (value_range_equiv *, gassign *);
-  void extract_range_from_comparison (value_range_equiv *, enum tree_code,
-				      tree, tree, tree);
+  void extract_range_from_comparison (value_range_equiv *, gimple *,
+				      enum tree_code, tree, tree, tree);
   void vrp_visit_assignment_or_call (gimple*, tree *, value_range_equiv *);
   void vrp_visit_switch_stmt (gswitch *, edge *);
 
@@ -167,9 +167,9 @@ class vr_values
 };
 
 inline const value_range_equiv *
-simplify_using_ranges::get_value_range (const_tree op)
+simplify_using_ranges::get_value_range (const_tree op, gimple *stmt)
 {
-  return store->get_value_range (op);
+  return store->get_value_range (op, stmt);
 }
 
 extern tree get_output_for_vrp (gimple *);
-- 
2.26.2

PING: Fwd: [PATCH 1/2] Add statement context to get_value_range.

[Aldy Hernandez]

---------- Forwarded message ---------
From: Aldy Hernandez <aldyh@redhat.com>
Date: Tue, Aug 4, 2020, 13:55
Subject: [PATCH 1/2] Add statement context to get_value_range.
To: <gcc-patches@gcc.gnu.org>
Cc: <law@redhat.com>, Aldy Hernandez <aldyh@redhat.com>


This is in line with the statement context that we have for get_value()
in the substitute_and_fold_engine class.
---
 gcc/vr-values.c | 64 ++++++++++++++++++++++++++-----------------------
 gcc/vr-values.h | 14 +++++------
 2 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 511342f2f13..9002d87c14b 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -147,7 +147,8 @@ vr_values::get_lattice_entry (const_tree var)
    return NULL.  Otherwise create an empty range if none existed for VAR.
*/

 const value_range_equiv *
-vr_values::get_value_range (const_tree var)
+vr_values::get_value_range (const_tree var,
+                           gimple *stmt ATTRIBUTE_UNUSED)
 {
   /* If we have no recorded ranges, then return NULL.  */
   if (!vr_value)
@@ -450,7 +451,7 @@ simplify_using_ranges::op_with_boolean_value_range_p
(tree op)

   /* ?? Errr, this should probably check for [0,0] and [1,1] as well
      as [0,1].  */
-  const value_range *vr = get_value_range (op);
+  const value_range *vr = get_value_range (op, NULL);
   return *vr == value_range (build_zero_cst (TREE_TYPE (op)),
                             build_one_cst (TREE_TYPE (op)));
 }
@@ -972,12 +973,13 @@ vr_values::extract_range_from_cond_expr
(value_range_equiv *vr, gassign *stmt)

 void
 vr_values::extract_range_from_comparison (value_range_equiv *vr,
+                                         gimple *stmt,
                                          enum tree_code code,
                                          tree type, tree op0, tree op1)
 {
   bool sop;
   tree val
-    = simplifier.vrp_evaluate_conditional_warnv_with_ops (code, op0, op1,
+    = simplifier.vrp_evaluate_conditional_warnv_with_ops (stmt, code, op0,
op1,
                                                          false, &sop,
NULL);
   if (val)
     {
@@ -1008,14 +1010,14 @@ check_for_binary_op_overflow (vr_values *store,
 {
   value_range vr0, vr1;
   if (TREE_CODE (op0) == SSA_NAME)
-    vr0 = *store->get_value_range (op0);
+    vr0 = *store->get_value_range (op0, NULL);
   else if (TREE_CODE (op0) == INTEGER_CST)
     vr0.set (op0);
   else
     vr0.set_varying (TREE_TYPE (op0));

   if (TREE_CODE (op1) == SSA_NAME)
-    vr1 = *store->get_value_range (op1);
+    vr1 = *store->get_value_range (op1, NULL);
   else if (TREE_CODE (op1) == INTEGER_CST)
     vr1.set (op1);
   else
@@ -1472,7 +1474,7 @@ vr_values::extract_range_from_assignment
(value_range_equiv *vr, gassign *stmt)
   else if (code == COND_EXPR)
     extract_range_from_cond_expr (vr, stmt);
   else if (TREE_CODE_CLASS (code) == tcc_comparison)
-    extract_range_from_comparison (vr, gimple_assign_rhs_code (stmt),
+    extract_range_from_comparison (vr, stmt, gimple_assign_rhs_code (stmt),
                                   gimple_expr_type (stmt),
                                   gimple_assign_rhs1 (stmt),
                                   gimple_assign_rhs2 (stmt));
@@ -1805,7 +1807,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)->kind () == VR_RANGE))
+         || get_value_range (init, stmt)->kind () == VR_RANGE))
     {
       widest_int nit;

@@ -1838,7 +1840,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));
+                   initvr = *(get_value_range (init, stmt));
                  else if (is_gimple_min_invariant (init))
                    initvr.set (init);
                  else
@@ -2090,7 +2092,7 @@ const value_range_equiv *
 simplify_using_ranges::get_vr_for_comparison (int i, value_range_equiv
*tem)
 {
   /* Shallow-copy equiv bitmap.  */
-  const value_range_equiv *vr = get_value_range (ssa_name (i));
+  const value_range_equiv *vr = get_value_range (ssa_name (i), NULL);

   /* If name N_i does not have a valid range, use N_i as its own
      range.  This allows us to compare against names that may
@@ -2115,7 +2117,7 @@ simplify_using_ranges::compare_name_with_value
                                 bool *strict_overflow_p, bool use_equiv_p)
 {
   /* Get the set of equivalences for VAR.  */
-  bitmap e = get_value_range (var)->equiv ();
+  bitmap e = get_value_range (var, NULL)->equiv ();

   /* Start at -1.  Set it to 0 if we do a comparison without relying
      on overflow, or 1 if all comparisons rely on overflow.  */
@@ -2195,8 +2197,8 @@ simplify_using_ranges::compare_names (enum tree_code
comp, tree n1, tree n2,
 {
   /* Compare the ranges of every name equivalent to N1 against the
      ranges of every name equivalent to N2.  */
-  bitmap e1 = get_value_range (n1)->equiv ();
-  bitmap e2 = get_value_range (n2)->equiv ();
+  bitmap e1 = get_value_range (n1, NULL)->equiv ();
+  bitmap e2 = get_value_range (n2, NULL)->equiv ();

   /* Use the fake bitmaps if e1 or e2 are not available.  */
   static bitmap s_e1 = NULL, s_e2 = NULL;
@@ -2308,8 +2310,8 @@
simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops_using_ranges
     (enum tree_code code, tree op0, tree op1, bool * strict_overflow_p)
 {
   const value_range_equiv *vr0, *vr1;
-  vr0 = (TREE_CODE (op0) == SSA_NAME) ? get_value_range (op0) : NULL;
-  vr1 = (TREE_CODE (op1) == SSA_NAME) ? get_value_range (op1) : NULL;
+  vr0 = (TREE_CODE (op0) == SSA_NAME) ? get_value_range (op0, NULL) : NULL;
+  vr1 = (TREE_CODE (op1) == SSA_NAME) ? get_value_range (op1, NULL) : NULL;

   tree res = NULL_TREE;
   if (vr0 && vr1)
@@ -2326,7 +2328,8 @@
simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops_using_ranges

 tree
 simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops
-                                               (enum tree_code code,
+                                               (gimple *stmt,
+                                                enum tree_code code,
                                                 tree op0, tree op1,
                                                 bool use_equiv_p,
                                                 bool *strict_overflow_p,
@@ -2387,7 +2390,7 @@
simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops
            }
          else
            gcc_unreachable ();
-         const value_range_equiv *vr0 = get_value_range (op0);
+         const value_range_equiv *vr0 = get_value_range (op0, stmt);
          /* If vro, the range for OP0 to pass the overflow test, has
             no intersection with *vr0, OP0's known range, then the
             overflow test can't pass, so return the node for false.
@@ -2449,8 +2452,8 @@ simplify_using_ranges::vrp_evaluate_conditional
(tree_code code, tree op0,
     return NULL_TREE;

   sop = false;
-  ret = vrp_evaluate_conditional_warnv_with_ops (code, op0, op1, true,
&sop,
-                                                &only_ranges);
+  ret = vrp_evaluate_conditional_warnv_with_ops (stmt, code, op0, op1,
true,
+                                                &sop, &only_ranges);

   if (ret && sop)
     {
@@ -2493,7 +2496,7 @@ simplify_using_ranges::vrp_evaluate_conditional
(tree_code code, tree op0,
         always fold regardless of the value of OP0.  If -Wtype-limits
         was specified, emit a warning.  */
       tree type = TREE_TYPE (op0);
-      const value_range_equiv *vr0 = get_value_range (op0);
+      const value_range_equiv *vr0 = get_value_range (op0, stmt);

       if (vr0->varying_p ()
          && INTEGRAL_TYPE_P (type)
@@ -2544,7 +2547,7 @@ simplify_using_ranges::vrp_visit_cond_stmt (gcond
*stmt, edge *taken_edge_p)
          fprintf (dump_file, "\t");
          print_generic_expr (dump_file, use);
          fprintf (dump_file, ": ");
-         dump_value_range (dump_file, get_value_range (use));
+         dump_value_range (dump_file, get_value_range (use, stmt));
        }

       fprintf (dump_file, "\n");
@@ -2594,7 +2597,8 @@ simplify_using_ranges::vrp_visit_cond_stmt (gcond
*stmt, edge *taken_edge_p)
      4 more predicates folded in SPEC.  */

   bool sop;
-  val = vrp_evaluate_conditional_warnv_with_ops (gimple_cond_code (stmt),
+  val = vrp_evaluate_conditional_warnv_with_ops (stmt,
+                                                gimple_cond_code (stmt),
                                                 gimple_cond_lhs (stmt),
                                                 gimple_cond_rhs (stmt),
                                                 false, &sop, NULL);
@@ -3119,7 +3123,7 @@
simplify_using_ranges::simplify_div_or_mod_using_ranges
     }
   else
     {
-      vr = get_value_range (op0);
+      vr = get_value_range (op0, stmt);
       if (range_int_cst_p (vr))
        {
          op0min = vr->min ();
@@ -3130,7 +3134,7 @@
simplify_using_ranges::simplify_div_or_mod_using_ranges
   if (rhs_code == TRUNC_MOD_EXPR
       && TREE_CODE (op1) == SSA_NAME)
     {
-      const value_range_equiv *vr1 = get_value_range (op1);
+      const value_range_equiv *vr1 = get_value_range (op1, stmt);
       if (range_int_cst_p (vr1))
        op1min = vr1->min ();
     }
@@ -3279,7 +3283,7 @@ simplify_using_ranges::simplify_abs_using_ranges
(gimple_stmt_iterator *gsi,
                                                  gimple *stmt)
 {
   tree op = gimple_assign_rhs1 (stmt);
-  const value_range *vr = get_value_range (op);
+  const value_range *vr = get_value_range (op, stmt);

   if (vr)
     {
@@ -3369,14 +3373,14 @@ simplify_using_ranges::simplify_bit_ops_using_ranges
   wide_int mask;

   if (TREE_CODE (op0) == SSA_NAME)
-    vr0 = *(get_value_range (op0));
+    vr0 = *(get_value_range (op0, stmt));
   else if (is_gimple_min_invariant (op0))
     vr0.set (op0);
   else
     return false;

   if (TREE_CODE (op1) == SSA_NAME)
-    vr1 = *(get_value_range (op1));
+    vr1 = *(get_value_range (op1, stmt));
   else if (is_gimple_min_invariant (op1))
     vr1.set (op1);
   else
@@ -3595,7 +3599,7 @@ simplify_using_ranges::simplify_cond_using_ranges_1
(gcond *stmt)
       && INTEGRAL_TYPE_P (TREE_TYPE (op0))
       && is_gimple_min_invariant (op1))
     {
-      const value_range *vr = get_value_range (op0);
+      const value_range *vr = get_value_range (op0, stmt);

       /* If we have range information for OP0, then we might be
         able to simplify this conditional. */
@@ -3739,7 +3743,7 @@ simplify_using_ranges::simplify_switch_using_ranges
(gswitch *stmt)

   if (TREE_CODE (op) == SSA_NAME)
     {
-      vr = get_value_range (op);
+      vr = get_value_range (op, stmt);

       /* We can only handle integer ranges.  */
       if (vr->varying_p ()
@@ -4032,7 +4036,7 @@
simplify_using_ranges::simplify_float_conversion_using_ranges
                                         gimple *stmt)
 {
   tree rhs1 = gimple_assign_rhs1 (stmt);
-  const value_range *vr = get_value_range (rhs1);
+  const value_range *vr = get_value_range (rhs1, stmt);
   scalar_float_mode fltmode
     = SCALAR_FLOAT_TYPE_MODE (TREE_TYPE (gimple_assign_lhs (stmt)));
   scalar_int_mode mode;
@@ -4196,7 +4200,7 @@
simplify_using_ranges::simplify_internal_call_using_ranges
 bool
 simplify_using_ranges::two_valued_val_range_p (tree var, tree *a, tree *b)
 {
-  value_range vr = *get_value_range (var);
+  value_range vr = *get_value_range (var, NULL);
   vr.normalize_symbolics ();
   if (vr.varying_p () || vr.undefined_p ())
     return false;
diff --git a/gcc/vr-values.h b/gcc/vr-values.h
index 62a20218c6d..3fbea9bd69f 100644
--- a/gcc/vr-values.h
+++ b/gcc/vr-values.h
@@ -38,12 +38,12 @@ public:
   // ?? These should be cleaned, merged, and made private.
   tree vrp_evaluate_conditional (tree_code, tree, tree, gimple *);
   void vrp_visit_cond_stmt (gcond *, edge *);
-  tree vrp_evaluate_conditional_warnv_with_ops (enum tree_code,
+  tree vrp_evaluate_conditional_warnv_with_ops (gimple *stmt, enum
tree_code,
                                                tree, tree, bool,
                                                bool *, bool *);

 private:
-  const value_range_equiv *get_value_range (const_tree op);
+  const value_range_equiv *get_value_range (const_tree op, gimple *stmt);
   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 *);
@@ -101,7 +101,7 @@ class vr_values
   vr_values (void);
   ~vr_values (void);

-  const value_range_equiv *get_value_range (const_tree);
+  const value_range_equiv *get_value_range (const_tree, gimple * = NULL);
   void set_vr_value (tree, value_range_equiv *);
   value_range_equiv *swap_vr_value (tree, value_range_equiv *);

@@ -140,8 +140,8 @@ class vr_values
   void extract_range_from_unary_expr (value_range_equiv *, enum tree_code,
                                      tree, tree);
   void extract_range_from_cond_expr (value_range_equiv *, gassign *);
-  void extract_range_from_comparison (value_range_equiv *, enum tree_code,
-                                     tree, tree, tree);
+  void extract_range_from_comparison (value_range_equiv *, gimple *,
+                                     enum tree_code, tree, tree, tree);
   void vrp_visit_assignment_or_call (gimple*, tree *, value_range_equiv *);
   void vrp_visit_switch_stmt (gswitch *, edge *);

@@ -167,9 +167,9 @@ class vr_values
 };

 inline const value_range_equiv *
-simplify_using_ranges::get_value_range (const_tree op)
+simplify_using_ranges::get_value_range (const_tree op, gimple *stmt)
 {
-  return store->get_value_range (op);
+  return store->get_value_range (op, stmt);
 }

 extern tree get_output_for_vrp (gimple *);
-- 
2.26.2

Re: PING: Fwd: [PATCH 1/2] Add statement context to get_value_range.

[Andrew MacLeod]

On 8/11/20 7:53 AM, Aldy Hernandez via Gcc-patches wrote:
> ---------- Forwarded message ---------
> From: Aldy Hernandez <aldyh@redhat.com>
> Date: Tue, Aug 4, 2020, 13:55
> Subject: [PATCH 1/2] Add statement context to get_value_range.
> To: <gcc-patches@gcc.gnu.org>
> Cc: <law@redhat.com>, Aldy Hernandez <aldyh@redhat.com>
>
>
> This is in line with the statement context that we have for get_value()
> in the substitute_and_fold_engine class.
> ---
>   gcc/vr-values.c | 64 ++++++++++++++++++++++++++-----------------------
>   gcc/vr-values.h | 14 +++++------
>   2 files changed, 41 insertions(+), 37 deletions(-)
>
> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
> index 511342f2f13..9002d87c14b 100644
> --- a/gcc/vr-values.c
> +++ b/gcc/vr-values.c
> @@ -147,7 +147,8 @@ vr_values::get_lattice_entry (const_tree var)
>      return NULL.  Otherwise create an empty range if none existed for VAR.
> */
>
>   const value_range_equiv *
> -vr_values::get_value_range (const_tree var)
> +vr_values::get_value_range (const_tree var,
> +                           gimple *stmt ATTRIBUTE_UNUSED)
>   {
>     /* If we have no recorded ranges, then return NULL.  */
>     if (!vr_value)
> @@ -450,7 +451,7 @@ simplify_using_ranges::op_with_boolean_value_range_p
> (tree op)
>
>     /* ?? Errr, this should probably check for [0,0] and [1,1] as well
>        as [0,1].  */
> -  const value_range *vr = get_value_range (op);
> +  const value_range *vr = get_value_range (op, NULL);
>     return *vr == value_range (build_zero_cst (TREE_TYPE (op)),
>                               build_one_cst (TREE_TYPE (op)));
>   }

I think if we are adding "gimple *stmt" as a parameter, we should make 
if default to NULL...  Then we won't have to change all the callers that 
don't have a need for it.
I get that it helped us find all the places where stmts were 
available/needed originally, but I think that need is no longer relevant 
and we can revert to making it a default parameter now.

further more, I don't think it should be a ATTRIBUTE_UNUSED, and then 
pass a NULL further down :)  we should be able to pass stmt.

> @@ -972,12 +973,13 @@ vr_values::extract_range_from_cond_expr
> (value_range_equiv *vr, gassign *stmt)
>
>   void
>   vr_values::extract_range_from_comparison (value_range_equiv *vr,
> +                                         gimple *stmt,
>                                            enum tree_code code,
>                                            tree type, tree op0, tree op1)

Now that we are passing stmt in, and there is only one use of this 
function, I think you can kill the final 4 parameters and just get them 
in the function itself...

>   {
>     bool sop;
>     tree val
> -    = simplifier.vrp_evaluate_conditional_warnv_with_ops (code, op0, op1,
> +    = simplifier.vrp_evaluate_conditional_warnv_with_ops (stmt, code, op0,
> op1,
>                                                            false, &sop,
> NULL);
>     if (val)
>       {
> @@ -1008,14 +1010,14 @@ check_for_binary_op_overflow (vr_values *store,
>   {
>     value_range vr0, vr1;
>     if (TREE_CODE (op0) == SSA_NAME)
> -    vr0 = *store->get_value_range (op0);
> +    vr0 = *store->get_value_range (op0, NULL);
>     else if (TREE_CODE (op0) == INTEGER_CST)
>       vr0.set (op0);
>     else
>       vr0.set_varying (TREE_TYPE (op0));
>
>     if (TREE_CODE (op1) == SSA_NAME)
> -    vr1 = *store->get_value_range (op1);
> +    vr1 = *store->get_value_range (op1, NULL);
>     else if (TREE_CODE (op1) == INTEGER_CST)
>       vr1.set (op1);
>     else
> @@ -1472,7 +1474,7 @@ vr_values::extract_range_from_assignment
> (value_range_equiv *vr, gassign *stmt)
>     else if (code == COND_EXPR)
>       extract_range_from_cond_expr (vr, stmt);
>     else if (TREE_CODE_CLASS (code) == tcc_comparison)
> -    extract_range_from_comparison (vr, gimple_assign_rhs_code (stmt),
> +    extract_range_from_comparison (vr, stmt, gimple_assign_rhs_code (stmt),
>                                     gimple_expr_type (stmt),
>                                     gimple_assign_rhs1 (stmt),
>                                     gimple_assign_rhs2 (stmt));
> @@ -1805,7 +1807,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)->kind () == VR_RANGE))
> +         || get_value_range (init, stmt)->kind () == VR_RANGE))
>       {
>         widest_int nit;
>
> @@ -1838,7 +1840,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));
> +                   initvr = *(get_value_range (init, stmt));
>                    else if (is_gimple_min_invariant (init))
>                      initvr.set (init);
>                    else
> @@ -2090,7 +2092,7 @@ const value_range_equiv *
>   simplify_using_ranges::get_vr_for_comparison (int i, value_range_equiv
> *tem)
>   {
>     /* Shallow-copy equiv bitmap.  */
> -  const value_range_equiv *vr = get_value_range (ssa_name (i));
> +  const value_range_equiv *vr = get_value_range (ssa_name (i), NULL);
>
>     /* If name N_i does not have a valid range, use N_i as its own
>        range.  This allows us to compare against names that may
> @@ -2115,7 +2117,7 @@ simplify_using_ranges::compare_name_with_value
>                                   bool *strict_overflow_p, bool use_equiv_p)
>   {
>     /* Get the set of equivalences for VAR.  */
> -  bitmap e = get_value_range (var)->equiv ();
> +  bitmap e = get_value_range (var, NULL)->equiv ();
>
>     /* Start at -1.  Set it to 0 if we do a comparison without relying
>        on overflow, or 1 if all comparisons rely on overflow.  */
> @@ -2195,8 +2197,8 @@ simplify_using_ranges::compare_names (enum tree_code
> comp, tree n1, tree n2,
>   {
>     /* Compare the ranges of every name equivalent to N1 against the
>        ranges of every name equivalent to N2.  */
> -  bitmap e1 = get_value_range (n1)->equiv ();
> -  bitmap e2 = get_value_range (n2)->equiv ();
> +  bitmap e1 = get_value_range (n1, NULL)->equiv ();
> +  bitmap e2 = get_value_range (n2, NULL)->equiv ();
>
>     /* Use the fake bitmaps if e1 or e2 are not available.  */
>     static bitmap s_e1 = NULL, s_e2 = NULL;
> @@ -2308,8 +2310,8 @@
> simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops_using_ranges
>       (enum tree_code code, tree op0, tree op1, bool * strict_overflow_p)
>   {
>     const value_range_equiv *vr0, *vr1;
> -  vr0 = (TREE_CODE (op0) == SSA_NAME) ? get_value_range (op0) : NULL;
> -  vr1 = (TREE_CODE (op1) == SSA_NAME) ? get_value_range (op1) : NULL;
> +  vr0 = (TREE_CODE (op0) == SSA_NAME) ? get_value_range (op0, NULL) : NULL;
> +  vr1 = (TREE_CODE (op1) == SSA_NAME) ? get_value_range (op1, NULL) : NULL;
>
>     tree res = NULL_TREE;
>     if (vr0 && vr1)
> @@ -2326,7 +2328,8 @@
> simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops_using_ranges
>
>   tree
>   simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops
> -                                               (enum tree_code code,
> +                                               (gimple *stmt,
> +                                                enum tree_code code,
>                                                   tree op0, tree op1,
>                                                   bool use_equiv_p,
>                                                   bool *strict_overflow_p,

I was really hoping that by passing stmt in here, we could avoid passing 
code, op1 and op2 as well... but unfortunately, with further digging it 
seems that there are issues with VRP .   in particular there are places 
which tweak  op1 and op2 before being passed for consideration...   
specifically  vrp_evaluate_conditional  calls here and has incoming 
tweaks.  they should be fine as far as ranger interaction goes, but 
prevents us from condensing those parameters... :-(

So that is OK for now. Perhaps when we get into replacing VRP, we can 
clean this up more.



> @@ -2387,7 +2390,7 @@
> simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops
>              }
>            else
>              gcc_unreachable ();
> -         const value_range_equiv *vr0 = get_value_range (op0);
> +         const value_range_equiv *vr0 = get_value_range (op0, stmt);
>            /* If vro, the range for OP0 to pass the overflow test, has
>               no intersection with *vr0, OP0's known range, then the
>               overflow test can't pass, so return the node for false.
> @@ -2449,8 +2452,8 @@ simplify_using_ranges::vrp_evaluate_conditional
> (tree_code code, tree op0,
>       return NULL_TREE;
>
>     sop = false;
> -  ret = vrp_evaluate_conditional_warnv_with_ops (code, op0, op1, true,
> &sop,
> -                                                &only_ranges);
> +  ret = vrp_evaluate_conditional_warnv_with_ops (stmt, code, op0, op1,
> true,
> +                                                &sop, &only_ranges);
>
>     if (ret && sop)
>       {
> @@ -2493,7 +2496,7 @@ simplify_using_ranges::vrp_evaluate_conditional
> (tree_code code, tree op0,
>           always fold regardless of the value of OP0.  If -Wtype-limits
>           was specified, emit a warning.  */
>         tree type = TREE_TYPE (op0);
> -      const value_range_equiv *vr0 = get_value_range (op0);
> +      const value_range_equiv *vr0 = get_value_range (op0, stmt);
>
>         if (vr0->varying_p ()
>            && INTEGRAL_TYPE_P (type)
> @@ -2544,7 +2547,7 @@ simplify_using_ranges::vrp_visit_cond_stmt (gcond
> *stmt, edge *taken_edge_p)
>            fprintf (dump_file, "\t");
>            print_generic_expr (dump_file, use);
>            fprintf (dump_file, ": ");
> -         dump_value_range (dump_file, get_value_range (use));
> +         dump_value_range (dump_file, get_value_range (use, stmt));
>          }
>
>         fprintf (dump_file, "\n");
> @@ -2594,7 +2597,8 @@ simplify_using_ranges::vrp_visit_cond_stmt (gcond
> *stmt, edge *taken_edge_p)
>        4 more predicates folded in SPEC.  */
>
>     bool sop;
> -  val = vrp_evaluate_conditional_warnv_with_ops (gimple_cond_code (stmt),
> +  val = vrp_evaluate_conditional_warnv_with_ops (stmt,
> +                                                gimple_cond_code (stmt),
>                                                   gimple_cond_lhs (stmt),
>                                                   gimple_cond_rhs (stmt),
>                                                   false, &sop, NULL);
> @@ -3119,7 +3123,7 @@
> simplify_using_ranges::simplify_div_or_mod_using_ranges
>       }
>     else
>       {
> -      vr = get_value_range (op0);
> +      vr = get_value_range (op0, stmt);
>         if (range_int_cst_p (vr))
>          {
>            op0min = vr->min ();
> @@ -3130,7 +3134,7 @@
> simplify_using_ranges::simplify_div_or_mod_using_ranges
>     if (rhs_code == TRUNC_MOD_EXPR
>         && TREE_CODE (op1) == SSA_NAME)
>       {
> -      const value_range_equiv *vr1 = get_value_range (op1);
> +      const value_range_equiv *vr1 = get_value_range (op1, stmt);
>         if (range_int_cst_p (vr1))
>          op1min = vr1->min ();
>       }
> @@ -3279,7 +3283,7 @@ simplify_using_ranges::simplify_abs_using_ranges
> (gimple_stmt_iterator *gsi,
>                                                    gimple *stmt)
>   {
>     tree op = gimple_assign_rhs1 (stmt);
> -  const value_range *vr = get_value_range (op);
> +  const value_range *vr = get_value_range (op, stmt);
>
>     if (vr)
>       {
> @@ -3369,14 +3373,14 @@ simplify_using_ranges::simplify_bit_ops_using_ranges
>     wide_int mask;
>
>     if (TREE_CODE (op0) == SSA_NAME)
> -    vr0 = *(get_value_range (op0));
> +    vr0 = *(get_value_range (op0, stmt));
>     else if (is_gimple_min_invariant (op0))
>       vr0.set (op0);
>     else
>       return false;
>
>     if (TREE_CODE (op1) == SSA_NAME)
> -    vr1 = *(get_value_range (op1));
> +    vr1 = *(get_value_range (op1, stmt));
>     else if (is_gimple_min_invariant (op1))
>       vr1.set (op1);
>     else
> @@ -3595,7 +3599,7 @@ simplify_using_ranges::simplify_cond_using_ranges_1
> (gcond *stmt)
>         && INTEGRAL_TYPE_P (TREE_TYPE (op0))
>         && is_gimple_min_invariant (op1))
>       {
> -      const value_range *vr = get_value_range (op0);
> +      const value_range *vr = get_value_range (op0, stmt);
>
>         /* If we have range information for OP0, then we might be
>           able to simplify this conditional. */
> @@ -3739,7 +3743,7 @@ simplify_using_ranges::simplify_switch_using_ranges
> (gswitch *stmt)
>
>     if (TREE_CODE (op) == SSA_NAME)
>       {
> -      vr = get_value_range (op);
> +      vr = get_value_range (op, stmt);
>
>         /* We can only handle integer ranges.  */
>         if (vr->varying_p ()
> @@ -4032,7 +4036,7 @@
> simplify_using_ranges::simplify_float_conversion_using_ranges
>                                           gimple *stmt)
>   {
>     tree rhs1 = gimple_assign_rhs1 (stmt);
> -  const value_range *vr = get_value_range (rhs1);
> +  const value_range *vr = get_value_range (rhs1, stmt);
>     scalar_float_mode fltmode
>       = SCALAR_FLOAT_TYPE_MODE (TREE_TYPE (gimple_assign_lhs (stmt)));
>     scalar_int_mode mode;
> @@ -4196,7 +4200,7 @@
> simplify_using_ranges::simplify_internal_call_using_ranges
>   bool
>   simplify_using_ranges::two_valued_val_range_p (tree var, tree *a, tree *b)
>   {
> -  value_range vr = *get_value_range (var);
> +  value_range vr = *get_value_range (var, NULL);
>     vr.normalize_symbolics ();
>     if (vr.varying_p () || vr.undefined_p ())
>       return false;
> diff --git a/gcc/vr-values.h b/gcc/vr-values.h
> index 62a20218c6d..3fbea9bd69f 100644
> --- a/gcc/vr-values.h
> +++ b/gcc/vr-values.h
> @@ -38,12 +38,12 @@ public:
>     // ?? These should be cleaned, merged, and made private.
>     tree vrp_evaluate_conditional (tree_code, tree, tree, gimple *);
>     void vrp_visit_cond_stmt (gcond *, edge *);
> -  tree vrp_evaluate_conditional_warnv_with_ops (enum tree_code,
> +  tree vrp_evaluate_conditional_warnv_with_ops (gimple *stmt, enum
> tree_code,
>                                                  tree, tree, bool,
>                                                  bool *, bool *);
>
>   private:
> -  const value_range_equiv *get_value_range (const_tree op);
> +  const value_range_equiv *get_value_range (const_tree op, gimple *stmt);
>     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 *);
> @@ -101,7 +101,7 @@ class vr_values
>     vr_values (void);
>     ~vr_values (void);
>
> -  const value_range_equiv *get_value_range (const_tree);
> +  const value_range_equiv *get_value_range (const_tree, gimple * = NULL);
>     void set_vr_value (tree, value_range_equiv *);
>     value_range_equiv *swap_vr_value (tree, value_range_equiv *);
>
> @@ -140,8 +140,8 @@ class vr_values
>     void extract_range_from_unary_expr (value_range_equiv *, enum tree_code,
>                                        tree, tree);
>     void extract_range_from_cond_expr (value_range_equiv *, gassign *);
> -  void extract_range_from_comparison (value_range_equiv *, enum tree_code,
> -                                     tree, tree, tree);
> +  void extract_range_from_comparison (value_range_equiv *, gimple *,
> +                                     enum tree_code, tree, tree, tree);
>     void vrp_visit_assignment_or_call (gimple*, tree *, value_range_equiv *);
>     void vrp_visit_switch_stmt (gswitch *, edge *);
>
> @@ -167,9 +167,9 @@ class vr_values
>   };
>
>   inline const value_range_equiv *
> -simplify_using_ranges::get_value_range (const_tree op)
> +simplify_using_ranges::get_value_range (const_tree op, gimple *stmt)
>   {
> -  return store->get_value_range (op);
> +  return store->get_value_range (op, stmt);
>   }
>
>   extern tree get_output_for_vrp (gimple *);

Re: PING: Fwd: [PATCH 1/2] Add statement context to get_value_range.

[Aldy Hernandez]

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



On 8/14/20 6:03 PM, Andrew MacLeod wrote:
> On 8/11/20 7:53 AM, Aldy Hernandez via Gcc-patches wrote:
>> ---------- Forwarded message ---------
>> From: Aldy Hernandez <aldyh@redhat.com>
>> Date: Tue, Aug 4, 2020, 13:55
>> Subject: [PATCH 1/2] Add statement context to get_value_range.
>> To: <gcc-patches@gcc.gnu.org>
>> Cc: <law@redhat.com>, Aldy Hernandez <aldyh@redhat.com>
>>
>>
>> This is in line with the statement context that we have for get_value()
>> in the substitute_and_fold_engine class.
>> ---
>>   gcc/vr-values.c | 64 ++++++++++++++++++++++++++-----------------------
>>   gcc/vr-values.h | 14 +++++------
>>   2 files changed, 41 insertions(+), 37 deletions(-)
>>
>> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
>> index 511342f2f13..9002d87c14b 100644
>> --- a/gcc/vr-values.c
>> +++ b/gcc/vr-values.c
>> @@ -147,7 +147,8 @@ vr_values::get_lattice_entry (const_tree var)
>>      return NULL.  Otherwise create an empty range if none existed for 
>> VAR.
>> */
>>
>>   const value_range_equiv *
>> -vr_values::get_value_range (const_tree var)
>> +vr_values::get_value_range (const_tree var,
>> +                           gimple *stmt ATTRIBUTE_UNUSED)
>>   {
>>     /* If we have no recorded ranges, then return NULL.  */
>>     if (!vr_value)
>> @@ -450,7 +451,7 @@ simplify_using_ranges::op_with_boolean_value_range_p
>> (tree op)
>>
>>     /* ?? Errr, this should probably check for [0,0] and [1,1] as well
>>        as [0,1].  */
>> -  const value_range *vr = get_value_range (op);
>> +  const value_range *vr = get_value_range (op, NULL);
>>     return *vr == value_range (build_zero_cst (TREE_TYPE (op)),
>>                               build_one_cst (TREE_TYPE (op)));
>>   }
> 
> I think if we are adding "gimple *stmt" as a parameter, we should make 
> if default to NULL...  Then we won't have to change all the callers that 
> don't have a need for it.
> I get that it helped us find all the places where stmts were 
> available/needed originally, but I think that need is no longer relevant 
> and we can revert to making it a default parameter now.

Done.

> 
> further more, I don't think it should be a ATTRIBUTE_UNUSED, and then 
> pass a NULL further down :)  we should be able to pass stmt.
> 
>> @@ -972,12 +973,13 @@ vr_values::extract_range_from_cond_expr
>> (value_range_equiv *vr, gassign *stmt)
>>
>>   void
>>   vr_values::extract_range_from_comparison (value_range_equiv *vr,
>> +                                         gimple *stmt,
>>                                            enum tree_code code,
>>                                            tree type, tree op0, tree op1)
> 
> Now that we are passing stmt in, and there is only one use of this 
> function, I think you can kill the final 4 parameters and just get them 
> in the function itself...

Done.

> 
>>   {
>>     bool sop;
>>     tree val
>> -    = simplifier.vrp_evaluate_conditional_warnv_with_ops (code, op0, 
>> op1,
>> +    = simplifier.vrp_evaluate_conditional_warnv_with_ops (stmt, code, 
>> op0,
>> op1,
>>                                                            false, &sop,
>> NULL);
>>     if (val)
>>       {
>> @@ -1008,14 +1010,14 @@ check_for_binary_op_overflow (vr_values *store,
>>   {
>>     value_range vr0, vr1;
>>     if (TREE_CODE (op0) == SSA_NAME)
>> -    vr0 = *store->get_value_range (op0);
>> +    vr0 = *store->get_value_range (op0, NULL);
>>     else if (TREE_CODE (op0) == INTEGER_CST)
>>       vr0.set (op0);
>>     else
>>       vr0.set_varying (TREE_TYPE (op0));
>>
>>     if (TREE_CODE (op1) == SSA_NAME)
>> -    vr1 = *store->get_value_range (op1);
>> +    vr1 = *store->get_value_range (op1, NULL);
>>     else if (TREE_CODE (op1) == INTEGER_CST)
>>       vr1.set (op1);
>>     else
>> @@ -1472,7 +1474,7 @@ vr_values::extract_range_from_assignment
>> (value_range_equiv *vr, gassign *stmt)
>>     else if (code == COND_EXPR)
>>       extract_range_from_cond_expr (vr, stmt);
>>     else if (TREE_CODE_CLASS (code) == tcc_comparison)
>> -    extract_range_from_comparison (vr, gimple_assign_rhs_code (stmt),
>> +    extract_range_from_comparison (vr, stmt, gimple_assign_rhs_code 
>> (stmt),
>>                                     gimple_expr_type (stmt),
>>                                     gimple_assign_rhs1 (stmt),
>>                                     gimple_assign_rhs2 (stmt));
>> @@ -1805,7 +1807,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)->kind () == VR_RANGE))
>> +         || get_value_range (init, stmt)->kind () == VR_RANGE))
>>       {
>>         widest_int nit;
>>
>> @@ -1838,7 +1840,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));
>> +                   initvr = *(get_value_range (init, stmt));
>>                    else if (is_gimple_min_invariant (init))
>>                      initvr.set (init);
>>                    else
>> @@ -2090,7 +2092,7 @@ const value_range_equiv *
>>   simplify_using_ranges::get_vr_for_comparison (int i, value_range_equiv
>> *tem)
>>   {
>>     /* Shallow-copy equiv bitmap.  */
>> -  const value_range_equiv *vr = get_value_range (ssa_name (i));
>> +  const value_range_equiv *vr = get_value_range (ssa_name (i), NULL);
>>
>>     /* If name N_i does not have a valid range, use N_i as its own
>>        range.  This allows us to compare against names that may
>> @@ -2115,7 +2117,7 @@ simplify_using_ranges::compare_name_with_value
>>                                   bool *strict_overflow_p, bool 
>> use_equiv_p)
>>   {
>>     /* Get the set of equivalences for VAR.  */
>> -  bitmap e = get_value_range (var)->equiv ();
>> +  bitmap e = get_value_range (var, NULL)->equiv ();
>>
>>     /* Start at -1.  Set it to 0 if we do a comparison without relying
>>        on overflow, or 1 if all comparisons rely on overflow.  */
>> @@ -2195,8 +2197,8 @@ simplify_using_ranges::compare_names (enum 
>> tree_code
>> comp, tree n1, tree n2,
>>   {
>>     /* Compare the ranges of every name equivalent to N1 against the
>>        ranges of every name equivalent to N2.  */
>> -  bitmap e1 = get_value_range (n1)->equiv ();
>> -  bitmap e2 = get_value_range (n2)->equiv ();
>> +  bitmap e1 = get_value_range (n1, NULL)->equiv ();
>> +  bitmap e2 = get_value_range (n2, NULL)->equiv ();
>>
>>     /* Use the fake bitmaps if e1 or e2 are not available.  */
>>     static bitmap s_e1 = NULL, s_e2 = NULL;
>> @@ -2308,8 +2310,8 @@
>> simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops_using_ranges 
>>
>>       (enum tree_code code, tree op0, tree op1, bool * strict_overflow_p)
>>   {
>>     const value_range_equiv *vr0, *vr1;
>> -  vr0 = (TREE_CODE (op0) == SSA_NAME) ? get_value_range (op0) : NULL;
>> -  vr1 = (TREE_CODE (op1) == SSA_NAME) ? get_value_range (op1) : NULL;
>> +  vr0 = (TREE_CODE (op0) == SSA_NAME) ? get_value_range (op0, NULL) : 
>> NULL;
>> +  vr1 = (TREE_CODE (op1) == SSA_NAME) ? get_value_range (op1, NULL) : 
>> NULL;
>>
>>     tree res = NULL_TREE;
>>     if (vr0 && vr1)
>> @@ -2326,7 +2328,8 @@
>> simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops_using_ranges 
>>
>>
>>   tree
>>   simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops
>> -                                               (enum tree_code code,
>> +                                               (gimple *stmt,
>> +                                                enum tree_code code,
>>                                                   tree op0, tree op1,
>>                                                   bool use_equiv_p,
>>                                                   bool 
>> *strict_overflow_p,
> 
> I was really hoping that by passing stmt in here, we could avoid passing 
> code, op1 and op2 as well... but unfortunately, with further digging it 
> seems that there are issues with VRP .   in particular there are places 
> which tweak  op1 and op2 before being passed for consideration... 
> specifically  vrp_evaluate_conditional  calls here and has incoming 
> tweaks.  they should be fine as far as ranger interaction goes, but 
> prevents us from condensing those parameters... :-(
> 
> So that is OK for now. Perhaps when we get into replacing VRP, we can 
> clean this up more.

Tested on ppc64le-linux and pushed the attached patch.

Aldy

[-- Attachment #2: 0001-Add-statement-context-to-get_value_range.patch --]
[-- Type: text/x-patch, Size: 10911 bytes --]

From d8b8023cdb0b275c3f4254380b7e41d14f5cb79f Mon Sep 17 00:00:00 2001
Date: Tue, 4 Aug 2020 12:18:21 +0200
Subject: [PATCH] Add statement context to get_value_range.

This is in line with the statement context that we have for get_value()
in the substitute_and_fold_engine class.

gcc/ChangeLog:

	* vr-values.c (vr_values::get_value_range): Add stmt param.
	(vr_values::extract_range_from_comparison): Same.
	(vr_values::extract_range_from_assignment): Pass stmt to
	extract_range_from_comparison.
	(vr_values::adjust_range_with_scev): Pass stmt to get_value_range.
	(simplify_using_ranges::vrp_evaluate_conditional): Add stmt param.
	Pass stmt to get_value_range.
	(simplify_using_ranges::vrp_visit_cond_stmt): Pass stmt to
	get_value_range.
	(simplify_using_ranges::simplify_abs_using_ranges): Same.
	(simplify_using_ranges::simplify_div_or_mod_using_ranges): Same.
	(simplify_using_ranges::simplify_bit_ops_using_ranges): Same.
	(simplify_using_ranges::simplify_cond_using_ranges_1): Same.
	(simplify_using_ranges::simplify_switch_using_ranges): Same.
	(simplify_using_ranges::simplify_float_conversion_using_ranges): Same.
	* vr-values.h (class vr_values): Add stmt arg to
	vrp_evaluate_conditional_warnv_with_ops.
	Add stmt arg to extract_range_from_comparison and get_value_range.
	(simplify_using_ranges::get_value_range): Add stmt arg.
---
 gcc/vr-values.c | 53 ++++++++++++++++++++++++++-----------------------
 gcc/vr-values.h | 14 ++++++-------
 2 files changed, 35 insertions(+), 32 deletions(-)

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 511342f2f13..fe51a6faeb8 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -147,7 +147,8 @@ vr_values::get_lattice_entry (const_tree var)
    return NULL.  Otherwise create an empty range if none existed for VAR.  */
 
 const value_range_equiv *
-vr_values::get_value_range (const_tree var)
+vr_values::get_value_range (const_tree var,
+			    gimple *stmt ATTRIBUTE_UNUSED)
 {
   /* If we have no recorded ranges, then return NULL.  */
   if (!vr_value)
@@ -972,12 +973,15 @@ vr_values::extract_range_from_cond_expr (value_range_equiv *vr, gassign *stmt)
 
 void
 vr_values::extract_range_from_comparison (value_range_equiv *vr,
-					  enum tree_code code,
-					  tree type, tree op0, tree op1)
+					  gimple *stmt)
 {
+  enum tree_code code = gimple_assign_rhs_code (stmt);
+  tree type = gimple_expr_type (stmt);
+  tree op0 = gimple_assign_rhs1 (stmt);
+  tree op1 = gimple_assign_rhs2 (stmt);
   bool sop;
   tree val
-    = simplifier.vrp_evaluate_conditional_warnv_with_ops (code, op0, op1,
+    = simplifier.vrp_evaluate_conditional_warnv_with_ops (stmt, code, op0, op1,
 							  false, &sop, NULL);
   if (val)
     {
@@ -1472,10 +1476,7 @@ vr_values::extract_range_from_assignment (value_range_equiv *vr, gassign *stmt)
   else if (code == COND_EXPR)
     extract_range_from_cond_expr (vr, stmt);
   else if (TREE_CODE_CLASS (code) == tcc_comparison)
-    extract_range_from_comparison (vr, gimple_assign_rhs_code (stmt),
-				   gimple_expr_type (stmt),
-				   gimple_assign_rhs1 (stmt),
-				   gimple_assign_rhs2 (stmt));
+    extract_range_from_comparison (vr, stmt);
   else if (get_gimple_rhs_class (code) == GIMPLE_SINGLE_RHS
 	   && is_gimple_min_invariant (gimple_assign_rhs1 (stmt)))
     vr->set (gimple_assign_rhs1 (stmt));
@@ -1805,7 +1806,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)->kind () == VR_RANGE))
+	  || get_value_range (init, stmt)->kind () == VR_RANGE))
     {
       widest_int nit;
 
@@ -1838,7 +1839,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));
+		    initvr = *(get_value_range (init, stmt));
 		  else if (is_gimple_min_invariant (init))
 		    initvr.set (init);
 		  else
@@ -2326,7 +2327,8 @@ simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops_using_ranges
 
 tree
 simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops
-						(enum tree_code code,
+						(gimple *stmt,
+						 enum tree_code code,
 						 tree op0, tree op1,
 						 bool use_equiv_p,
 						 bool *strict_overflow_p,
@@ -2387,7 +2389,7 @@ simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops
 	    }
 	  else
 	    gcc_unreachable ();
-	  const value_range_equiv *vr0 = get_value_range (op0);
+	  const value_range_equiv *vr0 = get_value_range (op0, stmt);
 	  /* If vro, the range for OP0 to pass the overflow test, has
 	     no intersection with *vr0, OP0's known range, then the
 	     overflow test can't pass, so return the node for false.
@@ -2449,8 +2451,8 @@ simplify_using_ranges::vrp_evaluate_conditional (tree_code code, tree op0,
     return NULL_TREE;
 
   sop = false;
-  ret = vrp_evaluate_conditional_warnv_with_ops (code, op0, op1, true, &sop,
-  						 &only_ranges);
+  ret = vrp_evaluate_conditional_warnv_with_ops (stmt, code, op0, op1, true,
+						 &sop, &only_ranges);
 
   if (ret && sop)
     {
@@ -2493,7 +2495,7 @@ simplify_using_ranges::vrp_evaluate_conditional (tree_code code, tree op0,
 	 always fold regardless of the value of OP0.  If -Wtype-limits
 	 was specified, emit a warning.  */
       tree type = TREE_TYPE (op0);
-      const value_range_equiv *vr0 = get_value_range (op0);
+      const value_range_equiv *vr0 = get_value_range (op0, stmt);
 
       if (vr0->varying_p ()
 	  && INTEGRAL_TYPE_P (type)
@@ -2544,7 +2546,7 @@ simplify_using_ranges::vrp_visit_cond_stmt (gcond *stmt, edge *taken_edge_p)
 	  fprintf (dump_file, "\t");
 	  print_generic_expr (dump_file, use);
 	  fprintf (dump_file, ": ");
-	  dump_value_range (dump_file, get_value_range (use));
+	  dump_value_range (dump_file, get_value_range (use, stmt));
 	}
 
       fprintf (dump_file, "\n");
@@ -2594,7 +2596,8 @@ simplify_using_ranges::vrp_visit_cond_stmt (gcond *stmt, edge *taken_edge_p)
      4 more predicates folded in SPEC.  */
 
   bool sop;
-  val = vrp_evaluate_conditional_warnv_with_ops (gimple_cond_code (stmt),
+  val = vrp_evaluate_conditional_warnv_with_ops (stmt,
+						 gimple_cond_code (stmt),
 						 gimple_cond_lhs (stmt),
 						 gimple_cond_rhs (stmt),
 						 false, &sop, NULL);
@@ -3119,7 +3122,7 @@ simplify_using_ranges::simplify_div_or_mod_using_ranges
     }
   else
     {
-      vr = get_value_range (op0);
+      vr = get_value_range (op0, stmt);
       if (range_int_cst_p (vr))
 	{
 	  op0min = vr->min ();
@@ -3130,7 +3133,7 @@ simplify_using_ranges::simplify_div_or_mod_using_ranges
   if (rhs_code == TRUNC_MOD_EXPR
       && TREE_CODE (op1) == SSA_NAME)
     {
-      const value_range_equiv *vr1 = get_value_range (op1);
+      const value_range_equiv *vr1 = get_value_range (op1, stmt);
       if (range_int_cst_p (vr1))
 	op1min = vr1->min ();
     }
@@ -3279,7 +3282,7 @@ simplify_using_ranges::simplify_abs_using_ranges (gimple_stmt_iterator *gsi,
 						  gimple *stmt)
 {
   tree op = gimple_assign_rhs1 (stmt);
-  const value_range *vr = get_value_range (op);
+  const value_range *vr = get_value_range (op, stmt);
 
   if (vr)
     {
@@ -3369,14 +3372,14 @@ simplify_using_ranges::simplify_bit_ops_using_ranges
   wide_int mask;
 
   if (TREE_CODE (op0) == SSA_NAME)
-    vr0 = *(get_value_range (op0));
+    vr0 = *(get_value_range (op0, stmt));
   else if (is_gimple_min_invariant (op0))
     vr0.set (op0);
   else
     return false;
 
   if (TREE_CODE (op1) == SSA_NAME)
-    vr1 = *(get_value_range (op1));
+    vr1 = *(get_value_range (op1, stmt));
   else if (is_gimple_min_invariant (op1))
     vr1.set (op1);
   else
@@ -3595,7 +3598,7 @@ simplify_using_ranges::simplify_cond_using_ranges_1 (gcond *stmt)
       && INTEGRAL_TYPE_P (TREE_TYPE (op0))
       && is_gimple_min_invariant (op1))
     {
-      const value_range *vr = get_value_range (op0);
+      const value_range *vr = get_value_range (op0, stmt);
 
       /* If we have range information for OP0, then we might be
 	 able to simplify this conditional. */
@@ -3739,7 +3742,7 @@ simplify_using_ranges::simplify_switch_using_ranges (gswitch *stmt)
 
   if (TREE_CODE (op) == SSA_NAME)
     {
-      vr = get_value_range (op);
+      vr = get_value_range (op, stmt);
 
       /* We can only handle integer ranges.  */
       if (vr->varying_p ()
@@ -4032,7 +4035,7 @@ simplify_using_ranges::simplify_float_conversion_using_ranges
 					 gimple *stmt)
 {
   tree rhs1 = gimple_assign_rhs1 (stmt);
-  const value_range *vr = get_value_range (rhs1);
+  const value_range *vr = get_value_range (rhs1, stmt);
   scalar_float_mode fltmode
     = SCALAR_FLOAT_TYPE_MODE (TREE_TYPE (gimple_assign_lhs (stmt)));
   scalar_int_mode mode;
diff --git a/gcc/vr-values.h b/gcc/vr-values.h
index 62a20218c6d..330b4605e39 100644
--- a/gcc/vr-values.h
+++ b/gcc/vr-values.h
@@ -38,12 +38,13 @@ public:
   // ?? These should be cleaned, merged, and made private.
   tree vrp_evaluate_conditional (tree_code, tree, tree, gimple *);
   void vrp_visit_cond_stmt (gcond *, edge *);
-  tree vrp_evaluate_conditional_warnv_with_ops (enum tree_code,
+  tree vrp_evaluate_conditional_warnv_with_ops (gimple *stmt, enum tree_code,
 						tree, tree, bool,
 						bool *, bool *);
 
 private:
-  const value_range_equiv *get_value_range (const_tree op);
+  const value_range_equiv *get_value_range (const_tree op,
+					    gimple *stmt = NULL);
   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 *);
@@ -101,7 +102,7 @@ class vr_values
   vr_values (void);
   ~vr_values (void);
 
-  const value_range_equiv *get_value_range (const_tree);
+  const value_range_equiv *get_value_range (const_tree, gimple * = NULL);
   void set_vr_value (tree, value_range_equiv *);
   value_range_equiv *swap_vr_value (tree, value_range_equiv *);
 
@@ -140,8 +141,7 @@ class vr_values
   void extract_range_from_unary_expr (value_range_equiv *, enum tree_code,
 				      tree, tree);
   void extract_range_from_cond_expr (value_range_equiv *, gassign *);
-  void extract_range_from_comparison (value_range_equiv *, enum tree_code,
-				      tree, tree, tree);
+  void extract_range_from_comparison (value_range_equiv *, gimple *);
   void vrp_visit_assignment_or_call (gimple*, tree *, value_range_equiv *);
   void vrp_visit_switch_stmt (gswitch *, edge *);
 
@@ -167,9 +167,9 @@ class vr_values
 };
 
 inline const value_range_equiv *
-simplify_using_ranges::get_value_range (const_tree op)
+simplify_using_ranges::get_value_range (const_tree op, gimple *stmt)
 {
-  return store->get_value_range (op);
+  return store->get_value_range (op, stmt);
 }
 
 extern tree get_output_for_vrp (gimple *);
-- 
2.26.2

[PATCH 2/2] Decouple adjust_range_from_scev from vr_values and value_range_equiv.

[Aldy Hernandez]

I've abstracted out the parts of the code that had nothing to do with
value_range_equiv into an externally visible range_of_var_in_loop().
This way, it can be called with any range.

adjust_range_with_scev still works as before, intersecting with a
known range.  Due to the way value_range_equiv::intersect works,
intersecting a value_range_equiv with no equivalences into one
with equivalences will result in the resulting range maintaining
whatever equivalences it had.  So everything works as the
vr->update() did before (remember that ::update() retains
equivalences).

OK?

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 | 140 ++++++++++++++++++++++--------------------------
 gcc/vr-values.h |  23 ++++++--
 2 files changed, 81 insertions(+), 82 deletions(-)

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 9002d87c14b..e7f97bdbf7b 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.  */
 
 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;
-
   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;
+    }
 
   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 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,20 @@ 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));
+}
+
+void
+vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
+				   gimple *stmt, tree var)
+{
+  value_range_equiv r;
+  range_of_var_in_loop (&r, this, loop, stmt, var);
+  vr->intersect (&r);
 }
 
 /* Dump value ranges of all SSA_NAMEs to FILE.  */
@@ -4217,7 +4203,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 3fbea9bd69f..28bccf62063 100644
--- a/gcc/vr-values.h
+++ b/gcc/vr-values.h
@@ -22,16 +22,25 @@ 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 *) = 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 *);
 
@@ -43,7 +52,8 @@ public:
 						bool *, bool *);
 
 private:
-  const value_range_equiv *get_value_range (const_tree op, gimple *stmt);
+  const value_range_equiv *get_value_range (const_tree op, gimple *stmt)
+    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 *);
@@ -78,7 +88,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
@@ -95,7 +105,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);
@@ -177,4 +187,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 void range_of_var_in_loop (irange *, range_query *,
+				  class loop *loop, gimple *stmt, tree var);
+
 #endif /* GCC_VR_VALUES_H */
-- 
2.26.2

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

[Richard Biener]

On Tue, Aug 4, 2020 at 2:05 PM Aldy Hernandez via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> I've abstracted out the parts of the code that had nothing to do with
> value_range_equiv into an externally visible range_of_var_in_loop().
> This way, it can be called with any range.
>
> adjust_range_with_scev still works as before, intersecting with a
> known range.  Due to the way value_range_equiv::intersect works,
> intersecting a value_range_equiv with no equivalences into one
> with equivalences will result in the resulting range maintaining
> whatever equivalences it had.  So everything works as the
> vr->update() did before (remember that ::update() retains
> equivalences).
>
> OK?
>
> 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 | 140 ++++++++++++++++++++++--------------------------
>  gcc/vr-values.h |  23 ++++++--
>  2 files changed, 81 insertions(+), 82 deletions(-)
>
> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
> index 9002d87c14b..e7f97bdbf7b 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.  */

Certainly this comment needs updating now.  It's tempting to provide
a range from the scalar evolution info separately from "adjusting" a range,
at least the comment suggests we'll not always do so.  I'm not sure
your patch factors that decision out or simply returns [-INF,+INF] for
intersection.  For example...

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

... this (probably the worst example).  The rest seem to be more
correctness issues than profitability.

>    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;
> +    }
>
>    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 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,20 @@ 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));
> +}
> +
> +void
> +vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
> +                                  gimple *stmt, tree var)
> +{
> +  value_range_equiv r;
> +  range_of_var_in_loop (&r, this, loop, stmt, var);
> +  vr->intersect (&r);
>  }
>
>  /* Dump value ranges of all SSA_NAMEs to FILE.  */
> @@ -4217,7 +4203,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 3fbea9bd69f..28bccf62063 100644
> --- a/gcc/vr-values.h
> +++ b/gcc/vr-values.h
> @@ -22,16 +22,25 @@ 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 *) = 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 *);
>
> @@ -43,7 +52,8 @@ public:
>                                                 bool *, bool *);
>
>  private:
> -  const value_range_equiv *get_value_range (const_tree op, gimple *stmt);
> +  const value_range_equiv *get_value_range (const_tree op, gimple *stmt)
> +    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 *);
> @@ -78,7 +88,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
> @@ -95,7 +105,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);
> @@ -177,4 +187,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 void range_of_var_in_loop (irange *, range_query *,
> +                                 class loop *loop, gimple *stmt, tree var);
> +
>  #endif /* GCC_VR_VALUES_H */
> --
> 2.26.2
>

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

[Aldy Hernandez]

On Tue, Aug 4, 2020 at 3:27 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Tue, Aug 4, 2020 at 2:05 PM Aldy Hernandez via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > I've abstracted out the parts of the code that had nothing to do with
> > value_range_equiv into an externally visible range_of_var_in_loop().
> > This way, it can be called with any range.
> >
> > adjust_range_with_scev still works as before, intersecting with a
> > known range.  Due to the way value_range_equiv::intersect works,
> > intersecting a value_range_equiv with no equivalences into one
> > with equivalences will result in the resulting range maintaining
> > whatever equivalences it had.  So everything works as the
> > vr->update() did before (remember that ::update() retains
> > equivalences).
> >
> > OK?
> >
> > 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 | 140 ++++++++++++++++++++++--------------------------
> >  gcc/vr-values.h |  23 ++++++--
> >  2 files changed, 81 insertions(+), 82 deletions(-)
> >
> > diff --git a/gcc/vr-values.c b/gcc/vr-values.c
> > index 9002d87c14b..e7f97bdbf7b 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.  */
>
> Certainly this comment needs updating now.  It's tempting to provide
> a range from the scalar evolution info separately from "adjusting" a range,
> at least the comment suggests we'll not always do so.  I'm not sure
> your patch factors that decision out or simply returns [-INF,+INF] for
> intersection.  For example...

The comment belonged to the original method which is now a wrapper.  I've moved
the comment to its original location and have added a  comment to the
new function.  And yes,
we return VARYING if we weren't able to determine a range.  I've documented it.

Thanks.
Aldy

>
> >  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;
> > -
>
> ... this (probably the worst example).  The rest seem to be more
> correctness issues than profitability.
>
> >    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;
> > +    }
> >
> >    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 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,20 @@ 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));
> > +}
> > +
> > +void
> > +vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
> > +                                  gimple *stmt, tree var)
> > +{
> > +  value_range_equiv r;
> > +  range_of_var_in_loop (&r, this, loop, stmt, var);
> > +  vr->intersect (&r);
> >  }
> >
> >  /* Dump value ranges of all SSA_NAMEs to FILE.  */
> > @@ -4217,7 +4203,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 3fbea9bd69f..28bccf62063 100644
> > --- a/gcc/vr-values.h
> > +++ b/gcc/vr-values.h
> > @@ -22,16 +22,25 @@ 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 *) = 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 *);
> >
> > @@ -43,7 +52,8 @@ public:
> >                                                 bool *, bool *);
> >
> >  private:
> > -  const value_range_equiv *get_value_range (const_tree op, gimple *stmt);
> > +  const value_range_equiv *get_value_range (const_tree op, gimple *stmt)
> > +    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 *);
> > @@ -78,7 +88,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
> > @@ -95,7 +105,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);
> > @@ -177,4 +187,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 void range_of_var_in_loop (irange *, range_query *,
> > +                                 class loop *loop, gimple *stmt, tree var);
> > +
> >  #endif /* GCC_VR_VALUES_H */
> > --
> > 2.26.2
> >
>

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

[Aldy Hernandez]

---------- Forwarded message ---------
From: Aldy Hernandez <aldyh@redhat.com>
Date: Tue, Aug 4, 2020, 14:03
Subject: [PATCH 2/2] Decouple adjust_range_from_scev from vr_values and
value_range_equiv.
To: <gcc-patches@gcc.gnu.org>
Cc: <law@redhat.com>, Aldy Hernandez <aldyh@redhat.com>


I've abstracted out the parts of the code that had nothing to do with
value_range_equiv into an externally visible range_of_var_in_loop().
This way, it can be called with any range.

adjust_range_with_scev still works as before, intersecting with a
known range.  Due to the way value_range_equiv::intersect works,
intersecting a value_range_equiv with no equivalences into one
with equivalences will result in the resulting range maintaining
whatever equivalences it had.  So everything works as the
vr->update() did before (remember that ::update() retains
equivalences).

OK?

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 | 140 ++++++++++++++++++++++--------------------------
 gcc/vr-values.h |  23 ++++++--
 2 files changed, 81 insertions(+), 82 deletions(-)

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 9002d87c14b..e7f97bdbf7b 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.  */

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

   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 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,20 @@ 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));
+}
+
+void
+vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
+                                  gimple *stmt, tree var)
+{
+  value_range_equiv r;
+  range_of_var_in_loop (&r, this, loop, stmt, var);
+  vr->intersect (&r);
 }

 /* Dump value ranges of all SSA_NAMEs to FILE.  */
@@ -4217,7 +4203,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 3fbea9bd69f..28bccf62063 100644
--- a/gcc/vr-values.h
+++ b/gcc/vr-values.h
@@ -22,16 +22,25 @@ 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 *)
= 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 *);

@@ -43,7 +52,8 @@ public:
                                                bool *, bool *);

 private:
-  const value_range_equiv *get_value_range (const_tree op, gimple *stmt);
+  const value_range_equiv *get_value_range (const_tree op, gimple *stmt)
+    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 *);
@@ -78,7 +88,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
@@ -95,7 +105,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);
@@ -177,4 +187,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 void range_of_var_in_loop (irange *, range_query *,
+                                 class loop *loop, gimple *stmt, tree var);
+
 #endif /* GCC_VR_VALUES_H */
-- 
2.26.2

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

[Aldy Hernandez]

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

    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 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));
+}
+
+/* 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.  */
+
+void
+vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
+				   gimple *stmt, tree var)
+{
+  value_range_equiv r;
+  range_of_var_in_loop (&r, this, loop, stmt, var);
+  vr->intersect (&r);
  }

  /* Dump value ranges of all SSA_NAMEs to FILE.  */
@@ -4217,7 +4207,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 3fbea9bd69f..28bccf62063 100644
--- a/gcc/vr-values.h
+++ b/gcc/vr-values.h
@@ -22,16 +22,25 @@ 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 
*) = 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 *);

@@ -43,7 +52,8 @@ public:
  						bool *, bool *);

  private:
-  const value_range_equiv *get_value_range (const_tree op, gimple *stmt);
+  const value_range_equiv *get_value_range (const_tree op, gimple *stmt)
+    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 *);
@@ -78,7 +88,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
@@ -95,7 +105,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);
@@ -177,4 +187,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 void range_of_var_in_loop (irange *, range_query *,
+				  class loop *loop, gimple *stmt, tree var);
+
  #endif /* GCC_VR_VALUES_H */
-- 
2.26.2

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

[Andrew MacLeod]

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


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


>
>    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 :-)


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



> +
> +/* 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.  */
> +
> +void
> +vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop 
> *loop,
> +                   gimple *stmt, tree var)
> +{
> +  value_range_equiv r;
> +  range_of_var_in_loop (&r, this, loop, stmt, var);
> +  vr->intersect (&r);
>  }
>
>  /* Dump value ranges of all SSA_NAMEs to FILE.  */
> @@ -4217,7 +4207,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 3fbea9bd69f..28bccf62063 100644
> --- a/gcc/vr-values.h
> +++ b/gcc/vr-values.h
> @@ -22,16 +22,25 @@ 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 *) = 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 *);
>
> @@ -43,7 +52,8 @@ public:
>                          bool *, bool *);
>
>  private:
> -  const value_range_equiv *get_value_range (const_tree op, gimple 
> *stmt);
> +  const value_range_equiv *get_value_range (const_tree op, gimple *stmt)
> +    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 *);
> @@ -78,7 +88,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
> @@ -95,7 +105,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);
> @@ -177,4 +187,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 void range_of_var_in_loop (irange *, range_query *,
> +                  class loop *loop, gimple *stmt, tree var);
> +
>  #endif /* GCC_VR_VALUES_H */

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

[Aldy Hernandez]

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



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?

Aldy

[-- Attachment #2: 0001-Decouple-adjust_range_from_scev-from-vr_values-and-v.patch --]
[-- Type: text/x-patch, Size: 11558 bytes --]

From ac80cba3410563175fe21ed4891911cce5d91011 Mon Sep 17 00:00:00 2001
Date: Tue, 4 Aug 2020 12:31:23 +0200
Subject: [PATCH] Decouple adjust_range_from_scev from vr_values and
 value_range_equiv.

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 | 155 ++++++++++++++++++++++--------------------------
 gcc/vr-values.h |  23 +++++--
 2 files changed, 90 insertions(+), 88 deletions(-)

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index fe51a6faeb8..4c7501da53c 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,21 +1736,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.  */
 
-void
-vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
-				   gimple *stmt, tree var)
+/* Given a VAR in STMT within LOOP, determine the range of the
+   variable and store it in VR.  If a range was set, return
+   true, otherwise return false and set VR to varying.  */
+
+bool
+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 = TREE_TYPE (var);
   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;
+  vr->set_varying (type);
 
   chrec = instantiate_parameters (loop, analyze_scalar_evolution (loop, var));
 
@@ -1758,20 +1755,22 @@ vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
   if (is_gimple_min_invariant (chrec))
     {
       vr->set (chrec);
-      return;
+      return true;
     }
 
   if (TREE_CODE (chrec) != POLYNOMIAL_CHREC)
-    return;
+    return false;
 
   init = initial_condition_in_loop_num (chrec, loop->num);
-  tem = op_with_constant_singleton_value_range (init);
-  if (tem)
-    init = tem;
   step = evolution_part_in_loop_num (chrec, loop->num);
-  tem = op_with_constant_singleton_value_range (step);
-  if (tem)
-    step = tem;
+
+  /* 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
@@ -1780,7 +1779,7 @@ 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;
+    return false;
 
   dir = scev_direction (chrec);
   if (/* Do not adjust ranges if we do not know whether the iv increases
@@ -1789,9 +1788,8 @@ 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;
+    return false;
 
-  type = TREE_TYPE (var);
   if (POINTER_TYPE_P (type) || !TYPE_MIN_VALUE (type))
     tmin = lower_bound_in_type (type, type);
   else
@@ -1806,7 +1804,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;
 
@@ -1829,21 +1827,29 @@ 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;
+		    return false;
 
 		  /* Check if init + nit * step overflows.  Though we checked
 		     scev {init, step}_loop doesn't wrap, it is not enough
@@ -1853,7 +1859,7 @@ 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;
+		    return false;
 
 		  tmin = maxvr.min ();
 		  tmax = maxvr.max ();
@@ -1862,56 +1868,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
@@ -1921,7 +1883,34 @@ 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 0
+  if (TREE_CODE (min) != INTEGER_CST)
+    min = tmin;
+  if (TREE_CODE (max) != INTEGER_CST)
+    max = tmax;
+  vr->set (min, max);
+  return !vr->varying_p ();
+#endif
+  if (TREE_CODE (min) == INTEGER_CST && TREE_CODE (max) == INTEGER_CST)
+    {
+      vr->set (min, max);
+      return true;
+    }
+  return false;
+}
+
+/* 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.  */
+
+void
+vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
+				   gimple *stmt, tree var)
+{
+  value_range_equiv r;
+  if (range_of_var_in_loop (&r, this, loop, stmt, var))
+    vr->intersect (&r);
 }
 
 /* Dump value ranges of all SSA_NAMEs to FILE.  */
@@ -4216,7 +4205,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..889ea04d08a 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);
@@ -177,4 +187,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 range_of_var_in_loop (irange *, range_query *,
+				  class loop *loop, gimple *stmt, tree var);
+
 #endif /* GCC_VR_VALUES_H */
-- 
2.26.2

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

[Andrew MacLeod]

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?

I guess its possible that the loop bounds info may return symbolics, 
which wont jive well with ranger/multi-range code.   I expect we'll have 
to enhance the loop interface for ranger later anyway.



  Andrew

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

[Aldy Hernandez]

[-- 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 */

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

[Aldy Hernandez]

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

And here's the patch without the sanity check.

Aldy

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

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index fe51a6faeb8..9b21441dff3 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,42 +1736,40 @@ 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.  */
 
-void
-vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
-				   gimple *stmt, tree var)
+/* 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, min, max, type, tem;
+  tree init, step, chrec, tmin, tmax, type = TREE_TYPE (var);
   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;
-
   chrec = instantiate_parameters (loop, analyze_scalar_evolution (loop, var));
 
   /* Like in PR19590, scev can return a constant function.  */
   if (is_gimple_min_invariant (chrec))
     {
-      vr->set (chrec);
-      return;
+      *min = *max = chrec;
+      return true;
     }
 
   if (TREE_CODE (chrec) != POLYNOMIAL_CHREC)
-    return;
+    return false;
 
   init = initial_condition_in_loop_num (chrec, loop->num);
-  tem = op_with_constant_singleton_value_range (init);
-  if (tem)
-    init = tem;
   step = evolution_part_in_loop_num (chrec, loop->num);
-  tem = op_with_constant_singleton_value_range (step);
-  if (tem)
-    step = tem;
+
+  /* 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
@@ -1780,7 +1778,7 @@ 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;
+    return false;
 
   dir = scev_direction (chrec);
   if (/* Do not adjust ranges if we do not know whether the iv increases
@@ -1789,9 +1787,8 @@ 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;
+    return false;
 
-  type = TREE_TYPE (var);
   if (POINTER_TYPE_P (type) || !TYPE_MIN_VALUE (type))
     tmin = lower_bound_in_type (type, type);
   else
@@ -1806,7 +1803,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;
 
@@ -1829,21 +1826,29 @@ 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;
+		    return false;
 
 		  /* Check if init + nit * step overflows.  Though we checked
 		     scev {init, step}_loop doesn't wrap, it is not enough
@@ -1853,7 +1858,7 @@ 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;
+		    return false;
 
 		  tmin = maxvr.min ();
 		  tmax = maxvr.max ();
@@ -1862,66 +1867,62 @@ vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
 	}
     }
 
-  if (vr->varying_p () || vr->undefined_p ())
-    {
-      min = tmin;
-      max = tmax;
+  *min = tmin;
+  *max = tmax;
+  if (dir == EV_DIR_DECREASES)
+    *max = init;
+  else
+    *min = init;
 
-      /* For VARYING or UNDEFINED ranges, just about anything we get
-	 from scalar evolutions should be better.  */
+  /* 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);
 
-      if (dir == EV_DIR_DECREASES)
-	max = init;
-      else
-	min = init;
-    }
-  else if (vr->kind () == VR_RANGE)
-    {
-      min = vr->min ();
-      max = vr->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.  */
 
-      if (dir == EV_DIR_DECREASES)
+void
+vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
+				   gimple *stmt, tree var)
+{
+  tree min, max;
+  if (bounds_of_var_in_loop (&min, &max, this, loop, stmt, var))
+    {
+      if (vr->undefined_p () || vr->varying_p ())
 	{
-	  /* 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;
+	  /* 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 ();
 
-	  /* According to the loop information, the variable does not
-	     overflow.  */
-	  if (compare_values (min, tmin) == -1)
-	    min = tmin;
+	  /* ...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
+      else if (vr->kind () == VR_ANTI_RANGE)
 	{
-	  /* 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;
+	  /* ?? As an enhancement, if VR, MIN, and MAX are constants, one
+	     could just intersect VR with a range of [MIN,MAX].  */
 	}
     }
-  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;
-
-  /* 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);
-
-  vr->update (min, max);
 }
 
 /* Dump value ranges of all SSA_NAMEs to FILE.  */
@@ -4216,7 +4217,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..7051e13fc00 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);
@@ -177,4 +187,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 */

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

[Andrew MacLeod]

On 8/18/20 12:38 PM, Aldy Hernandez wrote:
> And here's the patch without the sanity check.
>
> Aldy

That diff was difficult to read.. I had to apply the patch to really 
follow it :-P

Anyway, yeah, this looks better.  effectively, you have
   1) left the input range "vr" range merging in adjust-range_with_scev,
   2) adjusted for the fact that the code extracted into 
"bounds_of_var_in_loop" now returns a min/max properly set, which 
sometimes  includes  basic symbolic expressions which the ranger can 
simply invoke range-ops on.
   3) the only functional difference now is that we still fully call 
bounds_of_var_in_loop when "vr" is an anti-range.... whereas before we 
bailed early.  But you added comments that we may be able to utilize 
that under some circumstances

this is OK.



>
> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
> index fe51a6faeb8..9b21441dff3 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,42 +1736,40 @@ 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.  */
>   
> -void
> -vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
> -				   gimple *stmt, tree var)
> +/* 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, min, max, type, tem;
> +  tree init, step, chrec, tmin, tmax, type = TREE_TYPE (var);
>     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;
> -
>     chrec = instantiate_parameters (loop, analyze_scalar_evolution (loop, var));
>   
>     /* Like in PR19590, scev can return a constant function.  */
>     if (is_gimple_min_invariant (chrec))
>       {
> -      vr->set (chrec);
> -      return;
> +      *min = *max = chrec;
> +      return true;
>       }
>   
>     if (TREE_CODE (chrec) != POLYNOMIAL_CHREC)
> -    return;
> +    return false;
>   
>     init = initial_condition_in_loop_num (chrec, loop->num);
> -  tem = op_with_constant_singleton_value_range (init);
> -  if (tem)
> -    init = tem;
>     step = evolution_part_in_loop_num (chrec, loop->num);
> -  tem = op_with_constant_singleton_value_range (step);
> -  if (tem)
> -    step = tem;
> +
> +  /* 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
> @@ -1780,7 +1778,7 @@ 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;
> +    return false;
>   
>     dir = scev_direction (chrec);
>     if (/* Do not adjust ranges if we do not know whether the iv increases
> @@ -1789,9 +1787,8 @@ 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;
> +    return false;
>   
> -  type = TREE_TYPE (var);
>     if (POINTER_TYPE_P (type) || !TYPE_MIN_VALUE (type))
>       tmin = lower_bound_in_type (type, type);
>     else
> @@ -1806,7 +1803,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;
>   
> @@ -1829,21 +1826,29 @@ 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;
> +		    return false;
>   
>   		  /* Check if init + nit * step overflows.  Though we checked
>   		     scev {init, step}_loop doesn't wrap, it is not enough
> @@ -1853,7 +1858,7 @@ 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;
> +		    return false;
>   
>   		  tmin = maxvr.min ();
>   		  tmax = maxvr.max ();
> @@ -1862,66 +1867,62 @@ vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
>   	}
>       }
>   
> -  if (vr->varying_p () || vr->undefined_p ())
> -    {
> -      min = tmin;
> -      max = tmax;
> +  *min = tmin;
> +  *max = tmax;
> +  if (dir == EV_DIR_DECREASES)
> +    *max = init;
> +  else
> +    *min = init;
>   
> -      /* For VARYING or UNDEFINED ranges, just about anything we get
> -	 from scalar evolutions should be better.  */
> +  /* 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);
>   
> -      if (dir == EV_DIR_DECREASES)
> -	max = init;
> -      else
> -	min = init;
> -    }
> -  else if (vr->kind () == VR_RANGE)
> -    {
> -      min = vr->min ();
> -      max = vr->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.  */
>   
> -      if (dir == EV_DIR_DECREASES)
> +void
> +vr_values::adjust_range_with_scev (value_range_equiv *vr, class loop *loop,
> +				   gimple *stmt, tree var)
> +{
> +  tree min, max;
> +  if (bounds_of_var_in_loop (&min, &max, this, loop, stmt, var))
> +    {
> +      if (vr->undefined_p () || vr->varying_p ())
>   	{
> -	  /* 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;
> +	  /* 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 ();
>   
> -	  /* According to the loop information, the variable does not
> -	     overflow.  */
> -	  if (compare_values (min, tmin) == -1)
> -	    min = tmin;
> +	  /* ...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
> +      else if (vr->kind () == VR_ANTI_RANGE)
>   	{
> -	  /* 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;
> +	  /* ?? As an enhancement, if VR, MIN, and MAX are constants, one
> +	     could just intersect VR with a range of [MIN,MAX].  */
>   	}
>       }
> -  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;
> -
> -  /* 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);
> -
> -  vr->update (min, max);
>   }
>   
>   /* Dump value ranges of all SSA_NAMEs to FILE.  */
> @@ -4216,7 +4217,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..7051e13fc00 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);
> @@ -177,4 +187,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 *,