disentangle range_fold_*ary_expr into various pieces

Message ID 20396d4d-cc22-eeab-5cea-685fc077b5f7@redhat.com
State New
Headers show
Series
  • disentangle range_fold_*ary_expr into various pieces
Related show

Commit Message

Aldy Hernandez Oct. 4, 2019, 1:25 p.m.
I promised I would clean up the VRP/range-ops interface once the patch 
went in.  I was afraid of shuffling things too much initially, because I 
was trying to keep to the original structure of 
extract_range_from_*ary_expr.  Now that things are in place, it's easier 
to abstract things out, and clean them up a bit.

The gist of this patch is to pull out all the different sections of the 
VRP/range-ops glue (range_fold_{binary,unary}) into different functions. 
  In doing this, the overall structure is easier to see, and I was able 
to pin point (and remove) a lot of redundant work.

Unfortunately, the patch is hard to read because of the reshuffling.  It 
may be easier to just look at the new patched tree-vrp.c starting at 
get_range_op_handler() through range_fold_unary_expr().

The general idea is that we now have:

void
range_fold_binary_expr (value_range_base *vr,
			enum tree_code code,
			tree expr_type,
			const value_range_base *vr0_,
			const value_range_base *vr1_)
{
   if (!supported_types_p (vr, expr_type)
       || !defined_ranges_p (vr, vr0_, vr1_))
     return;
   const range_operator *op = get_range_op_handler (vr, code, expr_type);
   if (!op)
     return;

   value_range_base vr0 = drop_undefines_to_varying (vr0_, expr_type);
   value_range_base vr1 = drop_undefines_to_varying (vr1_, expr_type);
   if (range_fold_binary_symbolics_p (vr, code, expr_type, &vr0, &vr1))
     return;

   *vr = op->fold_range (expr_type,
			vr0.normalize_addresses (),
			vr1.normalize_addresses ());
}

...which is WAY easier to read than the spaghetti we had before.  Note 
that all the symbolic stuff is neatly contained in range_fold_*symbolics_p.

In doing so, I added the ability to normalize addresses into 
normalize_symbolics, which centralizes the place where we transform a 
non-numeric range into a strictly numeric range.  As a reference, 
"symbolics" have traditionally been things like [x + 5, 8], but a 
reference is not strictly symbolic, but it's still not numeric [&a, &a], 
etc.  With the current patch, normalize_symbolics will return a numeric 
(or varying or undefined).

[Yeah, we should call that ::normalize_non_numeric or something, perhaps 
later.]

I also exposed the new ::normalize_addresses method, so the 
range_fold_*ary* stuff can normalize addresses instead of going through 
the entire normalize_symbolics dance.  Though, I could just as easily 
just call normalize_symbolics from the range_fold* stuff.  It just 
seemed like a useful small optimization.

I was pleasantly surprised that the symbolic handling of 
CONVERT_EXPR_CODE_P melted in favor of the generic ::normalize_symbolics 
mechanism.

Anyways, sorry for the gymnastics here, but regardless of what happens 
to range_fold_*ary*, it needed a good clean-up.

OK?

Aldy

Comments

Jeff Law Oct. 4, 2019, 6:09 p.m. | #1
On 10/4/19 7:25 AM, Aldy Hernandez wrote:
> I promised I would clean up the VRP/range-ops interface once the patch

> went in.  I was afraid of shuffling things too much initially, because I

> was trying to keep to the original structure of

> extract_range_from_*ary_expr.  Now that things are in place, it's easier

> to abstract things out, and clean them up a bit.

> 

> The gist of this patch is to pull out all the different sections of the

> VRP/range-ops glue (range_fold_{binary,unary}) into different functions.

>  In doing this, the overall structure is easier to see, and I was able

> to pin point (and remove) a lot of redundant work.

> 

> Unfortunately, the patch is hard to read because of the reshuffling.  It

> may be easier to just look at the new patched tree-vrp.c starting at

> get_range_op_handler() through range_fold_unary_expr().

> 

> The general idea is that we now have:

> 

> void

> range_fold_binary_expr (value_range_base *vr,

>             enum tree_code code,

>             tree expr_type,

>             const value_range_base *vr0_,

>             const value_range_base *vr1_)

> {

>   if (!supported_types_p (vr, expr_type)

>       || !defined_ranges_p (vr, vr0_, vr1_))

>     return;

>   const range_operator *op = get_range_op_handler (vr, code, expr_type);

>   if (!op)

>     return;

> 

>   value_range_base vr0 = drop_undefines_to_varying (vr0_, expr_type);

>   value_range_base vr1 = drop_undefines_to_varying (vr1_, expr_type);

>   if (range_fold_binary_symbolics_p (vr, code, expr_type, &vr0, &vr1))

>     return;

> 

>   *vr = op->fold_range (expr_type,

>             vr0.normalize_addresses (),

>             vr1.normalize_addresses ());

> }

> 

> ...which is WAY easier to read than the spaghetti we had before.  Note

> that all the symbolic stuff is neatly contained in range_fold_*symbolics_p.

> 

> In doing so, I added the ability to normalize addresses into

> normalize_symbolics, which centralizes the place where we transform a

> non-numeric range into a strictly numeric range.  As a reference,

> "symbolics" have traditionally been things like [x + 5, 8], but a

> reference is not strictly symbolic, but it's still not numeric [&a, &a],

> etc.  With the current patch, normalize_symbolics will return a numeric

> (or varying or undefined).

> 

> [Yeah, we should call that ::normalize_non_numeric or something, perhaps

> later.]

> 

> I also exposed the new ::normalize_addresses method, so the

> range_fold_*ary* stuff can normalize addresses instead of going through

> the entire normalize_symbolics dance.  Though, I could just as easily

> just call normalize_symbolics from the range_fold* stuff.  It just

> seemed like a useful small optimization.

> 

> I was pleasantly surprised that the symbolic handling of

> CONVERT_EXPR_CODE_P melted in favor of the generic ::normalize_symbolics

> mechanism.

> 

> Anyways, sorry for the gymnastics here, but regardless of what happens

> to range_fold_*ary*, it needed a good clean-up.

> 

> OK?

> 

> Aldy

> 

> range-fold-cleanups.patch

> 

> commit 70d336374fb39e857ecbd720900df9869c3c2ce9

> Author: Aldy Hernandez <aldyh@redhat.com>

> Date:   Thu Oct 3 14:53:12 2019 +0200

> 

>     Disentangle range_fold_*ary_expr() into various independent pieces.

> 

> diff --git a/gcc/ChangeLog b/gcc/ChangeLog

> index 3934b41fdf9..714b22781ee 100644

> --- a/gcc/ChangeLog

> +++ b/gcc/ChangeLog

> @@ -1,3 +1,17 @@

> +2019-10-04  Aldy Hernandez  <aldyh@redhat.com>

> +

> +	* tree-vrp.c (get_range_op_handler): New.

> +	(supported_types_p): New.

> +	(defined_ranges_p): New.

> +	(drop_undefines_to_varying): New.

> +	(range_fold_binary_symbolics_p): New.

> +	(range_fold_unary_symbolics_p): New.

> +	(range_fold_unary_expr): Extract out into above functions.

> +	(range_fold_binary_expr): Same.

> +	(value_range_base::normalize_addresses): New.

> +	(value_range_base::normalize_symbolics): Normalize addresses.

> +	* tree-vrp.h (class value_range_base): Add normalize_addresses.

You're right.  Easier to refer to the before/after directly.  Sometimes
diffs just suck.

OK
jeff
Aldy Hernandez Oct. 7, 2019, 11:42 a.m. | #2
On 10/4/19 2:09 PM, Jeff Law wrote:

> You're right.  Easier to refer to the before/after directly.  Sometimes

> diffs just suck.

> 

> OK

> jeff


In testing this patch in isolation from the non-zero canonicalization 
patch, I found one regression due to the fact that:

a) As discussed, two non-zero representations currently exist for 
unsigned ranges.

