This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[range-ops] patch 02/04: enforce canonicalization in value_range


As discussed prior. This enforces canonicalization at creation time, which makes things a lot more consistent throughout.

Since now [MIN, MAX] will be canonicalized into VR_VARYING, we can no longer depend on normalizing VARYING's into [MIN, MAX] for more general handling. I've adjusted the few places where we were doing this so that they work correctly.

As a bonus, now that we're canonicalized, I've tweaked singleton to work with anti-ranges.

Note: There is a small note in ranges_from_anti_range, which I will start making heavier use of in subsequent patches:

+  /* ?? This function is called multiple times from num_pairs,
+     lower_bound, and upper_bound.  We should probably memoize this, or
+     rewrite the callers in such a way that we're not re-calculating
+     this constantly.  */

I don't think this is needed now, but just a note of things to come. It may not be an issue, but just something to keep in mind.

Tested on x86-64 Linux with --enable-languages=all.

Aldy
commit d220a3eeb77615b39260e532e34815a3810e8348
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Fri Jun 28 11:15:03 2019 +0200

    Enforce canonicalization in value_range.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index a5247735694..866200d9594 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,20 @@
+2019-07-01  Aldy Hernandez  <aldyh@redhat.com>
+
+	* tree-vrp.c (value_range_base::set_and_canonicalize): Rename to
+	set() and add normalization of [MIN, MAX] into varying.
+	(value_range_base::singleton_p): Make work with anti ranges.
+	(ranges_from_anti_range): Handle pointers.
+	(extract_range_into_wide_ints): Call normalize_symbolics.
+	(extract_range_from_binary_expr): Do not build a [MIN,MAX] range
+	because it will be canonicalized into varying.
+	Call set() instead of set_and_canonicalize().
+	(extract_range_from_unary_expr): Call set() instead of
+	set_and_canonicalize().
+	(intersect_helper): Do not call set_and_canonicalize.
+	* tree-vrp.h (value_range_base): Remove set_and_canonicalize.
+	(value_range): Same.
+	* vr-values.c (extract_range_from_var_from_comparison_expr): Same.
+
 2019-07-01  Aldy Hernandez  <aldyh@redhat.com>
 
 	* gimple-ssa-evrp-analyze.c (record_ranges_from_phis): Skip PHIs
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 97046c22ed1..f78517b5b3b 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -69,20 +69,15 @@ along with GCC; see the file COPYING3.  If not see
 #include "builtins.h"
 #include "wide-int-range.h"
 
+static bool
+ranges_from_anti_range (const value_range_base *ar,
+			value_range_base *vr0, value_range_base *vr1,
+			bool handle_pointers = false);
+
 /* Set of SSA names found live during the RPO traversal of the function
    for still active basic-blocks.  */
 static sbitmap *live;
 
