public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PUSHED 0/8] irange API adjustments for various files
@ 2020-08-04  6:34 Aldy Hernandez
  2020-08-04  6:34 ` [PUSHED 1/8] Remove ad-hoc range canonicalization from determine_block_size Aldy Hernandez
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Aldy Hernandez @ 2020-08-04  6:34 UTC (permalink / raw)
  To: gcc-patches

The following patchset adjusts various files for adherence to the
irange API.  There are no functional changes.  The changes are mostly
obvious and will be pushed pending tests.

No effort was made to improve the existing code.  That is, I didn't
bend over backwards to convert the code to multi-ranges unless it
was trivial.

There are still plenty of uses of min(), max(), kind(), etc.  I will
be converting those as time permits, but I make no promises that I'll
be able to get to all of them.

Aldy



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PUSHED 1/8] Remove ad-hoc range canonicalization from determine_block_size.
  2020-08-04  6:34 [PUSHED 0/8] irange API adjustments for various files Aldy Hernandez
@ 2020-08-04  6:34 ` Aldy Hernandez
  2020-08-04  6:34 ` [PUSHED 2/8] Adjust expr_not_equal_to to use irange API Aldy Hernandez
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Aldy Hernandez @ 2020-08-04  6:34 UTC (permalink / raw)
  To: gcc-patches

Anti ranges of ~[MIN,X] are automatically canonicalized to [X+1,MAX],
at creation time.  There is no need to handle them specially.

Tested by adding a gcc_unreachable and bootstrapping/testing.

gcc/ChangeLog:

	* builtins.c (determine_block_size): Remove ad-hoc range canonicalization.
---
 gcc/builtins.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 228db78f32b..beb56e06d8a 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -3287,12 +3287,6 @@ determine_block_size (tree len, rtx len_rtx,
 	}
       else if (range_type == VR_ANTI_RANGE)
 	{
-	  /* Anti range 0...N lets us to determine minimal size to N+1.  */
-	  if (min == 0)
-	    {
-	      if (wi::fits_uhwi_p (max) && max.to_uhwi () + 1 != 0)
-		*min_size = max.to_uhwi () + 1;
-	    }
 	  /* Code like
 
 	     int n;
@@ -3302,7 +3296,7 @@ determine_block_size (tree len, rtx len_rtx,
 	     Produce anti range allowing negative values of N.  We still
 	     can use the information and make a guess that N is not negative.
 	     */
-	  else if (!wi::leu_p (max, 1 << 30) && wi::fits_uhwi_p (min))
+	  if (!wi::leu_p (max, 1 << 30) && wi::fits_uhwi_p (min))
 	    *probable_max_size = min.to_uhwi () - 1;
 	}
     }
-- 
2.26.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PUSHED 2/8] Adjust expr_not_equal_to to use irange API.
  2020-08-04  6:34 [PUSHED 0/8] irange API adjustments for various files Aldy Hernandez
  2020-08-04  6:34 ` [PUSHED 1/8] Remove ad-hoc range canonicalization from determine_block_size Aldy Hernandez