b) ipa-prop.c has it's own hacked up value_range structure (ipa_vr) 
which doesn't use any API.  Since there is no agreed upon non-zero, 
range-ops can sometimes (correctly) create an unsigned [1,MAX], and 
ipa-prop.c is open-coding the check for a pointer non-zero to ~[0,0]. 
This seems like a latent bug.

I really have no idea, nor do I care (*), what we do with ipa-prop's 
lack of API.  For now, I have implemented ipa_vr::nonzero_p(), and used 
it.  When we agree on the non-zero normalization we can adjust this 
method if necessary.

+bool
+ipa_vr::nonzero_p (tree expr_type) const
+{
+  if (type == VR_ANTI_RANGE && wi::eq_p (min, 0) && wi::eq_p (max, 0))
+    return true;
+
+  unsigned prec = TYPE_PRECISION (expr_type);
+  return (type == VR_RANGE
+         && wi::eq_p (min, wi::one (prec))
+         && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN (expr_type))));
+}

...

            else if (POINTER_TYPE_P (TREE_TYPE (ddef))
-                  && vr[i].type == VR_ANTI_RANGE
-                  && wi::eq_p (vr[i].min, 0)
-                  && wi::eq_p (vr[i].max, 0))
+                  && vr[i].nonzero_p (TREE_TYPE (ddef)))

Attached is the final adjusted patch I have committed to trunk.

Aldy

(*) This can be loosely translated as "it'll eventually bite us in the 
ass, at which point I'll end up addressing it." ;-)
commit 3a7c5a78fe9226daed2d5363a97f87081acd82c7
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Thu Oct 3 14:53:12 2019 +0200

    Disentangle range_fold_*ary_expr() into various independent pieces.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index fbb1634cbae..83ad0017b5a 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,23 @@
+2019-10-07  Aldy Hernandez  <aldyh@redhat.com>
+
+	* ipa-prop.c (ipa_vr::nonzero_p): New.
+	(ipcp_update_vr): Use nonzero_p instead of open-coding check for
+	non-zero range.
+	* ipa-prop.h (class ipa_vr): Add nonzero_p.
+	* tree-vrp.c (range_has_numeric_bounds_p): New.
+	(range_int_cst_p): Use range_has_numeric_bounds_p.
+	(get_range_op_handler): New.
+	(supported_types_p): New.
+	(defined_ranges_p): New.
+	(drop_undefines_to_varying): New.
+	(range_fold_binary_symbolics_p): New.
+	(range_fold_unary_symbolics_p): New.
+	(range_fold_unary_expr): Extract out into above functions.
+	(range_fold_binary_expr): Same.
+	(value_range_base::normalize_addresses): New.
+	(value_range_base::normalize_symbolics): Normalize addresses.
+	* tree-vrp.h (class value_range_base): Add normalize_addresses.
+
 2019-10-07  Aldy Hernandez  <aldyh@redhat.com>
 
 	* tree-vrp.c (value_range_base::singleton_p): Use
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index e79add12b1e..20a0bddcbab 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -5109,6 +5109,18 @@ ipcp_update_bits (struct cgraph_node *node)
     }
 }
 
+bool
+ipa_vr::nonzero_p (tree expr_type) const
+{
+  if (type == VR_ANTI_RANGE && wi::eq_p (min, 0) && wi::eq_p (max, 0))
+    return true;
+
+  unsigned prec = TYPE_PRECISION (expr_type);
+  return (type == VR_RANGE
+	  && wi::eq_p (min, wi::one (prec))
+	  && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN (expr_type))));
+}
+
 /* Update value range of formal parameters as described in
    ipcp_transformation.  */
 
@@ -5181,9 +5193,7 @@ ipcp_update_vr (struct cgraph_node *node)
 						      TYPE_SIGN (type)));
 	    }
 	  else if (POINTER_TYPE_P (TREE_TYPE (ddef))
-		   && vr[i].type == VR_ANTI_RANGE
-		   && wi::eq_p (vr[i].min, 0)
-		   && wi::eq_p (vr[i].max, 0))
+		   && vr[i].nonzero_p (TREE_TYPE (ddef)))
 	    {
 	      if (dump_file)
 		fprintf (dump_file, "Setting nonnull for %u\n", i);
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 0ff80854284..eb3397a6d81 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -165,6 +165,7 @@ public:
   enum value_range_kind type;
   wide_int min;
   wide_int max;
+  bool nonzero_p (tree) const;
 };
 
 /* A jump function for a callsite represents the values passed as actual
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 86e4dace073..d69cfb107cb 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -910,15 +910,21 @@ vrp_bitmap_equal_p (const_bitmap b1, const_bitmap b2)
 	      && bitmap_equal_p (b1, b2)));
 }
 
+static bool
+range_has_numeric_bounds_p (const value_range_base *vr)
+{
+  return (vr->min ()
+	  && TREE_CODE (vr->min ()) == INTEGER_CST
+	  && TREE_CODE (vr->max ()) == INTEGER_CST);
+}
+
 /* Return true if max and min of VR are INTEGER_CST.  It's not necessary
    a singleton.  */
 
 bool
 range_int_cst_p (const value_range_base *vr)
 {
-  return (vr->kind () == VR_RANGE
-	  && TREE_CODE (vr->min ()) == INTEGER_CST
-	  && TREE_CODE (vr->max ()) == INTEGER_CST);
+  return (vr->kind () == VR_RANGE && range_has_numeric_bounds_p (vr));
 }
 
 /* Return true if VR is a INTEGER_CST singleton.  */
@@ -1760,119 +1766,102 @@ extract_range_from_plus_minus_expr (value_range_base *vr,
     vr->set (kind, min, max);
 }
 
-/* Normalize a value_range for use in range_ops and return it.  */
+/* Return the range-ops handler for CODE and EXPR_TYPE.  If no
+   suitable operator is found, return NULL and set VR to VARYING.  */
 
-static value_range_base
-normalize_for_range_ops (const value_range_base &vr)
+static const range_operator *
+get_range_op_handler (value_range_base *vr,
+		      enum tree_code code,
+		      tree expr_type)
 {
-  tree type = vr.type ();
+  const range_operator *op = range_op_handler (code, expr_type);
+  if (!op)
+    vr->set_varying (expr_type);
+  return op;
+}
+
+/* If the types passed are supported, return TRUE, otherwise set VR to
+   VARYING and return FALSE.  */
 
-  /* This will return ~[0,0] for [&var, &var].  */
-  if (POINTER_TYPE_P (type) && !range_includes_zero_p (&vr))
+static bool
+supported_types_p (value_range_base *vr,
+		   tree type0,
+		   tree type1 = NULL)
+{
+  if (!value_range_base::supports_type_p (type0)
+      || (type1 && !value_range_base::supports_type_p (type1)))
     {
-      value_range_base temp;
-      temp.set_nonzero (type);
-      return temp;
+      vr->set_varying (type0);
+      return false;
     }
-  if (vr.symbolic_p ())
-    return normalize_for_range_ops (vr.normalize_symbolics ());
-  if (TREE_CODE (vr.min ()) == INTEGER_CST
-      && TREE_CODE (vr.max ()) == INTEGER_CST)
-    return vr;
-  /* Anything not strictly numeric at this point becomes varying.  */
-  return value_range_base (vr.type ());
+  return true;
 }
 
-/* Fold a binary expression of two value_range's with range-ops.  */
+/* If any of the ranges passed are defined, return TRUE, otherwise set
+   VR to UNDEFINED and return FALSE.  */
 