-void
-value_range_base::set (enum value_range_kind kind, tree min, tree max)
-{
-  m_kind = kind;
-  m_min = min;
-  m_max = max;
-  if (flag_checking)
-    check ();
-}
-
 void
 value_range::set_equiv (bitmap equiv)
 {
@@ -363,6 +358,24 @@ value_range::equiv_add (const_tree var,
 bool
 value_range_base::singleton_p (tree *result) const
 {
+  if (m_kind == VR_ANTI_RANGE)
+    {
+      if (nonzero_p ())
+	{
+	  if (TYPE_PRECISION (type ()) == 1)
+	    {
+	      if (result)
+		*result = m_max;
+	      return true;
+	    }
+	  return false;
+	}
+
+      value_range_base vr0, vr1;
+      return (ranges_from_anti_range (this, &vr0, &vr1, true)
+	      && vr1.undefined_p ()
+	      && vr0.singleton_p (result));
+    }
   if (m_kind == VR_RANGE
       && vrp_operand_equal_p (min (), max ())
       && is_gimple_min_invariant (min ()))
@@ -690,8 +703,7 @@ intersect_range_with_nonzero_bits (enum value_range_kind vr_type,
    extract ranges from var + CST op limit.  */
 
 void
-value_range_base::set_and_canonicalize (enum value_range_kind kind,
-					tree min, tree max)
+value_range_base::set (enum value_range_kind kind, tree min, tree max)
 {
   if (kind == VR_UNDEFINED)
     {
@@ -711,7 +723,9 @@ value_range_base::set_and_canonicalize (enum value_range_kind kind,
   if (TREE_CODE (min) != INTEGER_CST
       || TREE_CODE (max) != INTEGER_CST)
     {
-      set (kind, min, max);
+      m_kind = kind;
+      m_min = min;
+      m_max = max;
       return;
     }
 
@@ -747,12 +761,13 @@ value_range_base::set_and_canonicalize (enum value_range_kind kind,
       kind = kind == VR_RANGE ? VR_ANTI_RANGE : VR_RANGE;
     }
 
+  tree type = TREE_TYPE (min);
+
   /* Anti-ranges that can be represented as ranges should be so.  */
   if (kind == VR_ANTI_RANGE)
     {
       /* For -fstrict-enums we may receive out-of-range ranges so consider
          values < -INF and values > INF as -INF/INF as well.  */
-      tree type = TREE_TYPE (min);
       bool is_min = (INTEGRAL_TYPE_P (type)
 		     && tree_int_cst_compare (min, TYPE_MIN_VALUE (type)) <= 0);
       bool is_max = (INTEGRAL_TYPE_P (type)
@@ -795,22 +810,37 @@ value_range_base::set_and_canonicalize (enum value_range_kind kind,
         }
     }
 
+  /* Normalize [MIN, MAX] into VARYING and ~[MIN, MAX] into UNDEFINED.
+
+     Avoid using TYPE_{MIN,MAX}_VALUE because -fstrict-enums can
+     restrict those to a subset of what actually fits in the type.
+     Instead use the extremes of the type precision which will allow
+     compare_range_with_value() to check if a value is inside a range,
+     whereas if we used TYPE_*_VAL, said function would just punt
+     upon seeing a VARYING.  */
+  unsigned prec = TYPE_PRECISION (type);
+  signop sign = TYPE_SIGN (type);
+  if (wi::eq_p (wi::to_wide (min), wi::min_value (prec, sign))
+      && wi::eq_p (wi::to_wide (max), wi::max_value (prec, sign)))
+    {
+      if (kind == VR_RANGE)
+	set_varying (type);
+      else if (kind == VR_ANTI_RANGE)
+	set_undefined (type);
+      else
+	gcc_unreachable ();
+      return;
+    }
+
   /* Do not drop [-INF(OVF), +INF(OVF)] to varying.  (OVF) has to be sticky
      to make sure VRP iteration terminates, otherwise we can get into
      oscillations.  */
 
-  set (kind, min, max);
-}
-
-void
-value_range::set_and_canonicalize (enum value_range_kind kind,
-				   tree min, tree max, bitmap equiv)
-{
-  value_range_base::set_and_canonicalize (kind, min, max);
-  if (this->kind () == VR_RANGE || this->kind () == VR_ANTI_RANGE)
-    set_equiv (equiv);
-  else
-    equiv_clear ();
+  m_kind = kind;
+  m_min = min;
+  m_max = max;
+  if (flag_checking)
+    check ();
 }
 
 void
@@ -1240,8 +1270,14 @@ vrp_set_zero_nonzero_bits (const tree expr_type,
 
 static bool
 ranges_from_anti_range (const value_range_base *ar,
-			value_range_base *vr0, value_range_base *vr1)
+			value_range_base *vr0, value_range_base *vr1,
+			bool handle_pointers)
 {
+  /* ?? This function is called multiple times from num_pairs,
+     lower_bound, and upper_bound.  We should probably memoize this, or
+     rewrite the callers in such a way that we're not re-calculating
+     this constantly.  */
+
   tree type = ar->type ();
 
   vr0->set_undefined (type);
@@ -1253,18 +1289,18 @@ ranges_from_anti_range (const value_range_base *ar,
   if (ar->kind () != VR_ANTI_RANGE
       || TREE_CODE (ar->min ()) != INTEGER_CST
       || TREE_CODE (ar->max ()) != INTEGER_CST
-      || !vrp_val_min (type)
-      || !vrp_val_max (type))
+      || !vrp_val_min (type, handle_pointers)
+      || !vrp_val_max (type, handle_pointers))
     return false;
 
-  if (tree_int_cst_lt (vrp_val_min (type), ar->min ()))
+  if (tree_int_cst_lt (vrp_val_min (type, handle_pointers), ar->min ()))
     vr0->set (VR_RANGE,
-	      vrp_val_min (type),
+	      vrp_val_min (type, handle_pointers),
 	      wide_int_to_tree (type, wi::to_wide (ar->min ()) - 1));
-  if (tree_int_cst_lt (ar->max (), vrp_val_max (type)))
+  if (tree_int_cst_lt (ar->max (), vrp_val_max (type, handle_pointers)))
     vr1->set (VR_RANGE,
 	      wide_int_to_tree (type, wi::to_wide (ar->max ()) + 1),
-	      vrp_val_max (type));
+	      vrp_val_max (type, handle_pointers));
   if (vr0->undefined_p ())
     {
       *vr0 = *vr1;
@@ -1275,21 +1311,19 @@ ranges_from_anti_range (const value_range_base *ar,
 }
 
 /* Extract the components of a value range into a pair of wide ints in
-   [WMIN, WMAX].
-
-   If the value range is anything but a VR_*RANGE of constants, the
-   resulting wide ints are set to [-MIN, +MAX] for the type.  */
+   [WMIN, WMAX], after having normalized any symbolics from the input.  */
 
 static void inline
-extract_range_into_wide_ints (const value_range_base *vr,
+extract_range_into_wide_ints (const value_range_base *vr_,
 			      signop sign, unsigned prec,
 			      wide_int &wmin, wide_int &wmax)
 {
-  gcc_assert (vr->kind () != VR_ANTI_RANGE || vr->symbolic_p ());
-  if (range_int_cst_p (vr))
+  gcc_assert (vr_->kind () != VR_ANTI_RANGE || vr_->symbolic_p ());
+  value_range vr = vr_->normalize_symbolics ();
+  if (range_int_cst_p (&vr))
     {
-      wmin = wi::to_wide (vr->min ());
-      wmax = wi::to_wide (vr->max ());
+      wmin = wi::to_wide (vr.min ());
+      wmax = wi::to_wide (vr.max ());
     }
   else
     {
@@ -1351,9 +1385,8 @@ extract_range_from_multiplicative_op (value_range_base *vr,
 					code, TYPE_SIGN (type), prec,
 					vr0_lb, vr0_ub, vr1_lb, vr1_ub,
 					overflow_undefined))
-    vr->set_and_canonicalize (VR_RANGE,
-			      wide_int_to_tree (type, res_lb),
-			      wide_int_to_tree (type, res_ub));
+    vr->set (VR_RANGE, wide_int_to_tree (type, res_lb),
+	     wide_int_to_tree (type, res_ub));
   else
     vr->set_varying (type);
 }
@@ -1747,19 +1780,30 @@ extract_range_from_binary_expr (value_range_base *vr,
      range and see what we end up with.  */
   if (code == PLUS_EXPR || code == MINUS_EXPR)
     {
+      value_range_kind vr0_kind = vr0.kind (), vr1_kind = vr1.kind ();
+      tree vr0_min = vr0.min (), vr0_max = vr0.max ();
+      tree vr1_min = vr1.min (), vr1_max = vr1.max ();
       /* This will normalize things such that calculating
 	 [0,0] - VR_VARYING is not dropped to varying, but is
 	 calculated as [MIN+1, MAX].  */
       if (vr0.varying_p ())
-	vr0.set (VR_RANGE, vrp_val_min (expr_type), vrp_val_max (expr_type));
+	{
+	  vr0_kind = VR_RANGE;
+	  vr0_min = vrp_val_min (expr_type);
+	  vr0_max = vrp_val_max (expr_type);
+	}
       if (vr1.varying_p ())
-	vr1.set (VR_RANGE, vrp_val_min (expr_type), vrp_val_max (expr_type));
+	{
+	  vr1_kind = VR_RANGE;
+	  vr1_min = vrp_val_min (expr_type);
+	  vr1_max = vrp_val_max (expr_type);
+	}
 
       const bool minus_p = (code == MINUS_EXPR);
-      tree min_op0 = vr0.min ();
-      tree min_op1 = minus_p ? vr1.max () : vr1.min ();
-      tree max_op0 = vr0.max ();
-      tree max_op1 = minus_p ? vr1.min () : vr1.max ();
+      tree min_op0 = vr0_min;
+      tree min_op1 = minus_p ? vr1_max : vr1_min;
+      tree max_op0 = vr0_max;
+      tree max_op1 = minus_p ? vr1_min : vr1_max;
       tree sym_min_op0 = NULL_TREE;
       tree sym_min_op1 = NULL_TREE;
       tree sym_max_op0 = NULL_TREE;
@@ -1772,7 +1816,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 	 single-symbolic ranges, try to compute the precise resulting range,
 	 but only if we know that this resulting range will also be constant
 	 or single-symbolic.  */
-      if (vr0.kind () == VR_RANGE && vr1.kind () == VR_RANGE
+      if (vr0_kind == VR_RANGE && vr1_kind == VR_RANGE
 	  && (TREE_CODE (min_op0) == INTEGER_CST
 	      || (sym_min_op0
 		  = get_single_symbol (min_op0, &neg_min_op0, &min_op0)))
@@ -1902,7 +1946,7 @@ extract_range_from_binary_expr (value_range_base *vr,
 		{
 		  min = wide_int_to_tree (expr_type, res_lb);
 		  max = wide_int_to_tree (expr_type, res_ub);
-		  vr->set_and_canonicalize (VR_RANGE, min, max);
+		  vr->set (VR_RANGE, min, max);
 		  return;
 		}
 	    }
@@ -2200,7 +2244,7 @@ extract_range_from_unary_expr (value_range_base *vr,
 	{
 	  tree min = wide_int_to_tree (outer_type, wmin);
 	  tree max = wide_int_to_tree (outer_type, wmax);
-	  vr->set_and_canonicalize (VR_RANGE, min, max);
+	  vr->set (VR_RANGE, min, max);
 	}
       else
 	vr->set_varying (outer_type);
@@ -6107,7 +6151,12 @@ value_range_base::intersect_helper (const value_range_base *vr0,
      VR_RANGE can still be a VR_RANGE.  Work on a temporary so we can
      fall back to vr0 when this turns things to varying.  */
   value_range_base tem;
-  tem.set_and_canonicalize (vr0type, vr0min, vr0max);
+  if (vr0type == VR_UNDEFINED)
+    tem.set_undefined (TREE_TYPE (vr0->min ()));
+  else if (vr0type == VR_VARYING)
+    tem.set_varying (TREE_TYPE (vr0->min ()));
+  else
+    tem.set (vr0type, vr0min, vr0max);
   /* If that failed, use the saved original VR0.  */
   if (tem.varying_p ())
     return *vr0;
@@ -6217,7 +6266,7 @@ value_range_base::union_helper (const value_range_base *vr0,
   else if (vr0type == VR_VARYING)
     tem.set_varying (TREE_TYPE (vr0->min ()));
   else
-    tem.set_and_canonicalize (vr0type, vr0min, vr0max);
+    tem.set (vr0type, vr0min, vr0max);
 
   /* Failed to find an efficient meet.  Before giving up and setting
      the result to VARYING, see if we can at least derive a useful
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
index 5baabfd8b8d..3511cfaf0fc 100644
--- a/gcc/tree-vrp.h
+++ b/gcc/tree-vrp.h
@@ -72,7 +72,6 @@ public:
   /* Misc methods.  */
   tree type () const;
   bool may_contain_p (tree) const;
-  void set_and_canonicalize (enum value_range_kind, tree, tree);
   bool zero_p () const;
   bool nonzero_p () const;
   bool singleton_p (tree *result = NULL) const;
@@ -148,7 +147,6 @@ class GTY((user)) value_range : public value_range_base
 
   /* Misc methods.  */
   void deep_copy (const value_range *);
-  void set_and_canonicalize (enum value_range_kind, tree, tree, bitmap = NULL);
   void dump (FILE *) const;
   void dump () const;
 
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 6569af391ed..421d05d9017 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -565,9 +565,9 @@ vr_values::extract_range_for_var_from_comparison_expr (tree var,
          vice-versa.  Use set_and_canonicalize which does this for
          us.  */
       if (cond_code == LE_EXPR)
-        vr_p->set_and_canonicalize (VR_RANGE, min, max, vr_p->equiv ());
+        vr_p->set (VR_RANGE, min, max, vr_p->equiv ());
       else if (cond_code == GT_EXPR)
-        vr_p->set_and_canonicalize (VR_ANTI_RANGE, min, max, vr_p->equiv ());
+        vr_p->set (VR_ANTI_RANGE, min, max, vr_p->equiv ());
       else
 	gcc_unreachable ();
     }
@@ -639,7 +639,7 @@ vr_values::extract_range_for_var_from_comparison_expr (tree var,
 	  && vrp_val_is_max (max))
 	min = max = limit;
 
-      vr_p->set_and_canonicalize (VR_ANTI_RANGE, min, max, vr_p->equiv ());
+      vr_p->set (VR_ANTI_RANGE, min, max, vr_p->equiv ());
     }
   else if (cond_code == LE_EXPR || cond_code == LT_EXPR)
     {

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]