@ 2020-08-04  6:34 ` Aldy Hernandez
  2020-08-04  6:52   ` Richard Biener
  2020-08-04  6:34 ` [PUSHED 3/8] Adjust get_range_info to use the base irange class Aldy Hernandez
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Aldy Hernandez @ 2020-08-04  6:34 UTC (permalink / raw)
  To: gcc-patches

gcc/ChangeLog:

	* fold-const.c (expr_not_equal_to): Adjust for irange API.
---
 gcc/fold-const.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 1324a194995..5d27927f6bf 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -10194,8 +10194,7 @@ tree_expr_nonzero_p (tree t)
 bool
 expr_not_equal_to (tree t, const wide_int &w)
 {
-  wide_int min, max, nz;
-  value_range_kind rtype;
+  value_range vr;
   switch (TREE_CODE (t))
     {
     case INTEGER_CST:
@@ -10204,17 +10203,9 @@ expr_not_equal_to (tree t, const wide_int &w)
     case SSA_NAME:
       if (!INTEGRAL_TYPE_P (TREE_TYPE (t)))
 	return false;
-      rtype = get_range_info (t, &min, &max);
-      if (rtype == VR_RANGE)
-	{
-	  if (wi::lt_p (max, w, TYPE_SIGN (TREE_TYPE (t))))
-	    return true;
-	  if (wi::lt_p (w, min, TYPE_SIGN (TREE_TYPE (t))))
-	    return true;
-	}
-      else if (rtype == VR_ANTI_RANGE
-	       && wi::le_p (min, w, TYPE_SIGN (TREE_TYPE (t)))
-	       && wi::le_p (w, max, TYPE_SIGN (TREE_TYPE (t))))
+      get_range_info (t, vr);
+      if (!vr.undefined_p ()
+	  && !vr.contains_p (wide_int_to_tree (TREE_TYPE (t), w)))
 	return true;
       /* If T has some known zero bits and W has any of those bits set,
 	 then T is known not to be equal to W.  */
-- 
2.26.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PUSHED 3/8] Adjust get_range_info to use the base irange class.
  2020-08-04  6:34 [PUSHED 0/8] irange API adjustments for various files Aldy Hernandez
  2020-08-04  6:34 ` [PUSHED 1/8] Remove ad-hoc range canonicalization from determine_block_size Aldy Hernandez
  2020-08-04  6:34 ` [PUSHED 2/8] Adjust expr_not_equal_to to use irange API Aldy Hernandez
@ 2020-08-04  6:34 ` Aldy Hernandez
  2020-08-04  6:34 ` [PUSHED 4/8] Adjust op_with_boolean_value_range_p for irange API Aldy Hernandez
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Aldy Hernandez @ 2020-08-04  6:34 UTC (permalink / raw)
  To: gcc-patches

gcc/ChangeLog:

	* tree-ssanames.c (get_range_info): Use irange instead of value_range.
	* tree-ssanames.h (get_range_info): Same.
---
 gcc/tree-ssanames.c | 2 +-
 gcc/tree-ssanames.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index c143a93b072..d9432657e30 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -441,7 +441,7 @@ get_range_info (const_tree name, wide_int *min, wide_int *max)
    in a value_range VR.  Returns the value_range_kind.  */
 
 enum value_range_kind
-get_range_info (const_tree name, value_range &vr)
+get_range_info (const_tree name, irange &vr)
 {
   tree min, max;
   wide_int wmin, wmax;
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index 47df4e40062..3395824e425 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -73,7 +73,7 @@ extern void set_range_info (tree, const value_range &);
 /* Gets the value range from SSA.  */
 extern enum value_range_kind get_range_info (const_tree, wide_int *,
 					     wide_int *);
-extern enum value_range_kind get_range_info (const_tree, value_range &);
+extern enum value_range_kind get_range_info (const_tree, irange &);
 extern void set_nonzero_bits (tree, const wide_int_ref &);
 extern wide_int get_nonzero_bits (const_tree);
 extern bool ssa_name_has_boolean_range (tree);
-- 
2.26.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PUSHED 4/8] Adjust op_with_boolean_value_range_p for irange API.
  2020-08-04  6:34 [PUSHED 0/8] irange API adjustments for various files Aldy Hernandez
                   ` (2 preceding siblings ...)
  2020-08-04  6:34 ` [PUSHED 3/8] Adjust get_range_info to use the base irange class Aldy Hernandez
@ 2020-08-04  6:34 ` Aldy Hernandez
  2020-08-04  6:55   ` Richard Biener
  2020-08-04  6:34 ` [PUSHED 5/8] Adjust vrp_evaluate_conditional " Aldy Hernandez
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Aldy Hernandez @ 2020-08-04  6:34 UTC (permalink / raw)
  To: gcc-patches

It seems to me that we should also check for [0,0] and [1,1] in the
range, but I am leaving things as is to avoid functional changes.

gcc/ChangeLog:

	* vr-values.c (simplify_using_ranges::op_with_boolean_value_range_p): Adjust
	for irange API.
---
 gcc/vr-values.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 609375c072e..1190fa96453 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -448,10 +448,11 @@ simplify_using_ranges::op_with_boolean_value_range_p (tree op)
   if (TREE_CODE (op) != SSA_NAME)
     return false;
 
+  /* ?? Errr, this should probably check for [0,0] and [1,1] as well
+     as [0,1].  */
   const value_range *vr = get_value_range (op);