-void
-range_fold_binary_expr (value_range_base *vr,
-			enum tree_code code,
-			tree expr_type,
-			const value_range_base *vr0_,
-			const value_range_base *vr1_)
+static bool
+defined_ranges_p (value_range_base *vr,
+		  const value_range_base *vr0,
+		  const value_range_base *vr1 = NULL)
 {
-  if (!value_range_base::supports_type_p (expr_type)
-      || (!vr0_->undefined_p ()
-	  && !value_range_base::supports_type_p (vr0_->type ()))
-      || (!vr1_->undefined_p ()
-	  && !value_range_base::supports_type_p (vr1_->type ())))
-    {
-      vr->set_varying (expr_type);
-      return;
-    }
-  if (vr0_->undefined_p () && vr1_->undefined_p ())
+  if (vr0->undefined_p () && (!vr1 || vr1->undefined_p ()))
     {
       vr->set_undefined ();
-      return;
-    }
-  range_operator *op = range_op_handler (code, expr_type);
-  if (!op)
-    {
-      vr->set_varying (expr_type);
-      return;
+      return false;
     }
+  return true;
+}
 
-  /* Mimic any behavior users of extract_range_from_binary_expr may
-     expect.  */
-  value_range_base vr0 = *vr0_, vr1 = *vr1_;
-  if (vr0.undefined_p ())
-    vr0.set_varying (expr_type);
-  else if (vr1.undefined_p ())
-    vr1.set_varying (expr_type);
+static value_range_base
+drop_undefines_to_varying (const value_range_base *vr, tree expr_type)
+{
+  if (vr->undefined_p ())
+    return value_range_base (expr_type);
+  else
+    return *vr;
+}
+
+/* If any operand is symbolic, perform a binary operation on them and
+   return TRUE, otherwise return FALSE.  */
 
-  /* Handle symbolics.  */
-  if (vr0.symbolic_p () || vr1.symbolic_p ())
+static bool
+range_fold_binary_symbolics_p (value_range_base *vr,
+			       tree_code code,
+			       tree expr_type,
+			       const value_range_base *vr0,
+			       const value_range_base *vr1)
+{
+  if (vr0->symbolic_p () || vr1->symbolic_p ())
     {
       if ((code == PLUS_EXPR || code == MINUS_EXPR))
 	{
-	  extract_range_from_plus_minus_expr (vr, code, expr_type,
-					      &vr0, &vr1);
-	  return;
+	  extract_range_from_plus_minus_expr (vr, code, expr_type, vr0, vr1);
+	  return true;
 	}
       if (POINTER_TYPE_P (expr_type) && code == POINTER_PLUS_EXPR)
 	{
-	  extract_range_from_pointer_plus_expr (vr, code, expr_type,
-						&vr0, &vr1);
-	  return;
+	  extract_range_from_pointer_plus_expr (vr, code, expr_type, vr0, vr1);
+	  return true;
 	}
+      const range_operator *op = get_range_op_handler (vr, code, expr_type);
+      *vr = op->fold_range (expr_type,
+			    vr0->normalize_symbolics (),
+			    vr1->normalize_symbolics ());
+      return true;
     }
-
-  /* Do the range-ops dance.  */
-  value_range_base n0 = normalize_for_range_ops (vr0);
-  value_range_base n1 = normalize_for_range_ops (vr1);
-  *vr = op->fold_range (expr_type, n0, n1);
+  return false;
 }
 
-/* Fold a unary expression of a value_range with range-ops.  */
+/* If operand is symbolic, perform a unary operation on it and return
+   TRUE, otherwise return FALSE.  */
 
-void
-range_fold_unary_expr (value_range_base *vr,
-		       enum tree_code code, tree expr_type,
-		       const value_range_base *vr0,
-		       tree vr0_type)
+static bool
+range_fold_unary_symbolics_p (value_range_base *vr,
+			      tree_code code,
+			      tree expr_type,
+			      const value_range_base *vr0)
 {
-  /* Mimic any behavior users of extract_range_from_unary_expr may
-     expect.  */
-  if (!value_range_base::supports_type_p (expr_type)
-      || !value_range_base::supports_type_p (vr0_type))
-    {
-      vr->set_varying (expr_type);
-      return;
-    }
-  if (vr0->undefined_p ())
-    {
-      vr->set_undefined ();
-      return;
-    }
-  range_operator *op = range_op_handler (code, expr_type);
-  if (!op)
-    {
-      vr->set_varying (expr_type);
-      return;
-    }
-
-  /* Handle symbolics.  */
   if (vr0->symbolic_p ())
     {
       if (code == NEGATE_EXPR)
@@ -1881,7 +1870,7 @@ range_fold_unary_expr (value_range_base *vr,
 	  value_range_base zero;
 	  zero.set_zero (vr0->type ());
 	  range_fold_binary_expr (vr, MINUS_EXPR, expr_type, &zero, vr0);
-	  return;
+	  return true;
 	}
       if (code == BIT_NOT_EXPR)
 	{
@@ -1889,30 +1878,64 @@ range_fold_unary_expr (value_range_base *vr,
 	  value_range_base minusone;
 	  minusone.set (build_int_cst (vr0->type (), -1));
 	  range_fold_binary_expr (vr, MINUS_EXPR, expr_type, &minusone, vr0);
-	  return;
+	  return true;
 	}
+      const range_operator *op = get_range_op_handler (vr, code, expr_type);
       *vr = op->fold_range (expr_type,
-			    normalize_for_range_ops (*vr0),
+			    vr0->normalize_symbolics (),
 			    value_range_base (expr_type));
-      return;
-    }
-  if (CONVERT_EXPR_CODE_P (code) && (POINTER_TYPE_P (expr_type)
-				     || POINTER_TYPE_P (vr0->type ())))
-    {
-      /* This handles symbolic conversions such such as [25, x_4].  */
-      if (!range_includes_zero_p (vr0))
-	vr->set_nonzero (expr_type);
-      else if (vr0->zero_p ())
-	vr->set_zero (expr_type);
-      else
-	vr->set_varying (expr_type);
-      return;
+      return true;
     }
+  return false;
+}
 
-  /* Do the range-ops dance.  */
-  value_range_base n0 = normalize_for_range_ops (*vr0);
-  value_range_base n1 (expr_type);
-  *vr = op->fold_range (expr_type, n0, n1);
+/* Perform a binary operation on a pair of ranges.  */
+
+void
+range_fold_binary_expr (value_range_base *vr,
+			enum tree_code code,
+			tree expr_type,
+			const value_range_base *vr0_,
+			const value_range_base *vr1_)
+{
+  if (!supported_types_p (vr, expr_type)
+      || !defined_ranges_p (vr, vr0_, vr1_))
+    return;
+  const range_operator *op = get_range_op_handler (vr, code, expr_type);
+  if (!op)
+    return;
+
+  value_range_base vr0 = drop_undefines_to_varying (vr0_, expr_type);
+  value_range_base vr1 = drop_undefines_to_varying (vr1_, expr_type);
+  if (range_fold_binary_symbolics_p (vr, code, expr_type, &vr0, &vr1))
+    return;
+
+  *vr = op->fold_range (expr_type,
+			vr0.normalize_addresses (),
+			vr1.normalize_addresses ());
+}
+
+/* Perform a unary operation on a range.  */
+
+void
+range_fold_unary_expr (value_range_base *vr,
+		       enum tree_code code, tree expr_type,
+		       const value_range_base *vr0,
+		       tree vr0_type)
+{
+  if (!supported_types_p (vr, expr_type, vr0_type)
+      || !defined_ranges_p (vr, vr0))
+    return;
+  const range_operator *op = get_range_op_handler (vr, code, expr_type);
+  if (!op)
+    return;
+
+  if (range_fold_unary_symbolics_p (vr, code, expr_type, vr0))
+    return;
+
+  *vr = op->fold_range (expr_type,
+			vr0->normalize_addresses (),
+			value_range_base (expr_type));
 }
 
 /* Given a COND_EXPR COND of the form 'V OP W', and an SSA name V,
@@ -5997,7 +6020,24 @@ value_range::union_ (const value_range *other)
     }
 }
 
-/* Normalize symbolics into constants.  */
+/* Normalize addresses into constants.  */
+
+value_range_base
+value_range_base::normalize_addresses () const
+{
+  if (!POINTER_TYPE_P (type ()) || range_has_numeric_bounds_p (this))
+    return *this;
+
+  if (!range_includes_zero_p (this))
+    {
+      gcc_checking_assert (TREE_CODE (m_min) == ADDR_EXPR
+			   || TREE_CODE (m_max) == ADDR_EXPR);
+      return range_nonzero (type ());
+    }
+  return value_range_base (type ());
+}
+
+/* Normalize symbolics and addresses into constants.  */
 
 value_range_base
 value_range_base::normalize_symbolics () const
@@ -6008,7 +6048,7 @@ value_range_base::normalize_symbolics () const
   bool min_symbolic = !is_gimple_min_invariant (min ());
   bool max_symbolic = !is_gimple_min_invariant (max ());
   if (!min_symbolic && !max_symbolic)
-    return *this;
+    return normalize_addresses ();
 
   // [SYM, SYM] -> VARYING
   if (min_symbolic && max_symbolic)
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
index d20d0043ba3..a3f9e90699d 100644
--- a/gcc/tree-vrp.h
+++ b/gcc/tree-vrp.h
@@ -86,6 +86,7 @@ public:
 
   static bool supports_type_p (tree);
   value_range_base normalize_symbolics () const;
+  value_range_base normalize_addresses () const;
 
   static const unsigned int m_max_pairs = 2;
   bool contains_p (tree) const;
Aldy Hernandez Oct. 7, 2019, 11:53 a.m. | #3
> +bool

> +ipa_vr::nonzero_p (tree expr_type) const

> +{

> +  if (type == VR_ANTI_RANGE && wi::eq_p (min, 0) && wi::eq_p (max, 0))

> +    return true;

> +

> +  unsigned prec = TYPE_PRECISION (expr_type);

> +  return (type == VR_RANGE

> +         && wi::eq_p (min, wi::one (prec))

> +         && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN (expr_type))));

> +}


Errr, wrong version posted.  There was a TYPE_UNSIGNED missing.

Fixed and committed.

diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 20a0bddcbab..5020f4a44d5 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -5117,6 +5117,7 @@ ipa_vr::nonzero_p (tree expr_type) const

    unsigned prec = TYPE_PRECISION (expr_type);
    return (type == VR_RANGE
+         && TYPE_UNSIGNED (expr_type)
           && wi::eq_p (min, wi::one (prec))
           && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN (expr_type))));
  }
Marc Glisse Oct. 7, 2019, 6:33 p.m. | #4
On Mon, 7 Oct 2019, Aldy Hernandez wrote:

> On 10/4/19 2:09 PM, Jeff Law wrote:

>

>> You're right.  Easier to refer to the before/after directly.  Sometimes

>> diffs just suck.

>> 

>> OK

>> jeff

>

> In testing this patch in isolation from the non-zero canonicalization patch, 

> I found one regression due to the fact that:

>

> a) As discussed, two non-zero representations currently exist for unsigned 

> ranges.

>

> b) ipa-prop.c has it's own hacked up value_range structure (ipa_vr) which 

> doesn't use any API.  Since there is no agreed upon non-zero, range-ops can 

> sometimes (correctly) create an unsigned [1,MAX], and ipa-prop.c is 

> open-coding the check for a pointer non-zero to ~[0,0]. This seems like a 

> latent bug.

>

> I really have no idea, nor do I care (*), what we do with ipa-prop's lack of 

> API.  For now, I have implemented ipa_vr::nonzero_p(), and used it.  When we 

> agree on the non-zero normalization we can adjust this method if necessary.

>

> +bool

> +ipa_vr::nonzero_p (tree expr_type) const

> +{

> +  if (type == VR_ANTI_RANGE && wi::eq_p (min, 0) && wi::eq_p (max, 0))

> +    return true;

> +

> +  unsigned prec = TYPE_PRECISION (expr_type);

> +  return (type == VR_RANGE

> +         && wi::eq_p (min, wi::one (prec))

> +         && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN (expr_type))));

> +}

>

> ...

>

>           else if (POINTER_TYPE_P (TREE_TYPE (ddef))

> -                  && vr[i].type == VR_ANTI_RANGE

> -                  && wi::eq_p (vr[i].min, 0)

> -                  && wi::eq_p (vr[i].max, 0))

> +                  && vr[i].nonzero_p (TREE_TYPE (ddef)))

>

> Attached is the final adjusted patch I have committed to trunk.


I wonder why we would ever want to ask "is this interval the one that 
misses exactly the value 0" instead of "does this interval contain the 
value 0". I naively believe there shouldn't even be any API for the first 
question. Or if pointers really only have 2 possible intervals (besides 
varying and undefined), aka [0,0] and ~[0,0], using intervals seems like 
overkill for them...

-- 
Marc Glisse
Martin Jambor Oct. 8, 2019, 11:17 a.m. | #5
Hi,

On Tue, Oct 08 2019, Marc Glisse wrote:
> On Mon, 7 Oct 2019, Aldy Hernandez wrote:

>>

>> In testing this patch in isolation from the non-zero canonicalization patch, 

>> I found one regression due to the fact that:

>>

>> a) As discussed, two non-zero representations currently exist for unsigned 

>> ranges.

>>

>> b) ipa-prop.c has it's own hacked up value_range structure (ipa_vr) which 

>> doesn't use any API.  Since there is no agreed upon non-zero, range-ops can 

>> sometimes (correctly) create an unsigned [1,MAX], and ipa-prop.c is 

>> open-coding the check for a pointer non-zero to ~[0,0]. This seems like a 

>> latent bug.

>>

>> I really have no idea, nor do I care (*), what we do with ipa-prop's lack of 

>> API.  For now, I have implemented ipa_vr::nonzero_p(), and used it.  When we 

>> agree on the non-zero normalization we can adjust this method if necessary.

>>

>> +bool

>> +ipa_vr::nonzero_p (tree expr_type) const

>> +{

>> +  if (type == VR_ANTI_RANGE && wi::eq_p (min, 0) && wi::eq_p (max, 0))

>> +    return true;

>> +

>> +  unsigned prec = TYPE_PRECISION (expr_type);

>> +  return (type == VR_RANGE

>> +         && wi::eq_p (min, wi::one (prec))

>> +         && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN (expr_type))));

>> +}

>>

>> ...

>>

>>           else if (POINTER_TYPE_P (TREE_TYPE (ddef))

>> -                  && vr[i].type == VR_ANTI_RANGE

>> -                  && wi::eq_p (vr[i].min, 0)

>> -                  && wi::eq_p (vr[i].max, 0))

>> +                  && vr[i].nonzero_p (TREE_TYPE (ddef)))

>>

>> Attached is the final adjusted patch I have committed to trunk.

>

> I wonder why we would ever want to ask "is this interval the one that 

> misses exactly the value 0" instead of "does this interval contain the 

> value 0". I naively believe there shouldn't even be any API for the first 

> question. Or if pointers really only have 2 possible intervals (besides 

> varying and undefined), aka [0,0] and ~[0,0], using intervals seems like 

> overkill for them...

>


The only use of this code is to see if we can do

  set_ptr_nonnull (ddef)

where ddef is the default definition of a pointer argument described by
the value range.  For integer arguments, we use the values in ipa_vr to
set_range_info of the default definition, so even if it is an overkill
for pointers, the data structure cannot be replaced with just a flag.

While I know that ~[0,0] is by far the most common pointer ipa_vr, I
will have a look whether it makes sense to rewrite the test as you
suggested after I pull the changes from trunk.

Thanks for pointing it out,

Martin
Christophe Lyon Oct. 10, 2019, 9:25 a.m. | #6
On Mon, 7 Oct 2019 at 13:53, Aldy Hernandez <aldyh@redhat.com> wrote:

>

> > +bool

> > +ipa_vr::nonzero_p (tree expr_type) const

> > +{

> > +  if (type == VR_ANTI_RANGE && wi::eq_p (min, 0) && wi::eq_p (max, 0))

> > +    return true;

> > +

> > +  unsigned prec = TYPE_PRECISION (expr_type);

> > +  return (type == VR_RANGE

> > +         && wi::eq_p (min, wi::one (prec))

> > +         && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN

> (expr_type))));