-  return (vr->kind () == VR_RANGE
-	  && integer_zerop (vr->min ())
-	  && integer_onep (vr->max ()));
+  return *vr == value_range (build_zero_cst (TREE_TYPE (op)),
+			     build_one_cst (TREE_TYPE (op)));
 }
 
 /* Extract value range information for VAR when (OP COND_CODE LIMIT) is
-- 
2.26.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PUSHED 5/8] Adjust vrp_evaluate_conditional for irange API.
  2020-08-04  6:34 [PUSHED 0/8] irange API adjustments for various files Aldy Hernandez
                   ` (3 preceding siblings ...)
  2020-08-04  6:34 ` [PUSHED 4/8] Adjust op_with_boolean_value_range_p for irange API Aldy Hernandez
@ 2020-08-04  6:34 ` Aldy Hernandez
  2020-08-04  6:55   ` Richard Biener
  2020-08-04  6:34 ` [PUSHED 6/8] Use irange API in test_for_singularity Aldy Hernandez
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Aldy Hernandez @ 2020-08-04  6:34 UTC (permalink / raw)
  To: gcc-patches

VR_RANGE of [-INF,+INF] is canonicalized to VARYING at creation.
That is why the test now becomes varying_p().

gcc/ChangeLog:

	* vr-values.c (simplify_using_ranges::vrp_evaluate_conditional): Adjust
	for irange API.
---
 gcc/vr-values.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 1190fa96453..90ba8fca246 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -2495,11 +2495,7 @@ simplify_using_ranges::vrp_evaluate_conditional (tree_code code, tree op0,
       tree type = TREE_TYPE (op0);
       const value_range_equiv *vr0 = get_value_range (op0);
 
-      if (vr0->kind () == VR_RANGE
-	  && INTEGRAL_TYPE_P (type)
-	  && vrp_val_is_min (vr0->min ())
-	  && vrp_val_is_max (vr0->max ())
-	  && is_gimple_min_invariant (op1))
+      if (vr0->varying_p () && INTEGRAL_TYPE_P (type))
 	{
 	  location_t location;
 
-- 
2.26.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PUSHED 6/8] Use irange API in test_for_singularity.
  2020-08-04  6:34 [PUSHED 0/8] irange API adjustments for various files Aldy Hernandez
                   ` (4 preceding siblings ...)
  2020-08-04  6:34 ` [PUSHED 5/8] Adjust vrp_evaluate_conditional " Aldy Hernandez
@ 2020-08-04  6:34 ` Aldy Hernandez
  2020-08-04  6:58   ` Richard Biener
  2020-08-04  6:34 ` [PUSHED 7/8] Adjust simplify_conversion_using_ranges for irange API Aldy Hernandez
  2020-08-04  6:34 ` [PUSHED 8/8] Adjust two_valued_val_range_p " Aldy Hernandez
  7 siblings, 1 reply; 17+ messages in thread
From: Aldy Hernandez @ 2020-08-04  6:34 UTC (permalink / raw)
  To: gcc-patches

gcc/ChangeLog:

	* vr-values.c (test_for_singularity): Use irange API.
	(simplify_using_ranges::simplify_cond_using_ranges_1): Do not
	special case VR_RANGE.
---
 gcc/vr-values.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 90ba8fca246..e78b25596b0 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -3480,10 +3480,13 @@ test_for_singularity (enum tree_code cond_code, tree op0,
      value range information we have for op0.  */
   if (min && max)
     {
-      if (compare_values (vr->min (), min) == 1)
-	min = vr->min ();
-      if (compare_values (vr->max (), max) == -1)
-	max = vr->max ();
+      tree type = TREE_TYPE (op0);
+      tree tmin = wide_int_to_tree (type, vr->lower_bound ());
+      tree tmax = wide_int_to_tree (type, vr->upper_bound ());
+      if (compare_values (tmin, min) == 1)
+	min = tmin;
+      if (compare_values (tmax, max) == -1)
+	max = tmax;
 
       /* If the new min/max values have converged to a single value,
 	 then there is only one value which can satisfy the condition,
@@ -3594,7 +3597,7 @@ simplify_using_ranges::simplify_cond_using_ranges_1 (gcond *stmt)
 
       /* If we have range information for OP0, then we might be
 	 able to simplify this conditional. */
-      if (vr->kind () == VR_RANGE)
+      if (!vr->undefined_p () && !vr->varying_p ())
 	{
 	  tree new_tree = test_for_singularity (cond_code, op0, op1, vr);
 	  if (new_tree)
-- 
2.26.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PUSHED 7/8] Adjust simplify_conversion_using_ranges for irange API.
  2020-08-04  6:34 [PUSHED 0/8] irange API adjustments for various files Aldy Hernandez
                   ` (5 preceding siblings ...)
  2020-08-04  6:34 ` [PUSHED 6/8] Use irange API in test_for_singularity Aldy Hernandez
@ 2020-08-04  6:34 ` Aldy Hernandez
  2020-08-04  6:34 ` [PUSHED 8/8] Adjust two_valued_val_range_p " Aldy Hernandez
  7 siblings, 0 replies; 17+ messages in thread
From: Aldy Hernandez @ 2020-08-04  6:34 UTC (permalink / raw)
  To: gcc-patches

gcc/ChangeLog:

	* vr-values.c (simplify_conversion_using_ranges): Convert to irange API.
---
 gcc/vr-values.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index e78b25596b0..38c9a657dad 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -3969,11 +3969,14 @@ simplify_conversion_using_ranges (gimple_stmt_iterator *gsi, gimple *stmt)
   /* Get the value-range of the inner operand.  Use get_range_info in
      case innerop was created during substitute-and-fold.  */
   wide_int imin, imax;
-  if (!INTEGRAL_TYPE_P (TREE_TYPE (innerop))
-      || get_range_info (innerop, &imin, &imax) != VR_RANGE)
+  value_range vr;
+  if (!INTEGRAL_TYPE_P (TREE_TYPE (innerop)))
     return false;
-  innermin = widest_int::from (imin, TYPE_SIGN (TREE_TYPE (innerop)));
-  innermax = widest_int::from (imax, TYPE_SIGN (TREE_TYPE (innerop)));
+  get_range_info (innerop, vr);
+  if (vr.undefined_p () || vr.varying_p ())
+    return false;
+  innermin = widest_int::from (vr.lower_bound (), TYPE_SIGN (TREE_TYPE (innerop)));
+  innermax = widest_int::from (vr.upper_bound (), TYPE_SIGN (TREE_TYPE (innerop)));
 
   /* Simulate the conversion chain to check if the result is equal if
      the middle conversion is removed.  */
-- 
2.26.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PUSHED 8/8] Adjust two_valued_val_range_p for irange API.
  2020-08-04  6:34 [PUSHED 0/8] irange API adjustments for various files Aldy Hernandez
                   ` (6 preceding siblings ...)
  2020-08-04  6:34 ` [PUSHED 7/8] Adjust simplify_conversion_using_ranges for irange API Aldy Hernandez