> > +}

>

> Errr, wrong version posted.  There was a TYPE_UNSIGNED missing.

>

> Fixed and committed.

>

>

Hi,

Since this was committed (r276654), I've noticed regressions on aarch64:
    gcc.target/aarch64/pr88838.c (test for excess errors)
    gcc.target/aarch64/stack-check-cfa-3.c (test for excess errors)
    gcc.target/aarch64/stack-check-prologue-16.c (test for excess errors)
    gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve
 scan-assembler-times \\tabs\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b\\n 1
    gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve
 scan-assembler-times \\tabs\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d\\n 1
    gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve
 scan-assembler-times \\tabs\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h\\n 1
    gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve
 scan-assembler-times \\tabs\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s\\n 1
    gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve (test for excess
errors)
    gcc.target/aarch64/sve/adr_1.c -march=armv8.2-a+sve (test for excess
errors)

and many others

For instance:
compiler exited with status 1
FAIL: gcc.target/aarch64/pr88838.c (internal compiler error)
FAIL: gcc.target/aarch64/pr88838.c (test for excess errors)
Excess errors:
during GIMPLE pass: dom
/gcc/testsuite/gcc.target/aarch64/pr88838.c:5:1: internal compiler error:
tree check: expected integer_cst, have poly_int_cst in to_wide, at
tree.h:5795
0x5efa71 tree_check_failed(tree_node const*, char const*, int, char const*,
...)
        /gcc/tree.c:9926
0x749584 tree_check(tree_node const*, char const*, int, char const*,
tree_code)
        /gcc/tree.h:3523
0x749584 wi::to_wide(tree_node const*)
        /gcc/tree.h:5795
0xf7c0c0 value_range_base::lower_bound(unsigned int) const
        /gcc/tree-vrp.c:6136
0x155d2e6 range_operator::fold_range(tree_node*, value_range_base const&,
value_range_base const&) const
        /gcc/range-op.cc:156
0xf87597 range_fold_binary_expr(value_range_base*, tree_code, tree_node*,
value_range_base const*, value_range_base const*)
        /gcc/tree-vrp.c:1915
0x1007962 vr_values::extract_range_from_binary_expr(value_range*,
tree_code, tree_node*, tree_node*, tree_node*)
        /gcc/vr-values.c:808
0x1011f0c vr_values::extract_range_from_assignment(value_range*, gassign*)
        /gcc/vr-values.c:1469
0x1499d21 evrp_range_analyzer::record_ranges_from_stmt(gimple*, bool)
        /gcc/gimple-ssa-evrp-analyze.c:307
0xdd76dd dom_opt_dom_walker::before_dom_children(basic_block_def*)
        /gcc/tree-ssa-dom.c:1503
0x146e68a dom_walker::walk(basic_block_def*)
        /gcc/domwalk.c:309
0xdd4759 execute
        /gcc/tree-ssa-dom.c:724

gcc.target/aarch64/pr88838.c: output file does not exist
UNRESOLVED: gcc.target/aarch64/pr88838.c scan-assembler-not sxtw

Christophe

diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
> index 20a0bddcbab..5020f4a44d5 100644

> --- a/gcc/ipa-prop.c

> +++ b/gcc/ipa-prop.c

> @@ -5117,6 +5117,7 @@ ipa_vr::nonzero_p (tree expr_type) const

>

>     unsigned prec = TYPE_PRECISION (expr_type);

>     return (type == VR_RANGE

> +         && TYPE_UNSIGNED (expr_type)

>            && wi::eq_p (min, wi::one (prec))

>            && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN (expr_type))));

>   }

>
Aldy Hernandez Oct. 10, 2019, 3:18 p.m. | #7
It's not that particular commit, but the previous one, but yeah, that's 
all me.

(gdb)
#4  0x000000000145bf44 in value_range_base::lower_bound 
(this=0x7fffffffc360,
     pair=0) at /home/cygnus/aldyh/src/gcc/gcc/tree-vrp.c:6148
6148      return wi::to_wide (t);
(gdb)
#5  0x0000000001ccb1d9 in range_operator::fold_range (
     this=0x271edc8 <op_plus>, type=<integer_type 0x7fffefd451f8>, lh=...,
     rh=...) at /home/cygnus/aldyh/src/gcc/gcc/range-op.cc:156
156             wide_int rh_lb = rh.lower_bound (y);
(gdb)
#6  0x000000000144b850 in range_fold_binary_expr (vr=0x7fffffffc6c0,
     code=PLUS_EXPR, expr_type=<integer_type 0x7fffefd451f8>,
     vr0_=0x7fffffffc480, vr1_=0x7fffffffc460)
     at /home/cygnus/aldyh/src/gcc/gcc/tree-vrp.c:1927
1927                            vr1.normalize_addresses ());
(gdb) l
1922      if (range_fold_binary_symbolics_p (vr, code, expr_type, &vr0, 
&vr1))
1923        return;
1924
1925      *vr = op->fold_range (expr_type,
1926                            vr0.normalize_addresses (),
1927                            vr1.normalize_addresses ());
1928    }
1929
1930    /* Perform a unary operation on a range.  */
1931
(gdb) print vr1.dump()
unsigned long [POLY_INT_CST [4, 4], POLY_INT_CST [4, 4]]$1 = void
(gdb)

We have a value_range_base containing a POLYT_INT_CST which we don't handle.

That's rather unfortunate.  I was hoping by this point, anything we 
couldn't handle was just an address.  The previous range-ops code was 
normalizing anything not INTEGER_CST into VARYING, which I thought was a 
rather big hammer, and was trying to avoid.

Interesting that the old VRP code also allowing the POLY_INT to pass on 
through all the way into extract_range_binary_expr, only to bail when it 
see's it's not an INTEGER_CST:

           && (TREE_CODE (max_op0) == INTEGER_CST
               || (sym_max_op0
                   = get_single_symbol (max_op0, &neg_max_op0, &max_op0)))
           && (TREE_CODE (max_op1) == INTEGER_CST
               || (sym_max_op1
                   = get_single_symbol (max_op1, &neg_max_op1, &max_op1)))

I guess we could bring out the big hammer again if normalize_addresses 
doesn't give us an INTEGER_CST.

I'll take a look at this.  Thanks for pointing it out.

Aldy

On 10/10/19 5:25 AM, Christophe Lyon wrote:
> 

> 

> On Mon, 7 Oct 2019 at 13:53, Aldy Hernandez <aldyh@redhat.com 

> <mailto:aldyh@redhat.com>> wrote:

> 

> 

>      > +bool

>      > +ipa_vr::nonzero_p (tree expr_type) const

>      > +{

>      > +  if (type == VR_ANTI_RANGE && wi::eq_p (min, 0) && wi::eq_p

>     (max, 0))

>      > +    return true;

>      > +

>      > +  unsigned prec = TYPE_PRECISION (expr_type);

>      > +  return (type == VR_RANGE

>      > +         && wi::eq_p (min, wi::one (prec))

>      > +         && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN

>     (expr_type))));

>      > +}

> 

>     Errr, wrong version posted.  There was a TYPE_UNSIGNED missing.

> 

>     Fixed and committed.

> 

> 

> Hi,

> Since this was committed (r276654), I've noticed regressions on aarch64:

>      gcc.target/aarch64/pr88838.c (test for excess errors)

>      gcc.target/aarch64/stack-check-cfa-3.c (test for excess errors)

>      gcc.target/aarch64/stack-check-prologue-16.c (test for excess errors)

>      gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve 

>   scan-assembler-times \\tabs\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b\\n 1

>      gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve 

>   scan-assembler-times \\tabs\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d\\n 1

>      gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve 

>   scan-assembler-times \\tabs\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h\\n 1

>      gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve 

>   scan-assembler-times \\tabs\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s\\n 1

>      gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve (test for 

> excess errors)

>      gcc.target/aarch64/sve/adr_1.c -march=armv8.2-a+sve (test for 

> excess errors)

> 

> and many others

> 

> For instance:

> compiler exited with status 1

> FAIL: gcc.target/aarch64/pr88838.c (internal compiler error)

> FAIL: gcc.target/aarch64/pr88838.c (test for excess errors)

> Excess errors:

> during GIMPLE pass: dom

> /gcc/testsuite/gcc.target/aarch64/pr88838.c:5:1: internal compiler 

> error: tree check: expected integer_cst, have poly_int_cst in to_wide, 

> at tree.h:5795

> 0x5efa71 tree_check_failed(tree_node const*, char const*, int, char 

> const*, ...)

>          /gcc/tree.c:9926

> 0x749584 tree_check(tree_node const*, char const*, int, char const*, 

> tree_code)

>          /gcc/tree.h:3523

> 0x749584 wi::to_wide(tree_node const*)

>          /gcc/tree.h:5795

> 0xf7c0c0 value_range_base::lower_bound(unsigned int) const

>          /gcc/tree-vrp.c:6136

> 0x155d2e6 range_operator::fold_range(tree_node*, value_range_base 

> const&, value_range_base const&) const

>          /gcc/range-op.cc:156

> 0xf87597 range_fold_binary_expr(value_range_base*, tree_code, 

> tree_node*, value_range_base const*, value_range_base const*)

>          /gcc/tree-vrp.c:1915

> 0x1007962 vr_values::extract_range_from_binary_expr(value_range*, 

> tree_code, tree_node*, tree_node*, tree_node*)

>          /gcc/vr-values.c:808

> 0x1011f0c vr_values::extract_range_from_assignment(value_range*, gassign*)

>          /gcc/vr-values.c:1469

> 0x1499d21 evrp_range_analyzer::record_ranges_from_stmt(gimple*, bool)

>          /gcc/gimple-ssa-evrp-analyze.c:307

> 0xdd76dd dom_opt_dom_walker::before_dom_children(basic_block_def*)

>          /gcc/tree-ssa-dom.c:1503

> 0x146e68a dom_walker::walk(basic_block_def*)

>          /gcc/domwalk.c:309

> 0xdd4759 execute

>          /gcc/tree-ssa-dom.c:724

> 

> gcc.target/aarch64/pr88838.c: output file does not exist

> UNRESOLVED: gcc.target/aarch64/pr88838.c scan-assembler-not sxtw

> 

> Christophe

> 

>     diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c

>     index 20a0bddcbab..5020f4a44d5 100644

>     --- a/gcc/ipa-prop.c

>     +++ b/gcc/ipa-prop.c

>     @@ -5117,6 +5117,7 @@ ipa_vr::nonzero_p (tree expr_type) const

> 

>          unsigned prec = TYPE_PRECISION (expr_type);

>          return (type == VR_RANGE

>     +         && TYPE_UNSIGNED (expr_type)

>                 && wi::eq_p (min, wi::one (prec))

>                 && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN

>     (expr_type))));

>        }

>
Christophe Lyon Oct. 10, 2019, 5:20 p.m. | #8
On Thu, 10 Oct 2019 at 17:19, Aldy Hernandez <aldyh@redhat.com> wrote:

> It's not that particular commit, but the previous one, but yeah, that's

> all me.


Interesting, bisect really identified r276654.

I think that's what Steve reported as PR 92051 (although his backtrace is
different, but it's also for a different testcase).




> (gdb)

> #4  0x000000000145bf44 in value_range_base::lower_bound

> (this=0x7fffffffc360,

>      pair=0) at /home/cygnus/aldyh/src/gcc/gcc/tree-vrp.c:6148

> 6148      return wi::to_wide (t);

> (gdb)

> #5  0x0000000001ccb1d9 in range_operator::fold_range (

>      this=0x271edc8 <op_plus>, type=<integer_type 0x7fffefd451f8>, lh=...,

>      rh=...) at /home/cygnus/aldyh/src/gcc/gcc/range-op.cc:156

> 156             wide_int rh_lb = rh.lower_bound (y);

> (gdb)

> #6  0x000000000144b850 in range_fold_binary_expr (vr=0x7fffffffc6c0,

>      code=PLUS_EXPR, expr_type=<integer_type 0x7fffefd451f8>,

>      vr0_=0x7fffffffc480, vr1_=0x7fffffffc460)

>      at /home/cygnus/aldyh/src/gcc/gcc/tree-vrp.c:1927

> 1927                            vr1.normalize_addresses ());

> (gdb) l

> 1922      if (range_fold_binary_symbolics_p (vr, code, expr_type, &vr0,

> &vr1))

> 1923        return;

> 1924

> 1925      *vr = op->fold_range (expr_type,

> 1926                            vr0.normalize_addresses (),

> 1927                            vr1.normalize_addresses ());

> 1928    }

> 1929

> 1930    /* Perform a unary operation on a range.  */

> 1931

> (gdb) print vr1.dump()

> unsigned long [POLY_INT_CST [4, 4], POLY_INT_CST [4, 4]]$1 = void

> (gdb)

>

> We have a value_range_base containing a POLYT_INT_CST which we don't

> handle.

>

> That's rather unfortunate.  I was hoping by this point, anything we

> couldn't handle was just an address.  The previous range-ops code was

> normalizing anything not INTEGER_CST into VARYING, which I thought was a

> rather big hammer, and was trying to avoid.

>

> Interesting that the old VRP code also allowing the POLY_INT to pass on

> through all the way into extract_range_binary_expr, only to bail when it

> see's it's not an INTEGER_CST:

>

>            && (TREE_CODE (max_op0) == INTEGER_CST

>                || (sym_max_op0

>                    = get_single_symbol (max_op0, &neg_max_op0, &max_op0)))

>            && (TREE_CODE (max_op1) == INTEGER_CST

>                || (sym_max_op1

>                    = get_single_symbol (max_op1, &neg_max_op1, &max_op1)))

>

> I guess we could bring out the big hammer again if normalize_addresses

> doesn't give us an INTEGER_CST.

>

> I'll take a look at this.  Thanks for pointing it out.

>

> Thanks,


Christophe


> Aldy

>

> On 10/10/19 5:25 AM, Christophe Lyon wrote:

> >

> >

> > On Mon, 7 Oct 2019 at 13:53, Aldy Hernandez <aldyh@redhat.com

> > <mailto:aldyh@redhat.com>> wrote:

> >

> >

> >      > +bool

> >      > +ipa_vr::nonzero_p (tree expr_type) const

> >      > +{

> >      > +  if (type == VR_ANTI_RANGE && wi::eq_p (min, 0) && wi::eq_p

> >     (max, 0))

> >      > +    return true;

> >      > +

> >      > +  unsigned prec = TYPE_PRECISION (expr_type);

> >      > +  return (type == VR_RANGE

> >      > +         && wi::eq_p (min, wi::one (prec))

> >      > +         && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN

> >     (expr_type))));

> >      > +}

> >

> >     Errr, wrong version posted.  There was a TYPE_UNSIGNED missing.

> >

> >     Fixed and committed.

> >

> >

> > Hi,

> > Since this was committed (r276654), I've noticed regressions on aarch64:

> >      gcc.target/aarch64/pr88838.c (test for excess errors)

> >      gcc.target/aarch64/stack-check-cfa-3.c (test for excess errors)

> >      gcc.target/aarch64/stack-check-prologue-16.c (test for excess

> errors)

> >      gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve

> >   scan-assembler-times \\tabs\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b\\n 1

> >      gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve

> >   scan-assembler-times \\tabs\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d\\n 1

> >      gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve

> >   scan-assembler-times \\tabs\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h\\n 1

> >      gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve

> >   scan-assembler-times \\tabs\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s\\n 1

> >      gcc.target/aarch64/sve/abs_1.c -march=armv8.2-a+sve (test for

> > excess errors)

> >      gcc.target/aarch64/sve/adr_1.c -march=armv8.2-a+sve (test for

> > excess errors)