@ 2020-08-04  6:34 ` Aldy Hernandez
  7 siblings, 0 replies; 17+ messages in thread
From: Aldy Hernandez @ 2020-08-04  6:34 UTC (permalink / raw)
  To: gcc-patches

gcc/ChangeLog:

	* vr-values.c (simplify_using_ranges::two_valued_val_range_p):
	Use irange API.
---
 gcc/vr-values.c | 31 +++++++++----------------------
 1 file changed, 9 insertions(+), 22 deletions(-)

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 38c9a657dad..2fd4956a2e4 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -4194,33 +4194,20 @@ simplify_using_ranges::simplify_internal_call_using_ranges
 bool
 simplify_using_ranges::two_valued_val_range_p (tree var, tree *a, tree *b)
 {
-  const value_range *vr = get_value_range (var);
-  if (vr->varying_p ()
-      || vr->undefined_p ()
-      || TREE_CODE (vr->min ()) != INTEGER_CST
-      || TREE_CODE (vr->max ()) != INTEGER_CST)
+  value_range vr = *get_value_range (var);
+  vr.normalize_symbolics ();
+  if (vr.varying_p () || vr.undefined_p ())
     return false;
 
-  if (vr->kind () == VR_RANGE
-      && wi::to_wide (vr->max ()) - wi::to_wide (vr->min ()) == 1)
-    {
-      *a = vr->min ();
-      *b = vr->max ();
-      return true;
-    }
-
-  /* ~[TYPE_MIN + 1, TYPE_MAX - 1] */
-  if (vr->kind () == VR_ANTI_RANGE
-      && (wi::to_wide (vr->min ())
-	  - wi::to_wide (vrp_val_min (TREE_TYPE (var)))) == 1
-      && (wi::to_wide (vrp_val_max (TREE_TYPE (var)))
-	  - wi::to_wide (vr->max ())) == 1)
+  if ((vr.num_pairs () == 1 && vr.upper_bound () - vr.lower_bound () == 1)
+      || (vr.num_pairs () == 2
+	  && vr.lower_bound (0) == vr.upper_bound (0)
+	  && vr.lower_bound (1) == vr.upper_bound (1)))
     {
-      *a = vrp_val_min (TREE_TYPE (var));
-      *b = vrp_val_max (TREE_TYPE (var));
+      *a = wide_int_to_tree (TREE_TYPE (var), vr.lower_bound ());
+      *b = wide_int_to_tree (TREE_TYPE (var), vr.upper_bound ());
       return true;
     }
-
   return false;
 }
 
-- 
2.26.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PUSHED 2/8] Adjust expr_not_equal_to to use irange API.
  2020-08-04  6:34 ` [PUSHED 2/8] Adjust expr_not_equal_to to use irange API Aldy Hernandez
@ 2020-08-04  6:52   ` Richard Biener
  2020-08-04  9:16     ` Aldy Hernandez
  0 siblings, 1 reply; 17+ messages in thread
From: Richard Biener @ 2020-08-04  6:52 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: GCC Patches