> >

> > and many others

> >

> > For instance:

> > compiler exited with status 1

> > FAIL: gcc.target/aarch64/pr88838.c (internal compiler error)

> > FAIL: gcc.target/aarch64/pr88838.c (test for excess errors)

> > Excess errors:

> > during GIMPLE pass: dom

> > /gcc/testsuite/gcc.target/aarch64/pr88838.c:5:1: internal compiler

> > error: tree check: expected integer_cst, have poly_int_cst in to_wide,

> > at tree.h:5795

> > 0x5efa71 tree_check_failed(tree_node const*, char const*, int, char

> > const*, ...)

> >          /gcc/tree.c:9926

> > 0x749584 tree_check(tree_node const*, char const*, int, char const*,

> > tree_code)

> >          /gcc/tree.h:3523

> > 0x749584 wi::to_wide(tree_node const*)

> >          /gcc/tree.h:5795

> > 0xf7c0c0 value_range_base::lower_bound(unsigned int) const

> >          /gcc/tree-vrp.c:6136

> > 0x155d2e6 range_operator::fold_range(tree_node*, value_range_base

> > const&, value_range_base const&) const

> >          /gcc/range-op.cc:156

> > 0xf87597 range_fold_binary_expr(value_range_base*, tree_code,

> > tree_node*, value_range_base const*, value_range_base const*)

> >          /gcc/tree-vrp.c:1915

> > 0x1007962 vr_values::extract_range_from_binary_expr(value_range*,

> > tree_code, tree_node*, tree_node*, tree_node*)

> >          /gcc/vr-values.c:808

> > 0x1011f0c vr_values::extract_range_from_assignment(value_range*,

> gassign*)

> >          /gcc/vr-values.c:1469

> > 0x1499d21 evrp_range_analyzer::record_ranges_from_stmt(gimple*, bool)

> >          /gcc/gimple-ssa-evrp-analyze.c:307

> > 0xdd76dd dom_opt_dom_walker::before_dom_children(basic_block_def*)

> >          /gcc/tree-ssa-dom.c:1503

> > 0x146e68a dom_walker::walk(basic_block_def*)

> >          /gcc/domwalk.c:309

> > 0xdd4759 execute

> >          /gcc/tree-ssa-dom.c:724

> >

> > gcc.target/aarch64/pr88838.c: output file does not exist

> > UNRESOLVED: gcc.target/aarch64/pr88838.c scan-assembler-not sxtw

> >

> > Christophe

> >

> >     diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c

> >     index 20a0bddcbab..5020f4a44d5 100644

> >     --- a/gcc/ipa-prop.c

> >     +++ b/gcc/ipa-prop.c

> >     @@ -5117,6 +5117,7 @@ ipa_vr::nonzero_p (tree expr_type) const

> >

> >          unsigned prec = TYPE_PRECISION (expr_type);

> >          return (type == VR_RANGE

> >     +         && TYPE_UNSIGNED (expr_type)

> >                 && wi::eq_p (min, wi::one (prec))

> >                 && wi::eq_p (max, wi::max_value (prec, TYPE_SIGN

> >     (expr_type))));

> >        }

> >

>

Patch

commit 70d336374fb39e857ecbd720900df9869c3c2ce9
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Thu Oct 3 14:53:12 2019 +0200

    Disentangle range_fold_*ary_expr() into various independent pieces.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 3934b41fdf9..714b22781ee 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,17 @@ 
+2019-10-04  Aldy Hernandez  <aldyh@redhat.com>
+
+	* tree-vrp.c (get_range_op_handler): New.
+	(supported_types_p): New.
+	(defined_ranges_p): New.
+	(drop_undefines_to_varying): New.
+	(range_fold_binary_symbolics_p): New.
+	(range_fold_unary_symbolics_p): New.
+	(range_fold_unary_expr): Extract out into above functions.
+	(range_fold_binary_expr): Same.
+	(value_range_base::normalize_addresses): New.
+	(value_range_base::normalize_symbolics): Normalize addresses.
+	* tree-vrp.h (class value_range_base): Add normalize_addresses.
+
 2019-10-04  Aldy Hernandez  <aldyh@redhat.com>
 
 	* tree-vrp.c (value_range_base::singleton_p): Use num_pairs
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 97dd2b7a734..4370c46e966 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -1778,119 +1778,102 @@  extract_range_from_plus_minus_expr (value_range_base *vr,
     vr->set (kind, min, max);
 }
 
-/* Normalize a value_range for use in range_ops and return it.  */
+/* Return the range-ops handler for CODE and EXPR_TYPE.  If no
+   suitable operator is found, return NULL and set VR to VARYING.  */
 
-static value_range_base
-normalize_for_range_ops (const value_range_base &vr)
+static const range_operator *
+get_range_op_handler (value_range_base *vr,
+		      enum tree_code code,
+		      tree expr_type)
 {
-  tree type = vr.type ();
+  const range_operator *op = range_op_handler (code, expr_type);
+  if (!op)
+    vr->set_varying (expr_type);
+  return op;
+}
+
+/* If the types passed are supported, return TRUE, otherwise set VR to
+   VARYING and return FALSE.  */
 
-  /* This will return ~[0,0] for [&var, &var].  */
-  if (POINTER_TYPE_P (type) && !range_includes_zero_p (&vr))
+static bool
+supported_types_p (value_range_base *vr,
+		   tree type0,
+		   tree type1 = NULL)
+{
+  if (!value_range_base::supports_type_p (type0)
+      || (type1 && !value_range_base::supports_type_p (type1)))
     {
-      value_range_base temp;
-      temp.set_nonzero (type);
-      return temp;
+      vr->set_varying (type0);
+      return false;
     }
-  if (vr.symbolic_p ())
-    return normalize_for_range_ops (vr.normalize_symbolics ());
-  if (TREE_CODE (vr.min ()) == INTEGER_CST
-      && TREE_CODE (vr.max ()) == INTEGER_CST)
-    return vr;
-  /* Anything not strictly numeric at this point becomes varying.  */
-  return value_range_base (vr.type ());
+  return true;
 }
 
-/* Fold a binary expression of two value_range's with range-ops.  */
+/* If any of the ranges passed are defined, return TRUE, otherwise set
+   VR to UNDEFINED and return FALSE.  */
 
-void
-range_fold_binary_expr (value_range_base *vr,
-			enum tree_code code,
-			tree expr_type,
-			const value_range_base *vr0_,
-			const value_range_base *vr1_)
+static bool
+defined_ranges_p (value_range_base *vr,
+		  const value_range_base *vr0,
+		  const value_range_base *vr1 = NULL)
 {
-  if (!value_range_base::supports_type_p (expr_type)
-      || (!vr0_->undefined_p ()
-	  && !value_range_base::supports_type_p (vr0_->type ()))
-      || (!vr1_->undefined_p ()
-	  && !value_range_base::supports_type_p (vr1_->type ())))
-    {
-      vr->set_varying (expr_type);
-      return;
-    }
-  if (vr0_->undefined_p () && vr1_->undefined_p ())
+  if (vr0->undefined_p () && (!vr1 || vr1->undefined_p ()))
     {
       vr->set_undefined ();
-      return;
-    }
-  range_operator *op = range_op_handler (code, expr_type);
-  if (!op)
-    {
-      vr->set_varying (expr_type);
-      return;
+      return false;
     }
+  return true;
+}
 
-  /* Mimic any behavior users of extract_range_from_binary_expr may
-     expect.  */
-  value_range_base vr0 = *vr0_, vr1 = *vr1_;
-  if (vr0.undefined_p ())
-    vr0.set_varying (expr_type);
-  else if (vr1.undefined_p ())
-    vr1.set_varying (expr_type);
+static value_range_base
+drop_undefines_to_varying (const value_range_base *vr, tree expr_type)
+{
+  if (vr->undefined_p ())
+    return value_range_base (expr_type);
+  else
+    return *vr;
+}
+
+/* If any operand is symbolic, perform a binary operation on them and
+   return TRUE, otherwise return FALSE.  */
 