On Tue, Aug 4, 2020 at 8:37 AM Aldy Hernandez via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> gcc/ChangeLog:
>
>         * fold-const.c (expr_not_equal_to): Adjust for irange API.
> ---
>  gcc/fold-const.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> index 1324a194995..5d27927f6bf 100644
> --- a/gcc/fold-const.c
> +++ b/gcc/fold-const.c
> @@ -10194,8 +10194,7 @@ tree_expr_nonzero_p (tree t)
>  bool
>  expr_not_equal_to (tree t, const wide_int &w)
>  {
> -  wide_int min, max, nz;
> -  value_range_kind rtype;
> +  value_range vr;
>    switch (TREE_CODE (t))
>      {
>      case INTEGER_CST:
> @@ -10204,17 +10203,9 @@ expr_not_equal_to (tree t, const wide_int &w)
>      case SSA_NAME:
>        if (!INTEGRAL_TYPE_P (TREE_TYPE (t)))
>         return false;
> -      rtype = get_range_info (t, &min, &max);
> -      if (rtype == VR_RANGE)
> -       {
> -         if (wi::lt_p (max, w, TYPE_SIGN (TREE_TYPE (t))))
> -           return true;
> -         if (wi::lt_p (w, min, TYPE_SIGN (TREE_TYPE (t))))
> -           return true;
> -       }
> -      else if (rtype == VR_ANTI_RANGE
> -              && wi::le_p (min, w, TYPE_SIGN (TREE_TYPE (t)))
> -              && wi::le_p (w, max, TYPE_SIGN (TREE_TYPE (t))))
> +      get_range_info (t, vr);

Ick.  Do we now use references for out parameters?  I find this
highly non-obvious semantics.  What's wrong with

     vr = get_range_info (t);

if you dislike get_range_info (t, &vr)?

Please reconsider.

Richard.

> +      if (!vr.undefined_p ()
> +         && !vr.contains_p (wide_int_to_tree (TREE_TYPE (t), w)))
>         return true;
>        /* If T has some known zero bits and W has any of those bits set,
>          then T is known not to be equal to W.  */
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PUSHED 4/8] Adjust op_with_boolean_value_range_p for irange API.
  2020-08-04  6:34 ` [PUSHED 4/8] Adjust op_with_boolean_value_range_p for irange API Aldy Hernandez
@ 2020-08-04  6:55   ` Richard Biener
  2020-08-04  9:45     ` Aldy Hernandez
  0 siblings, 1 reply; 17+ messages in thread
From: Richard Biener @ 2020-08-04  6:55 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: GCC Patches

On Tue, Aug 4, 2020 at 8:39 AM Aldy Hernandez via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> It seems to me that we should also check for [0,0] and [1,1] in the
> range, but I am leaving things as is to avoid functional changes.
>
> gcc/ChangeLog:
>
>         * vr-values.c (simplify_using_ranges::op_with_boolean_value_range_p): Adjust
>         for irange API.
> ---
>  gcc/vr-values.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
> index 609375c072e..1190fa96453 100644
> --- a/gcc/vr-values.c
> +++ b/gcc/vr-values.c
> @@ -448,10 +448,11 @@ simplify_using_ranges::op_with_boolean_value_range_p (tree op)
>    if (TREE_CODE (op) != SSA_NAME)
>      return false;
>
> +  /* ?? Errr, this should probably check for [0,0] and [1,1] as well
> +     as [0,1].  */
>    const value_range *vr = get_value_range (op);
> -  return (vr->kind () == VR_RANGE
> -         && integer_zerop (vr->min ())
> -         && integer_onep (vr->max ()));
> +  return *vr == value_range (build_zero_cst (TREE_TYPE (op)),
> +                            build_one_cst (TREE_TYPE (op)));

This now builds trees in a predicate which is highly dubious.  Isn't
there a better (cheaper) primitive to build a [0, 1] range to compare against?
I guess from what I read it will also allocate memory for the range entry?

Richard.


>  }
>
>  /* Extract value range information for VAR when (OP COND_CODE LIMIT) is
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PUSHED 5/8] Adjust vrp_evaluate_conditional for irange API.
  2020-08-04  6:34 ` [PUSHED 5/8] Adjust vrp_evaluate_conditional " Aldy Hernandez
@ 2020-08-04  6:55   ` Richard Biener
  2020-08-04  9:23     ` Aldy Hernandez
  0 siblings, 1 reply; 17+ messages in thread
From: Richard Biener @ 2020-08-04  6:55 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: GCC Patches

On Tue, Aug 4, 2020 at 8:40 AM Aldy Hernandez via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> VR_RANGE of [-INF,+INF] is canonicalized to VARYING at creation.
> That is why the test now becomes varying_p().
>
> gcc/ChangeLog:
>
>         * vr-values.c (simplify_using_ranges::vrp_evaluate_conditional): Adjust
>         for irange API.
> ---
>  gcc/vr-values.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
> index 1190fa96453..90ba8fca246 100644
> --- a/gcc/vr-values.c
> +++ b/gcc/vr-values.c
> @@ -2495,11 +2495,7 @@ simplify_using_ranges::vrp_evaluate_conditional (tree_code code, tree op0,
>        tree type = TREE_TYPE (op0);
>        const value_range_equiv *vr0 = get_value_range (op0);
>
> -      if (vr0->kind () == VR_RANGE
> -         && INTEGRAL_TYPE_P (type)
> -         && vrp_val_is_min (vr0->min ())
> -         && vrp_val_is_max (vr0->max ())
> -         && is_gimple_min_invariant (op1))
> +      if (vr0->varying_p () && INTEGRAL_TYPE_P (type))

You dropped the is_gimple_min_invariant (op1) check.

>         {
>           location_t location;
>
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PUSHED 6/8] Use irange API in test_for_singularity.
  2020-08-04  6:34 ` [PUSHED 6/8] Use irange API in test_for_singularity Aldy Hernandez
@ 2020-08-04  6:58   ` Richard Biener
  2020-08-04  9:55     ` Aldy Hernandez
  0 siblings, 1 reply; 17+ messages in thread
From: Richard Biener @ 2020-08-04  6:58 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: GCC Patches

On Tue, Aug 4, 2020 at 8:40 AM Aldy Hernandez via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> gcc/ChangeLog:
>
>         * vr-values.c (test_for_singularity): Use irange API.
>         (simplify_using_ranges::simplify_cond_using_ranges_1): Do not
>         special case VR_RANGE.
> ---
>  gcc/vr-values.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
> index 90ba8fca246..e78b25596b0 100644
> --- a/gcc/vr-values.c
> +++ b/gcc/vr-values.c
> @@ -3480,10 +3480,13 @@ test_for_singularity (enum tree_code cond_code, tree op0,
>       value range information we have for op0.  */
>    if (min && max)
>      {
> -      if (compare_values (vr->min (), min) == 1)
> -       min = vr->min ();
> -      if (compare_values (vr->max (), max) == -1)
> -       max = vr->max ();
> +      tree type = TREE_TYPE (op0);
> +      tree tmin = wide_int_to_tree (type, vr->lower_bound ());
> +      tree tmax = wide_int_to_tree (type, vr->upper_bound ());

I guess with symbolic ranges this just doesn't work anymore
(or rather will give a pessimistinc upper/lower bound)?

> +      if (compare_values (tmin, min) == 1)
> +       min = tmin;
> +      if (compare_values (tmax, max) == -1)
> +       max = tmax;
>
>        /* If the new min/max values have converged to a single value,
>          then there is only one value which can satisfy the condition,
> @@ -3594,7 +3597,7 @@ simplify_using_ranges::simplify_cond_using_ranges_1 (gcond *stmt)
>
>        /* If we have range information for OP0, then we might be
>          able to simplify this conditional. */
> -      if (vr->kind () == VR_RANGE)
> +      if (!vr->undefined_p () && !vr->varying_p ())
>         {
>           tree new_tree = test_for_singularity (cond_code, op0, op1, vr);
>           if (new_tree)
> --
> 2.26.2
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PUSHED 2/8] Adjust expr_not_equal_to to use irange API.
  2020-08-04  6:52   ` Richard Biener
@ 2020-08-04  9:16     ` Aldy Hernandez
  0 siblings, 0 replies; 17+ messages in thread
From: Aldy Hernandez @ 2020-08-04  9:16 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches



On 8/4/20 8:52 AM, Richard Biener wrote:
> On Tue, Aug 4, 2020 at 8:37 AM Aldy Hernandez via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> gcc/ChangeLog:
>>
>>          * fold-const.c (expr_not_equal_to): Adjust for irange API.
>> ---
>>   gcc/fold-const.c | 17 ++++-------------
>>   1 file changed, 4 insertions(+), 13 deletions(-)
>>
>> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
>> index 1324a194995..5d27927f6bf 100644
>> --- a/gcc/fold-const.c
>> +++ b/gcc/fold-const.c
>> @@ -10194,8 +10194,7 @@ tree_expr_nonzero_p (tree t)
>>   bool
>>   expr_not_equal_to (tree t, const wide_int &w)
>>   {
>> -  wide_int min, max, nz;
>> -  value_range_kind rtype;
>> +  value_range vr;
>>     switch (TREE_CODE (t))
>>       {
>>       case INTEGER_CST:
>> @@ -10204,17 +10203,9 @@ expr_not_equal_to (tree t, const wide_int &w)
>>       case SSA_NAME:
>>         if (!INTEGRAL_TYPE_P (TREE_TYPE (t)))
>>          return false;
>> -      rtype = get_range_info (t, &min, &max);
>> -      if (rtype == VR_RANGE)
>> -       {
>> -         if (wi::lt_p (max, w, TYPE_SIGN (TREE_TYPE (t))))
>> -           return true;
>> -         if (wi::lt_p (w, min, TYPE_SIGN (TREE_TYPE (t))))
>> -           return true;
>> -       }
>> -      else if (rtype == VR_ANTI_RANGE
>> -              && wi::le_p (min, w, TYPE_SIGN (TREE_TYPE (t)))
>> -              && wi::le_p (w, max, TYPE_SIGN (TREE_TYPE (t))))
>> +      get_range_info (t, vr);
> 
> Ick.  Do we now use references for out parameters?  I find this
> highly non-obvious semantics.  What's wrong with
> 
>       vr = get_range_info (t);
> 
> if you dislike get_range_info (t, &vr)?

Using references was decided last year.

We want the ability of to use the same range granularity as what the 
user requested, so we can't just return a range, as we don't know how 
many sub-ranges the user wants:

int_range<10> big_range;
twiddle_range(big_range);

We want the above to work with big_range, or a small range, or a 
value_range.  Twiddle_range should be range agnostic.

get_range_info(const_tree, value_range &) has been available since last 
year.  I suppose if one were so inclined, one could change it to take a 
pointer to be consistent with the other get_range_info() overload.

Aldy


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PUSHED 5/8] Adjust vrp_evaluate_conditional for irange API.
  2020-08-04  6:55   ` Richard Biener
@ 2020-08-04  9:23     ` Aldy Hernandez
  0 siblings, 0 replies; 17+ messages in thread
From: Aldy Hernandez @ 2020-08-04  9:23 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches

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



On 8/4/20 8:55 AM, Richard Biener wrote:
> On Tue, Aug 4, 2020 at 8:40 AM Aldy Hernandez via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> VR_RANGE of [-INF,+INF] is canonicalized to VARYING at creation.
>> That is why the test now becomes varying_p().
>>
>> gcc/ChangeLog:
>>
>>          * vr-values.c (simplify_using_ranges::vrp_evaluate_conditional): Adjust
>>          for irange API.
>> ---
>>   gcc/vr-values.c | 6 +-----
>>   1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
>> index 1190fa96453..90ba8fca246 100644
>> --- a/gcc/vr-values.c
>> +++ b/gcc/vr-values.c
>> @@ -2495,11 +2495,7 @@ simplify_using_ranges::vrp_evaluate_conditional (tree_code code, tree op0,
>>         tree type = TREE_TYPE (op0);
>>         const value_range_equiv *vr0 = get_value_range (op0);
>>
>> -      if (vr0->kind () == VR_RANGE
>> -         && INTEGRAL_TYPE_P (type)
>> -         && vrp_val_is_min (vr0->min ())
>> -         && vrp_val_is_max (vr0->max ())
>> -         && is_gimple_min_invariant (op1))
>> +      if (vr0->varying_p () && INTEGRAL_TYPE_P (type))
> 
> You dropped the is_gimple_min_invariant (op1) check.

Ah, thanks.

Pushed the attached patch.

Aldy

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

commit a44293840c0cb1376ac2f45212da7b8d5d21037a
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Tue Aug 4 11:19:39 2020 +0200

    Add is_gimple_min_invariant dropped from previous patch.
    
    gcc/ChangeLog:
    
            * vr-values.c (simplify_using_ranges::vrp_evaluate_conditional):
            Call is_gimple_min_invariant dropped from previous patch.

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 2fd4956a2e4..511342f2f13 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -2495,7 +2495,9 @@ simplify_using_ranges::vrp_evaluate_conditional (tree_code code, tree op0,
       tree type = TREE_TYPE (op0);
       const value_range_equiv *vr0 = get_value_range (op0);
 
-      if (vr0->varying_p () && INTEGRAL_TYPE_P (type))
+      if (vr0->varying_p ()
+	  && INTEGRAL_TYPE_P (type)
+	  && is_gimple_min_invariant (op1))
 	{
 	  location_t location;
 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PUSHED 4/8] Adjust op_with_boolean_value_range_p for irange API.
  2020-08-04  6:55   ` Richard Biener
@ 2020-08-04  9:45     ` Aldy Hernandez
  0 siblings, 0 replies; 17+ messages in thread
From: Aldy Hernandez @ 2020-08-04  9:45 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches



On 8/4/20 8:55 AM, Richard Biener wrote:
> On Tue, Aug 4, 2020 at 8:39 AM Aldy Hernandez via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> It seems to me that we should also check for [0,0] and [1,1] in the
>> range, but I am leaving things as is to avoid functional changes.
>>
>> gcc/ChangeLog:
>>
>>          * vr-values.c (simplify_using_ranges::op_with_boolean_value_range_p): Adjust
>>          for irange API.
>> ---
>>   gcc/vr-values.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
>> index 609375c072e..1190fa96453 100644
>> --- a/gcc/vr-values.c
>> +++ b/gcc/vr-values.c
>> @@ -448,10 +448,11 @@ simplify_using_ranges::op_with_boolean_value_range_p (tree op)
>>     if (TREE_CODE (op) != SSA_NAME)
>>       return false;
>>
>> +  /* ?? Errr, this should probably check for [0,0] and [1,1] as well
>> +     as [0,1].  */
>>     const value_range *vr = get_value_range (op);
>> -  return (vr->kind () == VR_RANGE
>> -         && integer_zerop (vr->min ())
>> -         && integer_onep (vr->max ()));
>> +  return *vr == value_range (build_zero_cst (TREE_TYPE (op)),
>> +                            build_one_cst (TREE_TYPE (op)));
> 
> This now builds trees in a predicate which is highly dubious.  Isn't
> there a better (cheaper) primitive to build a [0, 1] range to compare against?
> I guess from what I read it will also allocate memory for the range entry?

Both 0 and 1 are cached for all integer types.  We are also caching 1 
and MAX for pointers per the irange patchset, so this shouldn't be a 
performance issue.

Also, we do the same thing for irange::nonzero_p(), which is used a lot 
more often (and probably on critical paths), and per our benchmarks we 
didn't find it to take any significant time:

   tree zero = build_zero_cst (type ());
   return *this == int_range<1> (zero, zero, VR_ANTI_RANGE);

However, I suppose we could implement it with something like:

return vr->num_pairs() == 1
   && vr->lower_bound () == 0
   && vr->upper_bound () == 1

but that's hardly any faster, especially since num_pairs() is 
non-trivial for the value_range legacy code.

I just don't think it's worth it.

Aldy


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PUSHED 6/8] Use irange API in test_for_singularity.
  2020-08-04  6:58   ` Richard Biener
@ 2020-08-04  9:55     ` Aldy Hernandez
  0 siblings, 0 replies; 17+ messages in thread
From: Aldy Hernandez @ 2020-08-04  9:55 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC Patches



On 8/4/20 8:58 AM, Richard Biener wrote:
> On Tue, Aug 4, 2020 at 8:40 AM Aldy Hernandez via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> gcc/ChangeLog:
>>
>>          * vr-values.c (test_for_singularity): Use irange API.
>>          (simplify_using_ranges::simplify_cond_using_ranges_1): Do not
>>          special case VR_RANGE.
>> ---
>>   gcc/vr-values.c | 13 ++++++++-----
>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
>> index 90ba8fca246..e78b25596b0 100644
>> --- a/gcc/vr-values.c
>> +++ b/gcc/vr-values.c
>> @@ -3480,10 +3480,13 @@ test_for_singularity (enum tree_code cond_code, tree op0,
>>        value range information we have for op0.  */
>>     if (min && max)
>>       {
>> -      if (compare_values (vr->min (), min) == 1)
>> -       min = vr->min ();
>> -      if (compare_values (vr->max (), max) == -1)
>> -       max = vr->max ();
>> +      tree type = TREE_TYPE (op0);
>> +      tree tmin = wide_int_to_tree (type, vr->lower_bound ());
>> +      tree tmax = wide_int_to_tree (type, vr->upper_bound ());
> 
> I guess with symbolic ranges this just doesn't work anymore
> (or rather will give a pessimistinc upper/lower bound)?

Yes, though we do slightly better than VARYING.  The symbolic 
normalizing code will rewrite [SYM, 5] as [-INF, 5], etc.

When I implemented this originally in the ranger branch, I 
pessimistically downgraded all symbolics to [MIN,MAX] to see if there 
was any difference in the generated code.  There wasn't.

I think most of vr-values.c does no better without symbolics, with the 
exception of compare_value* and the corresponding code that handles 
comparisons and equivalences.

Aldy


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-08-04  9:56 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-04  6:34 [PUSHED 0/8] irange API adjustments for various files Aldy Hernandez
2020-08-04  6:34 ` [PUSHED 1/8] Remove ad-hoc range canonicalization from determine_block_size Aldy Hernandez
2020-08-04  6:34 ` [PUSHED 2/8] Adjust expr_not_equal_to to use irange API Aldy Hernandez
2020-08-04  6:52   ` Richard Biener
2020-08-04  9:16     ` Aldy Hernandez
2020-08-04  6:34 ` [PUSHED 3/8] Adjust get_range_info to use the base irange class Aldy Hernandez
2020-08-04  6:34 ` [PUSHED 4/8] Adjust op_with_boolean_value_range_p for irange API Aldy Hernandez
2020-08-04  6:55   ` Richard Biener
2020-08-04  9:45     ` Aldy Hernandez
2020-08-04  6:34 ` [PUSHED 5/8] Adjust vrp_evaluate_conditional " Aldy Hernandez
2020-08-04  6:55   ` Richard Biener
2020-08-04  9:23     ` Aldy Hernandez
2020-08-04  6:34 ` [PUSHED 6/8] Use irange API in test_for_singularity Aldy Hernandez
2020-08-04  6:58   ` Richard Biener
2020-08-04  9:55     ` Aldy Hernandez
2020-08-04  6:34 ` [PUSHED 7/8] Adjust simplify_conversion_using_ranges for irange API Aldy Hernandez
2020-08-04  6:34 ` [PUSHED 8/8] Adjust two_valued_val_range_p " Aldy Hernandez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).