-  /* Handle symbolics.  */
-  if (vr0.symbolic_p () || vr1.symbolic_p ())
+static bool
+range_fold_binary_symbolics_p (value_range_base *vr,
+			       tree_code code,
+			       tree expr_type,
+			       const value_range_base *vr0,
+			       const value_range_base *vr1)
+{
+  if (vr0->symbolic_p () || vr1->symbolic_p ())
     {
       if ((code == PLUS_EXPR || code == MINUS_EXPR))
 	{
-	  extract_range_from_plus_minus_expr (vr, code, expr_type,
-					      &vr0, &vr1);
-	  return;
+	  extract_range_from_plus_minus_expr (vr, code, expr_type, vr0, vr1);
+	  return true;
 	}
       if (POINTER_TYPE_P (expr_type) && code == POINTER_PLUS_EXPR)
 	{
-	  extract_range_from_pointer_plus_expr (vr, code, expr_type,
-						&vr0, &vr1);
-	  return;
+	  extract_range_from_pointer_plus_expr (vr, code, expr_type, vr0, vr1);
+	  return true;
 	}
+      const range_operator *op = get_range_op_handler (vr, code, expr_type);
+      *vr = op->fold_range (expr_type,
+			    vr0->normalize_symbolics (),
+			    vr1->normalize_symbolics ());
+      return true;
     }
-
-  /* Do the range-ops dance.  */
-  value_range_base n0 = normalize_for_range_ops (vr0);
-  value_range_base n1 = normalize_for_range_ops (vr1);
-  *vr = op->fold_range (expr_type, n0, n1);
+  return false;
 }
 
-/* Fold a unary expression of a value_range with range-ops.  */
+/* If operand is symbolic, perform a unary operation on it and return
+   TRUE, otherwise return FALSE.  */
 
-void
-range_fold_unary_expr (value_range_base *vr,
-		       enum tree_code code, tree expr_type,
-		       const value_range_base *vr0,
-		       tree vr0_type)
+static bool
+range_fold_unary_symbolics_p (value_range_base *vr,
+			      tree_code code,
+			      tree expr_type,
+			      const value_range_base *vr0)
 {
-  /* Mimic any behavior users of extract_range_from_unary_expr may
-     expect.  */
-  if (!value_range_base::supports_type_p (expr_type)
-      || !value_range_base::supports_type_p (vr0_type))
-    {
-      vr->set_varying (expr_type);
-      return;
-    }
-  if (vr0->undefined_p ())
-    {
-      vr->set_undefined ();
-      return;
-    }
-  range_operator *op = range_op_handler (code, expr_type);
-  if (!op)
-    {
-      vr->set_varying (expr_type);
-      return;
-    }
-
-  /* Handle symbolics.  */
   if (vr0->symbolic_p ())
     {
       if (code == NEGATE_EXPR)
@@ -1899,7 +1882,7 @@  range_fold_unary_expr (value_range_base *vr,
 	  value_range_base zero;
 	  zero.set_zero (vr0->type ());
 	  range_fold_binary_expr (vr, MINUS_EXPR, expr_type, &zero, vr0);
-	  return;
+	  return true;
 	}
       if (code == BIT_NOT_EXPR)
 	{
@@ -1907,30 +1890,64 @@  range_fold_unary_expr (value_range_base *vr,
 	  value_range_base minusone;
 	  minusone.set (build_int_cst (vr0->type (), -1));
 	  range_fold_binary_expr (vr, MINUS_EXPR, expr_type, &minusone, vr0);
-	  return;
+	  return true;
 	}
+      const range_operator *op = get_range_op_handler (vr, code, expr_type);
       *vr = op->fold_range (expr_type,
-			    normalize_for_range_ops (*vr0),
+			    vr0->normalize_symbolics (),
 			    value_range_base (expr_type));
-      return;
-    }
-  if (CONVERT_EXPR_CODE_P (code) && (POINTER_TYPE_P (expr_type)
-				     || POINTER_TYPE_P (vr0->type ())))
-    {
-      /* This handles symbolic conversions such such as [25, x_4].  */
-      if (!range_includes_zero_p (vr0))
-	vr->set_nonzero (expr_type);
-      else if (vr0->zero_p ())
-	vr->set_zero (expr_type);
-      else
-	vr->set_varying (expr_type);
-      return;
+      return true;
     }
+  return false;
+}
+
+/* Perform a binary operation on a pair of ranges.  */
 
-  /* Do the range-ops dance.  */
-  value_range_base n0 = normalize_for_range_ops (*vr0);
-  value_range_base n1 (expr_type);
-  *vr = op->fold_range (expr_type, n0, n1);
+void
+range_fold_binary_expr (value_range_base *vr,
+			enum tree_code code,
+			tree expr_type,
+			const value_range_base *vr0_,
+			const value_range_base *vr1_)
+{
+  if (!supported_types_p (vr, expr_type)
+      || !defined_ranges_p (vr, vr0_, vr1_))
+    return;
+  const range_operator *op = get_range_op_handler (vr, code, expr_type);
+  if (!op)
+    return;
+
+  value_range_base vr0 = drop_undefines_to_varying (vr0_, expr_type);
+  value_range_base vr1 = drop_undefines_to_varying (vr1_, expr_type);
+  if (range_fold_binary_symbolics_p (vr, code, expr_type, &vr0, &vr1))
+    return;
+
+  *vr = op->fold_range (expr_type,
+			vr0.normalize_addresses (),
+			vr1.normalize_addresses ());
+}
+
+/* Perform a unary operation on a range.  */
+
+void
+range_fold_unary_expr (value_range_base *vr,
+		       enum tree_code code, tree expr_type,
+		       const value_range_base *vr0,
+		       tree vr0_type)
+{
+  if (!supported_types_p (vr, expr_type, vr0_type)
+      || !defined_ranges_p (vr, vr0))
+    return;
+  const range_operator *op = get_range_op_handler (vr, code, expr_type);
+  if (!op)
+    return;
+
+  if (range_fold_unary_symbolics_p (vr, code, expr_type, vr0))
+    return;
+
+  *vr = op->fold_range (expr_type,
+			vr0->normalize_addresses (),
+			value_range_base (expr_type));
 }
 
 /* Given a COND_EXPR COND of the form 'V OP W', and an SSA name V,
@@ -6015,7 +6032,24 @@  value_range::union_ (const value_range *other)
     }
 }
 
-/* Normalize symbolics into constants.  */
+/* Normalize addresses into constants.  */
+
+value_range_base
+value_range_base::normalize_addresses () const
+{
+  if (!POINTER_TYPE_P (type ()) || range_has_numeric_bounds_p (this))
+    return *this;
+
+  if (!range_includes_zero_p (this))
+    {
+      gcc_checking_assert (TREE_CODE (m_min) == ADDR_EXPR
+			   || TREE_CODE (m_max) == ADDR_EXPR);
+      return range_nonzero (type ());
+    }
+  return value_range_base (type ());
+}
+
+/* Normalize symbolics and addresses into constants.  */
 
 value_range_base
 value_range_base::normalize_symbolics () const
@@ -6026,7 +6060,7 @@  value_range_base::normalize_symbolics () const
   bool min_symbolic = !is_gimple_min_invariant (min ());
   bool max_symbolic = !is_gimple_min_invariant (max ());
   if (!min_symbolic && !max_symbolic)
-    return *this;
+    return normalize_addresses ();
 
   // [SYM, SYM] -> VARYING
   if (min_symbolic && max_symbolic)
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
index d20d0043ba3..a3f9e90699d 100644
--- a/gcc/tree-vrp.h
+++ b/gcc/tree-vrp.h
@@ -86,6 +86,7 @@  public:
 
   static bool supports_type_p (tree);
   value_range_base normalize_symbolics () const;
+  value_range_base normalize_addresses () const;
 
   static const unsigned int m_max_pairs = 2;
   bool contains_p (tree) const;