Add a constant_range_value_p function (PR 92033)

Add a constant_range_value_p function (PR 92033)

[Richard Sandiford]

The range-tracking code has a pretty hard-coded assumption that
is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
ADDR_EXPR".  It seems better to add a predicate specifically for
that rather than contiually fight cases in which it can't handle
other invariants.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Richard


2019-10-11  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	PR tree-optimization/92033
	* tree-vrp.h (constant_range_value_p): Declare.
	* tree-vrp.c (constant_range_value_p): New function.
	(value_range_base::symbolic_p, value_range_base::singleton_p)
	(get_single_symbol, compare_values_warnv, intersect_ranges)
	(value_range_base::normalize_symbolics): Use it instead of
	is_gimple_min_invariant.
	(simplify_stmt_for_jump_threading): Likewise.
	* vr-values.c (symbolic_range_based_on_p, valid_value_p): Likewise.
	(vr_values::op_with_constant_singleton_value_range): Likewise.
	(vr_values::extract_range_from_binary_expr): Likewise.
	(vr_values::extract_range_from_unary_expr): Likewise.
	(vr_values::extract_range_from_cond_expr): Likewise.
	(vr_values::extract_range_from_comparison): Likewise.
	(vr_values::extract_range_from_assignment): Likewise.
	(vr_values::adjust_range_with_scev, vrp_valueize): Likewise.
	(vr_values::vrp_visit_assignment_or_call): Likewise.
	(vr_values::vrp_evaluate_conditional): Likewise.
	(vr_values::simplify_bit_ops_using_ranges): Likewise.
	(test_for_singularity): Likewise.
	(vr_values::simplify_cond_using_ranges_1): Likewise.

Index: gcc/tree-vrp.h
===================================================================
--- gcc/tree-vrp.h	2019-10-08 09:23:31.282533990 +0100
+++ gcc/tree-vrp.h	2019-10-11 15:41:20.380576059 +0100
@@ -284,6 +284,7 @@ value_range_base::supports_type_p (tree
   return false;
 }
 
+extern bool constant_range_value_p (const_tree);
 extern void register_edge_assert_for (tree, edge, enum tree_code,
 				      tree, tree, vec<assert_info> &);
 extern bool stmt_interesting_for_vrp (gimple *);
Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	2019-10-08 09:23:31.282533990 +0100
+++ gcc/tree-vrp.c	2019-10-11 15:41:20.380576059 +0100
@@ -78,6 +78,18 @@ ranges_from_anti_range (const value_rang
    for still active basic-blocks.  */
 static sbitmap *live;
 
+/* Return true if VALUE is considered constant for range tracking.
+   This is stricter than is_gimple_min_invariant and should be
+   used instead of it in range-related code.  */
+
+bool
+constant_range_value_p (const_tree value)
+{
+  return (TREE_CODE (value) == INTEGER_CST
+	  || (TREE_CODE (value) == ADDR_EXPR
+	      && is_gimple_invariant_address (value)));
+}
+
 void
 value_range::set_equiv (bitmap equiv)
 {
@@ -273,8 +285,8 @@ value_range_base::symbolic_p () const
 {
   return (!varying_p ()
 	  && !undefined_p ()
-	  && (!is_gimple_min_invariant (m_min)
-	      || !is_gimple_min_invariant (m_max)));
+	  && (!constant_range_value_p (m_min)
+	      || !constant_range_value_p (m_max)));
 }
 
 /* NOTE: This is not the inverse of symbolic_p because the range
@@ -388,7 +400,7 @@ value_range_base::singleton_p (tree *res
     }
   if (m_kind == VR_RANGE
       && vrp_operand_equal_p (min (), max ())
-      && is_gimple_min_invariant (min ()))
+      && constant_range_value_p (min ()))
     {
       if (result)
         *result = min ();
@@ -953,13 +965,13 @@ get_single_symbol (tree t, bool *neg, tr
       || TREE_CODE (t) == POINTER_PLUS_EXPR
       || TREE_CODE (t) == MINUS_EXPR)
     {
-      if (is_gimple_min_invariant (TREE_OPERAND (t, 0)))
+      if (constant_range_value_p (TREE_OPERAND (t, 0)))
 	{
 	  neg_ = (TREE_CODE (t) == MINUS_EXPR);
 	  inv_ = TREE_OPERAND (t, 0);
 	  t = TREE_OPERAND (t, 1);
 	}
-      else if (is_gimple_min_invariant (TREE_OPERAND (t, 1)))
+      else if (constant_range_value_p (TREE_OPERAND (t, 1)))
 	{
 	  neg_ = false;
 	  inv_ = TREE_OPERAND (t, 1);
@@ -1106,8 +1118,8 @@ compare_values_warnv (tree val1, tree va
 		      TYPE_SIGN (TREE_TYPE (val1)));
     }
 
-  const bool cst1 = is_gimple_min_invariant (val1);
-  const bool cst2 = is_gimple_min_invariant (val2);
+  const bool cst1 = constant_range_value_p (val1);
+  const bool cst2 = constant_range_value_p (val2);
 
   /* If one is of the form '[-]NAME + CST' and the other is constant, then
      it might be possible to say something depending on the constants.  */
@@ -5785,7 +5797,7 @@ intersect_ranges (enum value_range_kind
      correct estimate unless VR1 is a constant singleton range
      in which case we choose that.  */
   if (vr1type == VR_RANGE
-      && is_gimple_min_invariant (vr1min)
+      && constant_range_value_p (vr1min)
       && vrp_operand_equal_p (vr1min, vr1max))
     {
       *vr0type = vr1type;
@@ -6045,8 +6057,8 @@ value_range_base::normalize_symbolics ()
   if (varying_p () || undefined_p ())
     return *this;
   tree ttype = type ();
-  bool min_symbolic = !is_gimple_min_invariant (min ());
-  bool max_symbolic = !is_gimple_min_invariant (max ());
+  bool min_symbolic = !constant_range_value_p (min ());
+  bool max_symbolic = !constant_range_value_p (max ());
   if (!min_symbolic && !max_symbolic)
     return normalize_addresses ();
 
@@ -6432,7 +6444,7 @@ simplify_stmt_for_jump_threading (gimple
 {
   /* First see if the conditional is in the hash table.  */
   tree cached_lhs = avail_exprs_stack->lookup_avail_expr (stmt, false, true);
-  if (cached_lhs && is_gimple_min_invariant (cached_lhs))
+  if (cached_lhs && constant_range_value_p (cached_lhs))
     return cached_lhs;
 
   vr_values *vr_values = x_vr_values;
Index: gcc/vr-values.c
===================================================================
--- gcc/vr-values.c	2019-10-03 14:04:54.161520173 +0100
+++ gcc/vr-values.c	2019-10-11 15:41:20.380576059 +0100
@@ -263,14 +263,14 @@ symbolic_range_based_on_p (value_range_b
   bool neg, min_has_symbol, max_has_symbol;
   tree inv;
 
-  if (is_gimple_min_invariant (vr->min ()))
+  if (constant_range_value_p (vr->min ()))
     min_has_symbol = false;
   else if (get_single_symbol (vr->min (), &neg, &inv) == sym)
     min_has_symbol = true;
   else
     return false;
 
-  if (is_gimple_min_invariant (vr->max ()))
+  if (constant_range_value_p (vr->max ()))
     max_has_symbol = false;
   else if (get_single_symbol (vr->max (), &neg, &inv) == sym)
     max_has_symbol = true;
@@ -409,7 +409,7 @@ valid_value_p (tree expr)
     return (TREE_CODE (TREE_OPERAND (expr, 0)) == SSA_NAME
 	    && TREE_CODE (TREE_OPERAND (expr, 1)) == INTEGER_CST);
 
-  return is_gimple_min_invariant (expr);
+  return constant_range_value_p (expr);
 }
 
 /* If OP has a value range with a single constant value return that,
@@ -419,7 +419,7 @@ valid_value_p (tree expr)
 tree
 vr_values::op_with_constant_singleton_value_range (tree op)
 {
-  if (is_gimple_min_invariant (op))
+  if (constant_range_value_p (op))
     return op;
 
   if (TREE_CODE (op) != SSA_NAME)
@@ -778,14 +778,14 @@ vr_values::extract_range_from_binary_exp
   value_range_base vr0, vr1;
   if (TREE_CODE (op0) == SSA_NAME)
     vr0 = *(get_value_range (op0));
-  else if (is_gimple_min_invariant (op0))
+  else if (constant_range_value_p (op0))
     vr0.set (op0);
   else
     vr0.set_varying (TREE_TYPE (op0));
 
   if (TREE_CODE (op1) == SSA_NAME)
     vr1 = *(get_value_range (op1));
-  else if (is_gimple_min_invariant (op1))
+  else if (constant_range_value_p (op1))
     vr1.set (op1);
   else
     vr1.set_varying (TREE_TYPE (op1));
@@ -855,11 +855,11 @@ vr_values::extract_range_from_binary_exp
       value_range n_vr1;
 
       /* Try with VR0 and [-INF, OP1].  */
-      if (is_gimple_min_invariant (minus_p ? vr0.max () : vr0.min ()))
+      if (constant_range_value_p (minus_p ? vr0.max () : vr0.min ()))
 	n_vr1.set (VR_RANGE, vrp_val_min (expr_type), op1);
 
       /* Try with VR0 and [OP1, +INF].  */
-      else if (is_gimple_min_invariant (minus_p ? vr0.min () : vr0.max ()))
+      else if (constant_range_value_p (minus_p ? vr0.min () : vr0.max ()))
 	n_vr1.set (VR_RANGE, op1, vrp_val_max (expr_type));
 
       /* Try with VR0 and [OP1, OP1].  */
@@ -879,11 +879,11 @@ vr_values::extract_range_from_binary_exp
       value_range n_vr0;
 
       /* Try with [-INF, OP0] and VR1.  */
-      if (is_gimple_min_invariant (minus_p ? vr1.max () : vr1.min ()))
+      if (constant_range_value_p (minus_p ? vr1.max () : vr1.min ()))
 	n_vr0.set (VR_RANGE, vrp_val_min (expr_type), op0);
 
       /* Try with [OP0, +INF] and VR1.  */
-      else if (is_gimple_min_invariant (minus_p ? vr1.min (): vr1.max ()))
+      else if (constant_range_value_p (minus_p ? vr1.min (): vr1.max ()))
 	n_vr0.set (VR_RANGE, op0, vrp_val_max (expr_type));
 
       /* Try with [OP0, OP0] and VR1.  */
@@ -926,7 +926,7 @@ vr_values::extract_range_from_unary_expr
      a new value range with the operand to simplify processing.  */
   if (TREE_CODE (op0) == SSA_NAME)
     vr0 = *(get_value_range (op0));
-  else if (is_gimple_min_invariant (op0))
+  else if (constant_range_value_p (op0))
     vr0.set (op0);
   else
     vr0.set_varying (type);
@@ -948,7 +948,7 @@ vr_values::extract_range_from_cond_expr
   const value_range *vr0 = &tem0;
   if (TREE_CODE (op0) == SSA_NAME)
     vr0 = get_value_range (op0);
-  else if (is_gimple_min_invariant (op0))
+  else if (constant_range_value_p (op0))
     tem0.set (op0);
   else
     tem0.set_varying (TREE_TYPE (op0));
@@ -958,7 +958,7 @@ vr_values::extract_range_from_cond_expr
   const value_range *vr1 = &tem1;
   if (TREE_CODE (op1) == SSA_NAME)
     vr1 = get_value_range (op1);
-  else if (is_gimple_min_invariant (op1))
+  else if (constant_range_value_p (op1))
     tem1.set (op1);
   else
     tem1.set_varying (TREE_TYPE (op1));
@@ -987,7 +987,7 @@ vr_values::extract_range_from_comparison
 	 its type may be different from _Bool.  Convert VAL to EXPR's
 	 type.  */
       val = fold_convert (type, val);
-      if (is_gimple_min_invariant (val))
+      if (constant_range_value_p (val))
 	vr->set (val);
       else
 	vr->update (VR_RANGE, val, val);
@@ -1479,7 +1479,7 @@ vr_values::extract_range_from_assignment
 				   gimple_assign_rhs1 (stmt),
 				   gimple_assign_rhs2 (stmt));
   else if (get_gimple_rhs_class (code) == GIMPLE_SINGLE_RHS
-	   && is_gimple_min_invariant (gimple_assign_rhs1 (stmt)))
+	   && constant_range_value_p (gimple_assign_rhs1 (stmt)))
     vr->set (gimple_assign_rhs1 (stmt));
   else
     vr->set_varying (TREE_TYPE (gimple_assign_lhs (stmt)));
@@ -1756,7 +1756,7 @@ vr_values::adjust_range_with_scev (value
   chrec = instantiate_parameters (loop, analyze_scalar_evolution (loop, var));
 
   /* Like in PR19590, scev can return a constant function.  */
-  if (is_gimple_min_invariant (chrec))
+  if (constant_range_value_p (chrec))
     {
       vr->set (chrec);
       return;
@@ -1779,7 +1779,7 @@ vr_values::adjust_range_with_scev (value
      a simple expression, compare_values and possibly other functions
      in tree-vrp won't be able to handle it.  */
   if (step == NULL_TREE
-      || !is_gimple_min_invariant (step)
+      || !constant_range_value_p (step)
       || !valid_value_p (init))
     return;
 
@@ -1841,7 +1841,7 @@ vr_values::adjust_range_with_scev (value
 
 		  if (TREE_CODE (init) == SSA_NAME)
 		    initvr = *(get_value_range (init));
-		  else if (is_gimple_min_invariant (init))
+		  else if (constant_range_value_p (init))
 		    initvr.set (init);
 		  else
 		    return;
@@ -1995,7 +1995,7 @@ vrp_valueize (tree name)
       const value_range *vr = x_vr_values->get_value_range (name);
       if (vr->kind () == VR_RANGE
 	  && (TREE_CODE (vr->min ()) == SSA_NAME
-	      || is_gimple_min_invariant (vr->min ()))
+	      || constant_range_value_p (vr->min ()))
 	  && vrp_operand_equal_p (vr->min (), vr->max ()))
 	return vr->min ();
     }
@@ -2077,7 +2077,7 @@ vr_values::vrp_visit_assignment_or_call
 	      extract_range_from_ssa_name (vr, tem);
 	      return;
 	    }
-	  else if (is_gimple_min_invariant (tem))
+	  else if (constant_range_value_p (tem))
 	    {
 	      vr->set (tem);
 	      return;
@@ -2475,7 +2475,7 @@ vr_values::vrp_evaluate_conditional (tre
       enum warn_strict_overflow_code wc;
       const char* warnmsg;
 
-      if (is_gimple_min_invariant (ret))
+      if (constant_range_value_p (ret))
 	{
 	  wc = WARN_STRICT_OVERFLOW_CONDITIONAL;
 	  warnmsg = G_("assuming signed overflow does not occur when "
@@ -2517,7 +2517,7 @@ vr_values::vrp_evaluate_conditional (tre
 	  && INTEGRAL_TYPE_P (type)
 	  && vrp_val_is_min (vr0->min ())
 	  && vrp_val_is_max (vr0->max ())
-	  && is_gimple_min_invariant (op1))
+	  && constant_range_value_p (op1))
 	{
 	  location_t location;
 
@@ -3361,14 +3361,14 @@ vr_values::simplify_bit_ops_using_ranges
 
   if (TREE_CODE (op0) == SSA_NAME)
     vr0 = *(get_value_range (op0));
-  else if (is_gimple_min_invariant (op0))
+  else if (constant_range_value_p (op0))
     vr0.set (op0);
   else
     return false;
 
   if (TREE_CODE (op1) == SSA_NAME)
     vr1 = *(get_value_range (op1));
-  else if (is_gimple_min_invariant (op1))
+  else if (constant_range_value_p (op1))
     vr1.set (op1);
   else
     return false;
@@ -3481,7 +3481,7 @@ test_for_singularity (enum tree_code con
       /* If the new min/max values have converged to a single value,
 	 then there is only one value which can satisfy the condition,
 	 return that value.  */
-      if (operand_equal_p (min, max, 0) && is_gimple_min_invariant (min))
+      if (operand_equal_p (min, max, 0) && constant_range_value_p (min))
 	return min;
     }
   return NULL;
@@ -3554,7 +3554,7 @@ vr_values::simplify_cond_using_ranges_1
       && cond_code != EQ_EXPR
       && TREE_CODE (op0) == SSA_NAME
       && INTEGRAL_TYPE_P (TREE_TYPE (op0))
-      && is_gimple_min_invariant (op1))
+      && constant_range_value_p (op1))
     {
       const value_range *vr = get_value_range (op0);
 

Re: Add a constant_range_value_p function (PR 92033)

[Aldy Hernandez]

On 10/11/19 10:42 AM, Richard Sandiford wrote:
> The range-tracking code has a pretty hard-coded assumption that
> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
> ADDR_EXPR".  It seems better to add a predicate specifically for
> that rather than contiually fight cases in which it can't handle
> other invariants.

I was going to suggest we normalize ranges to numerics completely before 
folding.  That is, replacing normalize_addresses() here:

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

...into normalize_symbolics().  But I suppose getting the gate correct 
is even better.  Thanks for taking the care of this extensive and manual 
change.

The patch looks good to me.  However, I do wonder if VRP and 
subsidiaries can't handle non-integer invariants, if we shouldn't 
disallow them from the setters as well:

void
value_range_base::set (tree val)
{
   gcc_assert (TREE_CODE (val) == SSA_NAME || is_gimple_min_invariant 
(val));
   if (TREE_OVERFLOW_P (val))
     val = drop_tree_overflow (val);
   set (VR_RANGE, val, val);
}

void
value_range::set (tree val)
{
   gcc_assert (TREE_CODE (val) == SSA_NAME || is_gimple_min_invariant 
(val));
   if (TREE_OVERFLOW_P (val))
     val = drop_tree_overflow (val);
   set (VR_RANGE, val, val, NULL);
}

This would still allow setting of VARYING and UNDEFINED, but disallow 
poly-ints, etc from a range.

Was this a small oversight, or was there a reason you left those in?

Aldy

> 
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
> 
> Richard
> 
> 
> 2019-10-11  Richard Sandiford  <richard.sandiford@arm.com>
> 
> gcc/
> 	PR tree-optimization/92033
> 	* tree-vrp.h (constant_range_value_p): Declare.
> 	* tree-vrp.c (constant_range_value_p): New function.
> 	(value_range_base::symbolic_p, value_range_base::singleton_p)
> 	(get_single_symbol, compare_values_warnv, intersect_ranges)
> 	(value_range_base::normalize_symbolics): Use it instead of
> 	is_gimple_min_invariant.
> 	(simplify_stmt_for_jump_threading): Likewise.
> 	* vr-values.c (symbolic_range_based_on_p, valid_value_p): Likewise.
> 	(vr_values::op_with_constant_singleton_value_range): Likewise.
> 	(vr_values::extract_range_from_binary_expr): Likewise.
> 	(vr_values::extract_range_from_unary_expr): Likewise.
> 	(vr_values::extract_range_from_cond_expr): Likewise.
> 	(vr_values::extract_range_from_comparison): Likewise.
> 	(vr_values::extract_range_from_assignment): Likewise.
> 	(vr_values::adjust_range_with_scev, vrp_valueize): Likewise.
> 	(vr_values::vrp_visit_assignment_or_call): Likewise.
> 	(vr_values::vrp_evaluate_conditional): Likewise.
> 	(vr_values::simplify_bit_ops_using_ranges): Likewise.
> 	(test_for_singularity): Likewise.
> 	(vr_values::simplify_cond_using_ranges_1): Likewise.
> 
> Index: gcc/tree-vrp.h
> ===================================================================
> --- gcc/tree-vrp.h	2019-10-08 09:23:31.282533990 +0100
> +++ gcc/tree-vrp.h	2019-10-11 15:41:20.380576059 +0100
> @@ -284,6 +284,7 @@ value_range_base::supports_type_p (tree
>     return false;
>   }
>   
> +extern bool constant_range_value_p (const_tree);
>   extern void register_edge_assert_for (tree, edge, enum tree_code,
>   				      tree, tree, vec<assert_info> &);
>   extern bool stmt_interesting_for_vrp (gimple *);
> Index: gcc/tree-vrp.c
> ===================================================================
> --- gcc/tree-vrp.c	2019-10-08 09:23:31.282533990 +0100
> +++ gcc/tree-vrp.c	2019-10-11 15:41:20.380576059 +0100
> @@ -78,6 +78,18 @@ ranges_from_anti_range (const value_rang
>      for still active basic-blocks.  */
>   static sbitmap *live;
>   
> +/* Return true if VALUE is considered constant for range tracking.
> +   This is stricter than is_gimple_min_invariant and should be
> +   used instead of it in range-related code.  */
> +
> +bool
> +constant_range_value_p (const_tree value)
> +{
> +  return (TREE_CODE (value) == INTEGER_CST
> +	  || (TREE_CODE (value) == ADDR_EXPR
> +	      && is_gimple_invariant_address (value)));
> +}
> +
>   void
>   value_range::set_equiv (bitmap equiv)
>   {
> @@ -273,8 +285,8 @@ value_range_base::symbolic_p () const
>   {
>     return (!varying_p ()
>   	  && !undefined_p ()
> -	  && (!is_gimple_min_invariant (m_min)
> -	      || !is_gimple_min_invariant (m_max)));
> +	  && (!constant_range_value_p (m_min)
> +	      || !constant_range_value_p (m_max)));
>   }
>   
>   /* NOTE: This is not the inverse of symbolic_p because the range
> @@ -388,7 +400,7 @@ value_range_base::singleton_p (tree *res
>       }
>     if (m_kind == VR_RANGE
>         && vrp_operand_equal_p (min (), max ())
> -      && is_gimple_min_invariant (min ()))
> +      && constant_range_value_p (min ()))
>       {
>         if (result)
>           *result = min ();
> @@ -953,13 +965,13 @@ get_single_symbol (tree t, bool *neg, tr
>         || TREE_CODE (t) == POINTER_PLUS_EXPR
>         || TREE_CODE (t) == MINUS_EXPR)
>       {
> -      if (is_gimple_min_invariant (TREE_OPERAND (t, 0)))
> +      if (constant_range_value_p (TREE_OPERAND (t, 0)))
>   	{
>   	  neg_ = (TREE_CODE (t) == MINUS_EXPR);
>   	  inv_ = TREE_OPERAND (t, 0);
>   	  t = TREE_OPERAND (t, 1);
>   	}
> -      else if (is_gimple_min_invariant (TREE_OPERAND (t, 1)))
> +      else if (constant_range_value_p (TREE_OPERAND (t, 1)))
>   	{
>   	  neg_ = false;
>   	  inv_ = TREE_OPERAND (t, 1);
> @@ -1106,8 +1118,8 @@ compare_values_warnv (tree val1, tree va
>   		      TYPE_SIGN (TREE_TYPE (val1)));
>       }
>   
> -  const bool cst1 = is_gimple_min_invariant (val1);
> -  const bool cst2 = is_gimple_min_invariant (val2);
> +  const bool cst1 = constant_range_value_p (val1);
> +  const bool cst2 = constant_range_value_p (val2);
>   
>     /* If one is of the form '[-]NAME + CST' and the other is constant, then
>        it might be possible to say something depending on the constants.  */
> @@ -5785,7 +5797,7 @@ intersect_ranges (enum value_range_kind
>        correct estimate unless VR1 is a constant singleton range
>        in which case we choose that.  */
>     if (vr1type == VR_RANGE
> -      && is_gimple_min_invariant (vr1min)
> +      && constant_range_value_p (vr1min)
>         && vrp_operand_equal_p (vr1min, vr1max))
>       {
>         *vr0type = vr1type;
> @@ -6045,8 +6057,8 @@ value_range_base::normalize_symbolics ()
>     if (varying_p () || undefined_p ())
>       return *this;
>     tree ttype = type ();
> -  bool min_symbolic = !is_gimple_min_invariant (min ());
> -  bool max_symbolic = !is_gimple_min_invariant (max ());
> +  bool min_symbolic = !constant_range_value_p (min ());
> +  bool max_symbolic = !constant_range_value_p (max ());
>     if (!min_symbolic && !max_symbolic)
>       return normalize_addresses ();
>   
> @@ -6432,7 +6444,7 @@ simplify_stmt_for_jump_threading (gimple
>   {
>     /* First see if the conditional is in the hash table.  */
>     tree cached_lhs = avail_exprs_stack->lookup_avail_expr (stmt, false, true);
> -  if (cached_lhs && is_gimple_min_invariant (cached_lhs))
> +  if (cached_lhs && constant_range_value_p (cached_lhs))
>       return cached_lhs;
>   
>     vr_values *vr_values = x_vr_values;
> Index: gcc/vr-values.c
> ===================================================================
> --- gcc/vr-values.c	2019-10-03 14:04:54.161520173 +0100
> +++ gcc/vr-values.c	2019-10-11 15:41:20.380576059 +0100
> @@ -263,14 +263,14 @@ symbolic_range_based_on_p (value_range_b
>     bool neg, min_has_symbol, max_has_symbol;
>     tree inv;
>   
> -  if (is_gimple_min_invariant (vr->min ()))
> +  if (constant_range_value_p (vr->min ()))
>       min_has_symbol = false;
>     else if (get_single_symbol (vr->min (), &neg, &inv) == sym)
>       min_has_symbol = true;
>     else
>       return false;
>   
> -  if (is_gimple_min_invariant (vr->max ()))
> +  if (constant_range_value_p (vr->max ()))
>       max_has_symbol = false;
>     else if (get_single_symbol (vr->max (), &neg, &inv) == sym)
>       max_has_symbol = true;
> @@ -409,7 +409,7 @@ valid_value_p (tree expr)
>       return (TREE_CODE (TREE_OPERAND (expr, 0)) == SSA_NAME
>   	    && TREE_CODE (TREE_OPERAND (expr, 1)) == INTEGER_CST);
>   
> -  return is_gimple_min_invariant (expr);
> +  return constant_range_value_p (expr);
>   }
>   
>   /* If OP has a value range with a single constant value return that,
> @@ -419,7 +419,7 @@ valid_value_p (tree expr)
>   tree
>   vr_values::op_with_constant_singleton_value_range (tree op)
>   {
> -  if (is_gimple_min_invariant (op))
> +  if (constant_range_value_p (op))
>       return op;
>   
>     if (TREE_CODE (op) != SSA_NAME)
> @@ -778,14 +778,14 @@ vr_values::extract_range_from_binary_exp
>     value_range_base vr0, vr1;
>     if (TREE_CODE (op0) == SSA_NAME)
>       vr0 = *(get_value_range (op0));
> -  else if (is_gimple_min_invariant (op0))
> +  else if (constant_range_value_p (op0))
>       vr0.set (op0);
>     else
>       vr0.set_varying (TREE_TYPE (op0));
>   
>     if (TREE_CODE (op1) == SSA_NAME)
>       vr1 = *(get_value_range (op1));
> -  else if (is_gimple_min_invariant (op1))
> +  else if (constant_range_value_p (op1))
>       vr1.set (op1);
>     else
>       vr1.set_varying (TREE_TYPE (op1));
> @@ -855,11 +855,11 @@ vr_values::extract_range_from_binary_exp
>         value_range n_vr1;
>   
>         /* Try with VR0 and [-INF, OP1].  */
> -      if (is_gimple_min_invariant (minus_p ? vr0.max () : vr0.min ()))
> +      if (constant_range_value_p (minus_p ? vr0.max () : vr0.min ()))
>   	n_vr1.set (VR_RANGE, vrp_val_min (expr_type), op1);
>   
>         /* Try with VR0 and [OP1, +INF].  */
> -      else if (is_gimple_min_invariant (minus_p ? vr0.min () : vr0.max ()))
> +      else if (constant_range_value_p (minus_p ? vr0.min () : vr0.max ()))
>   	n_vr1.set (VR_RANGE, op1, vrp_val_max (expr_type));
>   
>         /* Try with VR0 and [OP1, OP1].  */
> @@ -879,11 +879,11 @@ vr_values::extract_range_from_binary_exp
>         value_range n_vr0;
>   
>         /* Try with [-INF, OP0] and VR1.  */
> -      if (is_gimple_min_invariant (minus_p ? vr1.max () : vr1.min ()))
> +      if (constant_range_value_p (minus_p ? vr1.max () : vr1.min ()))
>   	n_vr0.set (VR_RANGE, vrp_val_min (expr_type), op0);
>   
>         /* Try with [OP0, +INF] and VR1.  */
> -      else if (is_gimple_min_invariant (minus_p ? vr1.min (): vr1.max ()))
> +      else if (constant_range_value_p (minus_p ? vr1.min (): vr1.max ()))
>   	n_vr0.set (VR_RANGE, op0, vrp_val_max (expr_type));
>   
>         /* Try with [OP0, OP0] and VR1.  */
> @@ -926,7 +926,7 @@ vr_values::extract_range_from_unary_expr
>        a new value range with the operand to simplify processing.  */
>     if (TREE_CODE (op0) == SSA_NAME)
>       vr0 = *(get_value_range (op0));
> -  else if (is_gimple_min_invariant (op0))
> +  else if (constant_range_value_p (op0))
>       vr0.set (op0);
>     else
>       vr0.set_varying (type);
> @@ -948,7 +948,7 @@ vr_values::extract_range_from_cond_expr
>     const value_range *vr0 = &tem0;
>     if (TREE_CODE (op0) == SSA_NAME)
>       vr0 = get_value_range (op0);
> -  else if (is_gimple_min_invariant (op0))
> +  else if (constant_range_value_p (op0))
>       tem0.set (op0);
>     else
>       tem0.set_varying (TREE_TYPE (op0));
> @@ -958,7 +958,7 @@ vr_values::extract_range_from_cond_expr
>     const value_range *vr1 = &tem1;
>     if (TREE_CODE (op1) == SSA_NAME)
>       vr1 = get_value_range (op1);
> -  else if (is_gimple_min_invariant (op1))
> +  else if (constant_range_value_p (op1))
>       tem1.set (op1);
>     else
>       tem1.set_varying (TREE_TYPE (op1));
> @@ -987,7 +987,7 @@ vr_values::extract_range_from_comparison
>   	 its type may be different from _Bool.  Convert VAL to EXPR's
>   	 type.  */
>         val = fold_convert (type, val);
> -      if (is_gimple_min_invariant (val))
> +      if (constant_range_value_p (val))
>   	vr->set (val);
>         else
>   	vr->update (VR_RANGE, val, val);
> @@ -1479,7 +1479,7 @@ vr_values::extract_range_from_assignment
>   				   gimple_assign_rhs1 (stmt),
>   				   gimple_assign_rhs2 (stmt));
>     else if (get_gimple_rhs_class (code) == GIMPLE_SINGLE_RHS
> -	   && is_gimple_min_invariant (gimple_assign_rhs1 (stmt)))
> +	   && constant_range_value_p (gimple_assign_rhs1 (stmt)))
>       vr->set (gimple_assign_rhs1 (stmt));
>     else
>       vr->set_varying (TREE_TYPE (gimple_assign_lhs (stmt)));
> @@ -1756,7 +1756,7 @@ vr_values::adjust_range_with_scev (value
>     chrec = instantiate_parameters (loop, analyze_scalar_evolution (loop, var));
>   
>     /* Like in PR19590, scev can return a constant function.  */
> -  if (is_gimple_min_invariant (chrec))
> +  if (constant_range_value_p (chrec))
>       {
>         vr->set (chrec);
>         return;
> @@ -1779,7 +1779,7 @@ vr_values::adjust_range_with_scev (value
>        a simple expression, compare_values and possibly other functions
>        in tree-vrp won't be able to handle it.  */
>     if (step == NULL_TREE
> -      || !is_gimple_min_invariant (step)
> +      || !constant_range_value_p (step)
>         || !valid_value_p (init))
>       return;
>   
> @@ -1841,7 +1841,7 @@ vr_values::adjust_range_with_scev (value
>   
>   		  if (TREE_CODE (init) == SSA_NAME)
>   		    initvr = *(get_value_range (init));
> -		  else if (is_gimple_min_invariant (init))
> +		  else if (constant_range_value_p (init))
>   		    initvr.set (init);
>   		  else
>   		    return;
> @@ -1995,7 +1995,7 @@ vrp_valueize (tree name)
>         const value_range *vr = x_vr_values->get_value_range (name);
>         if (vr->kind () == VR_RANGE
>   	  && (TREE_CODE (vr->min ()) == SSA_NAME
> -	      || is_gimple_min_invariant (vr->min ()))
> +	      || constant_range_value_p (vr->min ()))
>   	  && vrp_operand_equal_p (vr->min (), vr->max ()))
>   	return vr->min ();
>       }
> @@ -2077,7 +2077,7 @@ vr_values::vrp_visit_assignment_or_call
>   	      extract_range_from_ssa_name (vr, tem);
>   	      return;
>   	    }
> -	  else if (is_gimple_min_invariant (tem))
> +	  else if (constant_range_value_p (tem))
>   	    {
>   	      vr->set (tem);
>   	      return;
> @@ -2475,7 +2475,7 @@ vr_values::vrp_evaluate_conditional (tre
>         enum warn_strict_overflow_code wc;
>         const char* warnmsg;
>   
> -      if (is_gimple_min_invariant (ret))
> +      if (constant_range_value_p (ret))
>   	{
>   	  wc = WARN_STRICT_OVERFLOW_CONDITIONAL;
>   	  warnmsg = G_("assuming signed overflow does not occur when "
> @@ -2517,7 +2517,7 @@ vr_values::vrp_evaluate_conditional (tre
>   	  && INTEGRAL_TYPE_P (type)
>   	  && vrp_val_is_min (vr0->min ())
>   	  && vrp_val_is_max (vr0->max ())
> -	  && is_gimple_min_invariant (op1))
> +	  && constant_range_value_p (op1))
>   	{
>   	  location_t location;
>   
> @@ -3361,14 +3361,14 @@ vr_values::simplify_bit_ops_using_ranges
>   
>     if (TREE_CODE (op0) == SSA_NAME)
>       vr0 = *(get_value_range (op0));
> -  else if (is_gimple_min_invariant (op0))
> +  else if (constant_range_value_p (op0))
>       vr0.set (op0);
>     else
>       return false;
>   
>     if (TREE_CODE (op1) == SSA_NAME)
>       vr1 = *(get_value_range (op1));
> -  else if (is_gimple_min_invariant (op1))
> +  else if (constant_range_value_p (op1))
>       vr1.set (op1);
>     else
>       return false;
> @@ -3481,7 +3481,7 @@ test_for_singularity (enum tree_code con
>         /* If the new min/max values have converged to a single value,
>   	 then there is only one value which can satisfy the condition,
>   	 return that value.  */
> -      if (operand_equal_p (min, max, 0) && is_gimple_min_invariant (min))
> +      if (operand_equal_p (min, max, 0) && constant_range_value_p (min))
>   	return min;
>       }
>     return NULL;
> @@ -3554,7 +3554,7 @@ vr_values::simplify_cond_using_ranges_1
>         && cond_code != EQ_EXPR
>         && TREE_CODE (op0) == SSA_NAME
>         && INTEGRAL_TYPE_P (TREE_TYPE (op0))
> -      && is_gimple_min_invariant (op1))
> +      && constant_range_value_p (op1))
>       {
>         const value_range *vr = get_value_range (op0);
>   
> 

Re: Add a constant_range_value_p function (PR 92033)

[Richard Sandiford]

Aldy Hernandez <aldyh@redhat.com> writes:
> On 10/11/19 10:42 AM, Richard Sandiford wrote:
>> The range-tracking code has a pretty hard-coded assumption that
>> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
>> ADDR_EXPR".  It seems better to add a predicate specifically for
>> that rather than contiually fight cases in which it can't handle
>> other invariants.
>
> I was going to suggest we normalize ranges to numerics completely before 
> folding.  That is, replacing normalize_addresses() here:
>
>    *vr = op->fold_range (expr_type,
> 			vr0.normalize_addresses (),
> 			vr1.normalize_addresses ());
>
> ...into normalize_symbolics().  But I suppose getting the gate correct 
> is even better.  Thanks for taking the care of this extensive and manual 
> change.
>
> The patch looks good to me.  However, I do wonder if VRP and 
> subsidiaries can't handle non-integer invariants, if we shouldn't 
> disallow them from the setters as well:
>
> void
> value_range_base::set (tree val)
> {
>    gcc_assert (TREE_CODE (val) == SSA_NAME || is_gimple_min_invariant 
> (val));
>    if (TREE_OVERFLOW_P (val))
>      val = drop_tree_overflow (val);
>    set (VR_RANGE, val, val);
> }
>
> void
> value_range::set (tree val)
> {
>    gcc_assert (TREE_CODE (val) == SSA_NAME || is_gimple_min_invariant 
> (val));
>    if (TREE_OVERFLOW_P (val))
>      val = drop_tree_overflow (val);
>    set (VR_RANGE, val, val, NULL);
> }
>
> This would still allow setting of VARYING and UNDEFINED, but disallow 
> poly-ints, etc from a range.
>
> Was this a small oversight, or was there a reason you left those in?

Yeah, this was intentional.  The patch is effectively treating
POLY_INY_CST as symbolic rather than constant.  (It's really somewhere
in the middle: it's at least a function invariant, but the invariant
depends on a runtime target property.)  So places like here that
can handle both symbolics and constants should be able to handle
POLY_INT_CST wihout problems.  We just need to make sure that
POLY_INT_CSTs aren't treated as constants for range tracking,
because they're not "constant enough" there.

Thanks,
Richard

Re: Add a constant_range_value_p function (PR 92033)

[Aldy Hernandez]

On 10/14/19 4:30 AM, Richard Sandiford wrote:
> Aldy Hernandez <aldyh@redhat.com> writes:
>> On 10/11/19 10:42 AM, Richard Sandiford wrote:
>>> The range-tracking code has a pretty hard-coded assumption that
>>> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
>>> ADDR_EXPR".  It seems better to add a predicate specifically for
>>> that rather than contiually fight cases in which it can't handle
>>> other invariants.
>>
>> I was going to suggest we normalize ranges to numerics completely before
>> folding.  That is, replacing normalize_addresses() here:
>>
>>     *vr = op->fold_range (expr_type,
>> 			vr0.normalize_addresses (),
>> 			vr1.normalize_addresses ());
>>
>> ...into normalize_symbolics().  But I suppose getting the gate correct
>> is even better.  Thanks for taking the care of this extensive and manual
>> change.
>>
>> The patch looks good to me.  However, I do wonder if VRP and
>> subsidiaries can't handle non-integer invariants, if we shouldn't
>> disallow them from the setters as well:
>>
>> void
>> value_range_base::set (tree val)
>> {
>>     gcc_assert (TREE_CODE (val) == SSA_NAME || is_gimple_min_invariant
>> (val));
>>     if (TREE_OVERFLOW_P (val))
>>       val = drop_tree_overflow (val);
>>     set (VR_RANGE, val, val);
>> }
>>
>> void
>> value_range::set (tree val)
>> {
>>     gcc_assert (TREE_CODE (val) == SSA_NAME || is_gimple_min_invariant
>> (val));
>>     if (TREE_OVERFLOW_P (val))
>>       val = drop_tree_overflow (val);
>>     set (VR_RANGE, val, val, NULL);
>> }
>>
>> This would still allow setting of VARYING and UNDEFINED, but disallow
>> poly-ints, etc from a range.
>>
>> Was this a small oversight, or was there a reason you left those in?
> 
> Yeah, this was intentional.  The patch is effectively treating
> POLY_INY_CST as symbolic rather than constant.  (It's really somewhere
> in the middle: it's at least a function invariant, but the invariant
> depends on a runtime target property.)  So places like here that
> can handle both symbolics and constants should be able to handle
> POLY_INT_CST wihout problems.  We just need to make sure that
> POLY_INT_CSTs aren't treated as constants for range tracking,
> because they're not "constant enough" there.

Hmmm... We don't handle POLY_INT_CST value_range's anywhere, so perhaps 
it's better to stop their creation at the source and fix the caller, to 
inhibit their proliferation.  AFAICT, the only POLY_INT_CST ranges that 
would be used are VARYING and UNDEFINED (for the lattice), and that 
already works.

If you don't agree, then at least a comment in ::set() would be nice, to 
document that we're allowing their creation and why.

Thanks.
Aldy

Re: Add a constant_range_value_p function (PR 92033)

[Richard Sandiford]

Aldy Hernandez <aldyh@redhat.com> writes:
> On 10/14/19 4:30 AM, Richard Sandiford wrote:
>> Aldy Hernandez <aldyh@redhat.com> writes:
>>> On 10/11/19 10:42 AM, Richard Sandiford wrote:
>>>> The range-tracking code has a pretty hard-coded assumption that
>>>> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
>>>> ADDR_EXPR".  It seems better to add a predicate specifically for
>>>> that rather than contiually fight cases in which it can't handle
>>>> other invariants.
>>>
>>> I was going to suggest we normalize ranges to numerics completely before
>>> folding.  That is, replacing normalize_addresses() here:
>>>
>>>     *vr = op->fold_range (expr_type,
>>> 			vr0.normalize_addresses (),
>>> 			vr1.normalize_addresses ());
>>>
>>> ...into normalize_symbolics().  But I suppose getting the gate correct
>>> is even better.  Thanks for taking the care of this extensive and manual
>>> change.
>>>
>>> The patch looks good to me.  However, I do wonder if VRP and
>>> subsidiaries can't handle non-integer invariants, if we shouldn't
>>> disallow them from the setters as well:
>>>
>>> void
>>> value_range_base::set (tree val)
>>> {
>>>     gcc_assert (TREE_CODE (val) == SSA_NAME || is_gimple_min_invariant
>>> (val));
>>>     if (TREE_OVERFLOW_P (val))
>>>       val = drop_tree_overflow (val);
>>>     set (VR_RANGE, val, val);
>>> }
>>>
>>> void
>>> value_range::set (tree val)
>>> {
>>>     gcc_assert (TREE_CODE (val) == SSA_NAME || is_gimple_min_invariant
>>> (val));
>>>     if (TREE_OVERFLOW_P (val))
>>>       val = drop_tree_overflow (val);
>>>     set (VR_RANGE, val, val, NULL);
>>> }
>>>
>>> This would still allow setting of VARYING and UNDEFINED, but disallow
>>> poly-ints, etc from a range.
>>>
>>> Was this a small oversight, or was there a reason you left those in?
>> 
>> Yeah, this was intentional.  The patch is effectively treating
>> POLY_INY_CST as symbolic rather than constant.  (It's really somewhere
>> in the middle: it's at least a function invariant, but the invariant
>> depends on a runtime target property.)  So places like here that
>> can handle both symbolics and constants should be able to handle
>> POLY_INT_CST wihout problems.  We just need to make sure that
>> POLY_INT_CSTs aren't treated as constants for range tracking,
>> because they're not "constant enough" there.
>
> Hmmm... We don't handle POLY_INT_CST value_range's anywhere, so perhaps 
> it's better to stop their creation at the source and fix the caller, to 
> inhibit their proliferation.  AFAICT, the only POLY_INT_CST ranges that 
> would be used are VARYING and UNDEFINED (for the lattice), and that 
> already works.
>
> If you don't agree, then at least a comment in ::set() would be nice, to 
> document that we're allowing their creation and why.

I don't think it makes sense to allow SSA_NAME (which is completely
unconstrained) and not allow POLY_INT_CST.  Like I say, POLY_INT_CST
can be conservatively treated as symbolic.

POLY_INT_CST can produce useful singleton ranges or be a minimum or
a maximum, even if we don't use them to reduce ranges arithmetically.

Richard

Re: Add a constant_range_value_p function (PR 92033)

[Aldy Hernandez]

On 10/14/19 8:31 AM, Richard Sandiford wrote:
> Aldy Hernandez <aldyh@redhat.com> writes:
>> On 10/14/19 4:30 AM, Richard Sandiford wrote:
>>> Aldy Hernandez <aldyh@redhat.com> writes:
>>>> On 10/11/19 10:42 AM, Richard Sandiford wrote:
>>>>> The range-tracking code has a pretty hard-coded assumption that
>>>>> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
>>>>> ADDR_EXPR".  It seems better to add a predicate specifically for
>>>>> that rather than contiually fight cases in which it can't handle
>>>>> other invariants.
>>>>
>>>> I was going to suggest we normalize ranges to numerics completely before
>>>> folding.  That is, replacing normalize_addresses() here:
>>>>
>>>>      *vr = op->fold_range (expr_type,
>>>> 			vr0.normalize_addresses (),
>>>> 			vr1.normalize_addresses ());
>>>>
>>>> ...into normalize_symbolics().  But I suppose getting the gate correct
>>>> is even better.  Thanks for taking the care of this extensive and manual
>>>> change.
>>>>
>>>> The patch looks good to me.  However, I do wonder if VRP and
>>>> subsidiaries can't handle non-integer invariants, if we shouldn't
>>>> disallow them from the setters as well:
>>>>
>>>> void
>>>> value_range_base::set (tree val)
>>>> {
>>>>      gcc_assert (TREE_CODE (val) == SSA_NAME || is_gimple_min_invariant
>>>> (val));
>>>>      if (TREE_OVERFLOW_P (val))
>>>>        val = drop_tree_overflow (val);
>>>>      set (VR_RANGE, val, val);
>>>> }
>>>>
>>>> void
>>>> value_range::set (tree val)
>>>> {
>>>>      gcc_assert (TREE_CODE (val) == SSA_NAME || is_gimple_min_invariant
>>>> (val));
>>>>      if (TREE_OVERFLOW_P (val))
>>>>        val = drop_tree_overflow (val);
>>>>      set (VR_RANGE, val, val, NULL);
>>>> }
>>>>
>>>> This would still allow setting of VARYING and UNDEFINED, but disallow
>>>> poly-ints, etc from a range.
>>>>
>>>> Was this a small oversight, or was there a reason you left those in?
>>>
>>> Yeah, this was intentional.  The patch is effectively treating
>>> POLY_INY_CST as symbolic rather than constant.  (It's really somewhere
>>> in the middle: it's at least a function invariant, but the invariant
>>> depends on a runtime target property.)  So places like here that
>>> can handle both symbolics and constants should be able to handle
>>> POLY_INT_CST wihout problems.  We just need to make sure that
>>> POLY_INT_CSTs aren't treated as constants for range tracking,
>>> because they're not "constant enough" there.
>>
>> Hmmm... We don't handle POLY_INT_CST value_range's anywhere, so perhaps
>> it's better to stop their creation at the source and fix the caller, to
>> inhibit their proliferation.  AFAICT, the only POLY_INT_CST ranges that
>> would be used are VARYING and UNDEFINED (for the lattice), and that
>> already works.
>>
>> If you don't agree, then at least a comment in ::set() would be nice, to
>> document that we're allowing their creation and why.
> 
> I don't think it makes sense to allow SSA_NAME (which is completely
> unconstrained) and not allow POLY_INT_CST.  Like I say, POLY_INT_CST
> can be conservatively treated as symbolic.

Oh, I see your change to ::symbolic_p() would effectively treat them as 
symbolics.

> 
> POLY_INT_CST can produce useful singleton ranges or be a minimum or
> a maximum, even if we don't use them to reduce ranges arithmetically.

In which case, singleton_p() should be adapted to return the singleton. 
I don't expect you to do this, unless you want to :).  Could you at 
least put a comment there, so we don't forget?

I'm feel a bit queasy allowing POLY_INT ranges throughout, when we have 
no code that handle them anywhere, but I won't object to your patch. 
Thanks for fixing this.

The patch with the comment on singleton is fine with me.

Aldy

Re: Add a constant_range_value_p function (PR 92033)

[Richard Biener]

On Fri, Oct 11, 2019 at 4:42 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> The range-tracking code has a pretty hard-coded assumption that
> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
> ADDR_EXPR".  It seems better to add a predicate specifically for
> that rather than contiually fight cases in which it can't handle
> other invariants.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

ICK.  Nobody is going to remember this new restriction and
constant_range_value_p reads like constant_value_range_p ;)

Btw, is_gimple_invariant_address shouldn't have been exported,
it's only use could have used is_gimple_min_invariant...

Richard.

> Richard
>
>
> 2019-10-11  Richard Sandiford  <richard.sandiford@arm.com>
>
> gcc/
>         PR tree-optimization/92033
>         * tree-vrp.h (constant_range_value_p): Declare.
>         * tree-vrp.c (constant_range_value_p): New function.
>         (value_range_base::symbolic_p, value_range_base::singleton_p)
>         (get_single_symbol, compare_values_warnv, intersect_ranges)
>         (value_range_base::normalize_symbolics): Use it instead of
>         is_gimple_min_invariant.
>         (simplify_stmt_for_jump_threading): Likewise.
>         * vr-values.c (symbolic_range_based_on_p, valid_value_p): Likewise.
>         (vr_values::op_with_constant_singleton_value_range): Likewise.
>         (vr_values::extract_range_from_binary_expr): Likewise.
>         (vr_values::extract_range_from_unary_expr): Likewise.
>         (vr_values::extract_range_from_cond_expr): Likewise.
>         (vr_values::extract_range_from_comparison): Likewise.
>         (vr_values::extract_range_from_assignment): Likewise.
>         (vr_values::adjust_range_with_scev, vrp_valueize): Likewise.
>         (vr_values::vrp_visit_assignment_or_call): Likewise.
>         (vr_values::vrp_evaluate_conditional): Likewise.
>         (vr_values::simplify_bit_ops_using_ranges): Likewise.
>         (test_for_singularity): Likewise.
>         (vr_values::simplify_cond_using_ranges_1): Likewise.
>
> Index: gcc/tree-vrp.h
> ===================================================================
> --- gcc/tree-vrp.h      2019-10-08 09:23:31.282533990 +0100
> +++ gcc/tree-vrp.h      2019-10-11 15:41:20.380576059 +0100
> @@ -284,6 +284,7 @@ value_range_base::supports_type_p (tree
>    return false;
>  }
>
> +extern bool constant_range_value_p (const_tree);
>  extern void register_edge_assert_for (tree, edge, enum tree_code,
>                                       tree, tree, vec<assert_info> &);
>  extern bool stmt_interesting_for_vrp (gimple *);
> Index: gcc/tree-vrp.c
> ===================================================================
> --- gcc/tree-vrp.c      2019-10-08 09:23:31.282533990 +0100
> +++ gcc/tree-vrp.c      2019-10-11 15:41:20.380576059 +0100
> @@ -78,6 +78,18 @@ ranges_from_anti_range (const value_rang
>     for still active basic-blocks.  */
>  static sbitmap *live;
>
> +/* Return true if VALUE is considered constant for range tracking.
> +   This is stricter than is_gimple_min_invariant and should be
> +   used instead of it in range-related code.  */
> +
> +bool
> +constant_range_value_p (const_tree value)
> +{
> +  return (TREE_CODE (value) == INTEGER_CST
> +         || (TREE_CODE (value) == ADDR_EXPR
> +             && is_gimple_invariant_address (value)));
> +}
> +
>  void
>  value_range::set_equiv (bitmap equiv)
>  {
> @@ -273,8 +285,8 @@ value_range_base::symbolic_p () const
>  {
>    return (!varying_p ()
>           && !undefined_p ()
> -         && (!is_gimple_min_invariant (m_min)
> -             || !is_gimple_min_invariant (m_max)));
> +         && (!constant_range_value_p (m_min)
> +             || !constant_range_value_p (m_max)));
>  }
>
>  /* NOTE: This is not the inverse of symbolic_p because the range
> @@ -388,7 +400,7 @@ value_range_base::singleton_p (tree *res
>      }
>    if (m_kind == VR_RANGE
>        && vrp_operand_equal_p (min (), max ())
> -      && is_gimple_min_invariant (min ()))
> +      && constant_range_value_p (min ()))
>      {
>        if (result)
>          *result = min ();
> @@ -953,13 +965,13 @@ get_single_symbol (tree t, bool *neg, tr
>        || TREE_CODE (t) == POINTER_PLUS_EXPR
>        || TREE_CODE (t) == MINUS_EXPR)
>      {
> -      if (is_gimple_min_invariant (TREE_OPERAND (t, 0)))
> +      if (constant_range_value_p (TREE_OPERAND (t, 0)))
>         {
>           neg_ = (TREE_CODE (t) == MINUS_EXPR);
>           inv_ = TREE_OPERAND (t, 0);
>           t = TREE_OPERAND (t, 1);
>         }
> -      else if (is_gimple_min_invariant (TREE_OPERAND (t, 1)))
> +      else if (constant_range_value_p (TREE_OPERAND (t, 1)))
>         {
>           neg_ = false;
>           inv_ = TREE_OPERAND (t, 1);
> @@ -1106,8 +1118,8 @@ compare_values_warnv (tree val1, tree va
>                       TYPE_SIGN (TREE_TYPE (val1)));
>      }
>
> -  const bool cst1 = is_gimple_min_invariant (val1);
> -  const bool cst2 = is_gimple_min_invariant (val2);
> +  const bool cst1 = constant_range_value_p (val1);
> +  const bool cst2 = constant_range_value_p (val2);
>
>    /* If one is of the form '[-]NAME + CST' and the other is constant, then
>       it might be possible to say something depending on the constants.  */
> @@ -5785,7 +5797,7 @@ intersect_ranges (enum value_range_kind
>       correct estimate unless VR1 is a constant singleton range
>       in which case we choose that.  */
>    if (vr1type == VR_RANGE
> -      && is_gimple_min_invariant (vr1min)
> +      && constant_range_value_p (vr1min)
>        && vrp_operand_equal_p (vr1min, vr1max))
>      {
>        *vr0type = vr1type;
> @@ -6045,8 +6057,8 @@ value_range_base::normalize_symbolics ()
>    if (varying_p () || undefined_p ())
>      return *this;
>    tree ttype = type ();
> -  bool min_symbolic = !is_gimple_min_invariant (min ());
> -  bool max_symbolic = !is_gimple_min_invariant (max ());
> +  bool min_symbolic = !constant_range_value_p (min ());
> +  bool max_symbolic = !constant_range_value_p (max ());
>    if (!min_symbolic && !max_symbolic)
>      return normalize_addresses ();
>
> @@ -6432,7 +6444,7 @@ simplify_stmt_for_jump_threading (gimple
>  {
>    /* First see if the conditional is in the hash table.  */
>    tree cached_lhs = avail_exprs_stack->lookup_avail_expr (stmt, false, true);
> -  if (cached_lhs && is_gimple_min_invariant (cached_lhs))
> +  if (cached_lhs && constant_range_value_p (cached_lhs))
>      return cached_lhs;
>
>    vr_values *vr_values = x_vr_values;
> Index: gcc/vr-values.c
> ===================================================================
> --- gcc/vr-values.c     2019-10-03 14:04:54.161520173 +0100
> +++ gcc/vr-values.c     2019-10-11 15:41:20.380576059 +0100
> @@ -263,14 +263,14 @@ symbolic_range_based_on_p (value_range_b
>    bool neg, min_has_symbol, max_has_symbol;
>    tree inv;
>
> -  if (is_gimple_min_invariant (vr->min ()))
> +  if (constant_range_value_p (vr->min ()))
>      min_has_symbol = false;
>    else if (get_single_symbol (vr->min (), &neg, &inv) == sym)
>      min_has_symbol = true;
>    else
>      return false;
>
> -  if (is_gimple_min_invariant (vr->max ()))
> +  if (constant_range_value_p (vr->max ()))
>      max_has_symbol = false;
>    else if (get_single_symbol (vr->max (), &neg, &inv) == sym)
>      max_has_symbol = true;
> @@ -409,7 +409,7 @@ valid_value_p (tree expr)
>      return (TREE_CODE (TREE_OPERAND (expr, 0)) == SSA_NAME
>             && TREE_CODE (TREE_OPERAND (expr, 1)) == INTEGER_CST);
>
> -  return is_gimple_min_invariant (expr);
> +  return constant_range_value_p (expr);
>  }
>
>  /* If OP has a value range with a single constant value return that,
> @@ -419,7 +419,7 @@ valid_value_p (tree expr)
>  tree
>  vr_values::op_with_constant_singleton_value_range (tree op)
>  {
> -  if (is_gimple_min_invariant (op))
> +  if (constant_range_value_p (op))
>      return op;
>
>    if (TREE_CODE (op) != SSA_NAME)
> @@ -778,14 +778,14 @@ vr_values::extract_range_from_binary_exp
>    value_range_base vr0, vr1;
>    if (TREE_CODE (op0) == SSA_NAME)
>      vr0 = *(get_value_range (op0));
> -  else if (is_gimple_min_invariant (op0))
> +  else if (constant_range_value_p (op0))
>      vr0.set (op0);
>    else
>      vr0.set_varying (TREE_TYPE (op0));
>
>    if (TREE_CODE (op1) == SSA_NAME)
>      vr1 = *(get_value_range (op1));
> -  else if (is_gimple_min_invariant (op1))
> +  else if (constant_range_value_p (op1))
>      vr1.set (op1);
>    else
>      vr1.set_varying (TREE_TYPE (op1));
> @@ -855,11 +855,11 @@ vr_values::extract_range_from_binary_exp
>        value_range n_vr1;
>
>        /* Try with VR0 and [-INF, OP1].  */
> -      if (is_gimple_min_invariant (minus_p ? vr0.max () : vr0.min ()))
> +      if (constant_range_value_p (minus_p ? vr0.max () : vr0.min ()))
>         n_vr1.set (VR_RANGE, vrp_val_min (expr_type), op1);
>
>        /* Try with VR0 and [OP1, +INF].  */
> -      else if (is_gimple_min_invariant (minus_p ? vr0.min () : vr0.max ()))
> +      else if (constant_range_value_p (minus_p ? vr0.min () : vr0.max ()))
>         n_vr1.set (VR_RANGE, op1, vrp_val_max (expr_type));
>
>        /* Try with VR0 and [OP1, OP1].  */
> @@ -879,11 +879,11 @@ vr_values::extract_range_from_binary_exp
>        value_range n_vr0;
>
>        /* Try with [-INF, OP0] and VR1.  */
> -      if (is_gimple_min_invariant (minus_p ? vr1.max () : vr1.min ()))
> +      if (constant_range_value_p (minus_p ? vr1.max () : vr1.min ()))
>         n_vr0.set (VR_RANGE, vrp_val_min (expr_type), op0);
>
>        /* Try with [OP0, +INF] and VR1.  */
> -      else if (is_gimple_min_invariant (minus_p ? vr1.min (): vr1.max ()))
> +      else if (constant_range_value_p (minus_p ? vr1.min (): vr1.max ()))
>         n_vr0.set (VR_RANGE, op0, vrp_val_max (expr_type));
>
>        /* Try with [OP0, OP0] and VR1.  */
> @@ -926,7 +926,7 @@ vr_values::extract_range_from_unary_expr
>       a new value range with the operand to simplify processing.  */
>    if (TREE_CODE (op0) == SSA_NAME)
>      vr0 = *(get_value_range (op0));
> -  else if (is_gimple_min_invariant (op0))
> +  else if (constant_range_value_p (op0))
>      vr0.set (op0);
>    else
>      vr0.set_varying (type);
> @@ -948,7 +948,7 @@ vr_values::extract_range_from_cond_expr
>    const value_range *vr0 = &tem0;
>    if (TREE_CODE (op0) == SSA_NAME)
>      vr0 = get_value_range (op0);
> -  else if (is_gimple_min_invariant (op0))
> +  else if (constant_range_value_p (op0))
>      tem0.set (op0);
>    else
>      tem0.set_varying (TREE_TYPE (op0));
> @@ -958,7 +958,7 @@ vr_values::extract_range_from_cond_expr
>    const value_range *vr1 = &tem1;
>    if (TREE_CODE (op1) == SSA_NAME)
>      vr1 = get_value_range (op1);
> -  else if (is_gimple_min_invariant (op1))
> +  else if (constant_range_value_p (op1))
>      tem1.set (op1);
>    else
>      tem1.set_varying (TREE_TYPE (op1));
> @@ -987,7 +987,7 @@ vr_values::extract_range_from_comparison
>          its type may be different from _Bool.  Convert VAL to EXPR's
>          type.  */
>        val = fold_convert (type, val);
> -      if (is_gimple_min_invariant (val))
> +      if (constant_range_value_p (val))
>         vr->set (val);
>        else
>         vr->update (VR_RANGE, val, val);
> @@ -1479,7 +1479,7 @@ vr_values::extract_range_from_assignment
>                                    gimple_assign_rhs1 (stmt),
>                                    gimple_assign_rhs2 (stmt));
>    else if (get_gimple_rhs_class (code) == GIMPLE_SINGLE_RHS
> -          && is_gimple_min_invariant (gimple_assign_rhs1 (stmt)))
> +          && constant_range_value_p (gimple_assign_rhs1 (stmt)))
>      vr->set (gimple_assign_rhs1 (stmt));
>    else
>      vr->set_varying (TREE_TYPE (gimple_assign_lhs (stmt)));
> @@ -1756,7 +1756,7 @@ vr_values::adjust_range_with_scev (value
>    chrec = instantiate_parameters (loop, analyze_scalar_evolution (loop, var));
>
>    /* Like in PR19590, scev can return a constant function.  */
> -  if (is_gimple_min_invariant (chrec))
> +  if (constant_range_value_p (chrec))
>      {
>        vr->set (chrec);
>        return;
> @@ -1779,7 +1779,7 @@ vr_values::adjust_range_with_scev (value
>       a simple expression, compare_values and possibly other functions
>       in tree-vrp won't be able to handle it.  */
>    if (step == NULL_TREE
> -      || !is_gimple_min_invariant (step)
> +      || !constant_range_value_p (step)
>        || !valid_value_p (init))
>      return;
>
> @@ -1841,7 +1841,7 @@ vr_values::adjust_range_with_scev (value
>
>                   if (TREE_CODE (init) == SSA_NAME)
>                     initvr = *(get_value_range (init));
> -                 else if (is_gimple_min_invariant (init))
> +                 else if (constant_range_value_p (init))
>                     initvr.set (init);
>                   else
>                     return;
> @@ -1995,7 +1995,7 @@ vrp_valueize (tree name)
>        const value_range *vr = x_vr_values->get_value_range (name);
>        if (vr->kind () == VR_RANGE
>           && (TREE_CODE (vr->min ()) == SSA_NAME
> -             || is_gimple_min_invariant (vr->min ()))
> +             || constant_range_value_p (vr->min ()))
>           && vrp_operand_equal_p (vr->min (), vr->max ()))
>         return vr->min ();
>      }
> @@ -2077,7 +2077,7 @@ vr_values::vrp_visit_assignment_or_call
>               extract_range_from_ssa_name (vr, tem);
>               return;
>             }
> -         else if (is_gimple_min_invariant (tem))
> +         else if (constant_range_value_p (tem))
>             {
>               vr->set (tem);
>               return;
> @@ -2475,7 +2475,7 @@ vr_values::vrp_evaluate_conditional (tre
>        enum warn_strict_overflow_code wc;
>        const char* warnmsg;
>
> -      if (is_gimple_min_invariant (ret))
> +      if (constant_range_value_p (ret))
>         {
>           wc = WARN_STRICT_OVERFLOW_CONDITIONAL;
>           warnmsg = G_("assuming signed overflow does not occur when "
> @@ -2517,7 +2517,7 @@ vr_values::vrp_evaluate_conditional (tre
>           && INTEGRAL_TYPE_P (type)
>           && vrp_val_is_min (vr0->min ())
>           && vrp_val_is_max (vr0->max ())
> -         && is_gimple_min_invariant (op1))
> +         && constant_range_value_p (op1))
>         {
>           location_t location;
>
> @@ -3361,14 +3361,14 @@ vr_values::simplify_bit_ops_using_ranges
>
>    if (TREE_CODE (op0) == SSA_NAME)
>      vr0 = *(get_value_range (op0));
> -  else if (is_gimple_min_invariant (op0))
> +  else if (constant_range_value_p (op0))
>      vr0.set (op0);
>    else
>      return false;
>
>    if (TREE_CODE (op1) == SSA_NAME)
>      vr1 = *(get_value_range (op1));
> -  else if (is_gimple_min_invariant (op1))
> +  else if (constant_range_value_p (op1))
>      vr1.set (op1);
>    else
>      return false;
> @@ -3481,7 +3481,7 @@ test_for_singularity (enum tree_code con
>        /* If the new min/max values have converged to a single value,
>          then there is only one value which can satisfy the condition,
>          return that value.  */
> -      if (operand_equal_p (min, max, 0) && is_gimple_min_invariant (min))
> +      if (operand_equal_p (min, max, 0) && constant_range_value_p (min))
>         return min;
>      }
>    return NULL;
> @@ -3554,7 +3554,7 @@ vr_values::simplify_cond_using_ranges_1
>        && cond_code != EQ_EXPR
>        && TREE_CODE (op0) == SSA_NAME
>        && INTEGRAL_TYPE_P (TREE_TYPE (op0))
> -      && is_gimple_min_invariant (op1))
> +      && constant_range_value_p (op1))
>      {
>        const value_range *vr = get_value_range (op0);
>

Re: Add a constant_range_value_p function (PR 92033)

[Richard Sandiford]

Richard Biener <richard.guenther@gmail.com> writes:
> On Fri, Oct 11, 2019 at 4:42 PM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> The range-tracking code has a pretty hard-coded assumption that
>> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
>> ADDR_EXPR".  It seems better to add a predicate specifically for
>> that rather than contiually fight cases in which it can't handle
>> other invariants.
>>
>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>
> ICK.  Nobody is going to remember this new restriction and
> constant_range_value_p reads like constant_value_range_p ;)
>
> Btw, is_gimple_invariant_address shouldn't have been exported,
> it's only use could have used is_gimple_min_invariant...

What do you think we should do instead?

Richard

>
> Richard.
>
>> Richard
>>
>>
>> 2019-10-11  Richard Sandiford  <richard.sandiford@arm.com>
>>
>> gcc/
>>         PR tree-optimization/92033
>>         * tree-vrp.h (constant_range_value_p): Declare.
>>         * tree-vrp.c (constant_range_value_p): New function.
>>         (value_range_base::symbolic_p, value_range_base::singleton_p)
>>         (get_single_symbol, compare_values_warnv, intersect_ranges)
>>         (value_range_base::normalize_symbolics): Use it instead of
>>         is_gimple_min_invariant.
>>         (simplify_stmt_for_jump_threading): Likewise.
>>         * vr-values.c (symbolic_range_based_on_p, valid_value_p): Likewise.
>>         (vr_values::op_with_constant_singleton_value_range): Likewise.
>>         (vr_values::extract_range_from_binary_expr): Likewise.
>>         (vr_values::extract_range_from_unary_expr): Likewise.
>>         (vr_values::extract_range_from_cond_expr): Likewise.
>>         (vr_values::extract_range_from_comparison): Likewise.
>>         (vr_values::extract_range_from_assignment): Likewise.
>>         (vr_values::adjust_range_with_scev, vrp_valueize): Likewise.
>>         (vr_values::vrp_visit_assignment_or_call): Likewise.
>>         (vr_values::vrp_evaluate_conditional): Likewise.
>>         (vr_values::simplify_bit_ops_using_ranges): Likewise.
>>         (test_for_singularity): Likewise.
>>         (vr_values::simplify_cond_using_ranges_1): Likewise.
>>
>> Index: gcc/tree-vrp.h
>> ===================================================================
>> --- gcc/tree-vrp.h      2019-10-08 09:23:31.282533990 +0100
>> +++ gcc/tree-vrp.h      2019-10-11 15:41:20.380576059 +0100
>> @@ -284,6 +284,7 @@ value_range_base::supports_type_p (tree
>>    return false;
>>  }
>>
>> +extern bool constant_range_value_p (const_tree);
>>  extern void register_edge_assert_for (tree, edge, enum tree_code,
>>                                       tree, tree, vec<assert_info> &);
>>  extern bool stmt_interesting_for_vrp (gimple *);
>> Index: gcc/tree-vrp.c
>> ===================================================================
>> --- gcc/tree-vrp.c      2019-10-08 09:23:31.282533990 +0100
>> +++ gcc/tree-vrp.c      2019-10-11 15:41:20.380576059 +0100
>> @@ -78,6 +78,18 @@ ranges_from_anti_range (const value_rang
>>     for still active basic-blocks.  */
>>  static sbitmap *live;
>>
>> +/* Return true if VALUE is considered constant for range tracking.
>> +   This is stricter than is_gimple_min_invariant and should be
>> +   used instead of it in range-related code.  */
>> +
>> +bool
>> +constant_range_value_p (const_tree value)
>> +{
>> +  return (TREE_CODE (value) == INTEGER_CST
>> +         || (TREE_CODE (value) == ADDR_EXPR
>> +             && is_gimple_invariant_address (value)));
>> +}
>> +
>>  void
>>  value_range::set_equiv (bitmap equiv)
>>  {
>> @@ -273,8 +285,8 @@ value_range_base::symbolic_p () const
>>  {
>>    return (!varying_p ()
>>           && !undefined_p ()
>> -         && (!is_gimple_min_invariant (m_min)
>> -             || !is_gimple_min_invariant (m_max)));
>> +         && (!constant_range_value_p (m_min)
>> +             || !constant_range_value_p (m_max)));
>>  }
>>
>>  /* NOTE: This is not the inverse of symbolic_p because the range
>> @@ -388,7 +400,7 @@ value_range_base::singleton_p (tree *res
>>      }
>>    if (m_kind == VR_RANGE
>>        && vrp_operand_equal_p (min (), max ())
>> -      && is_gimple_min_invariant (min ()))
>> +      && constant_range_value_p (min ()))
>>      {
>>        if (result)
>>          *result = min ();
>> @@ -953,13 +965,13 @@ get_single_symbol (tree t, bool *neg, tr
>>        || TREE_CODE (t) == POINTER_PLUS_EXPR
>>        || TREE_CODE (t) == MINUS_EXPR)
>>      {
>> -      if (is_gimple_min_invariant (TREE_OPERAND (t, 0)))
>> +      if (constant_range_value_p (TREE_OPERAND (t, 0)))
>>         {
>>           neg_ = (TREE_CODE (t) == MINUS_EXPR);
>>           inv_ = TREE_OPERAND (t, 0);
>>           t = TREE_OPERAND (t, 1);
>>         }
>> -      else if (is_gimple_min_invariant (TREE_OPERAND (t, 1)))
>> +      else if (constant_range_value_p (TREE_OPERAND (t, 1)))
>>         {
>>           neg_ = false;
>>           inv_ = TREE_OPERAND (t, 1);
>> @@ -1106,8 +1118,8 @@ compare_values_warnv (tree val1, tree va
>>                       TYPE_SIGN (TREE_TYPE (val1)));
>>      }
>>
>> -  const bool cst1 = is_gimple_min_invariant (val1);
>> -  const bool cst2 = is_gimple_min_invariant (val2);
>> +  const bool cst1 = constant_range_value_p (val1);
>> +  const bool cst2 = constant_range_value_p (val2);
>>
>>    /* If one is of the form '[-]NAME + CST' and the other is constant, then
>>       it might be possible to say something depending on the constants.  */
>> @@ -5785,7 +5797,7 @@ intersect_ranges (enum value_range_kind
>>       correct estimate unless VR1 is a constant singleton range
>>       in which case we choose that.  */
>>    if (vr1type == VR_RANGE
>> -      && is_gimple_min_invariant (vr1min)
>> +      && constant_range_value_p (vr1min)
>>        && vrp_operand_equal_p (vr1min, vr1max))
>>      {
>>        *vr0type = vr1type;
>> @@ -6045,8 +6057,8 @@ value_range_base::normalize_symbolics ()
>>    if (varying_p () || undefined_p ())
>>      return *this;
>>    tree ttype = type ();
>> -  bool min_symbolic = !is_gimple_min_invariant (min ());
>> -  bool max_symbolic = !is_gimple_min_invariant (max ());
>> +  bool min_symbolic = !constant_range_value_p (min ());
>> +  bool max_symbolic = !constant_range_value_p (max ());
>>    if (!min_symbolic && !max_symbolic)
>>      return normalize_addresses ();
>>
>> @@ -6432,7 +6444,7 @@ simplify_stmt_for_jump_threading (gimple
>>  {
>>    /* First see if the conditional is in the hash table.  */
>>    tree cached_lhs = avail_exprs_stack->lookup_avail_expr (stmt, false, true);
>> -  if (cached_lhs && is_gimple_min_invariant (cached_lhs))
>> +  if (cached_lhs && constant_range_value_p (cached_lhs))
>>      return cached_lhs;
>>
>>    vr_values *vr_values = x_vr_values;
>> Index: gcc/vr-values.c
>> ===================================================================
>> --- gcc/vr-values.c     2019-10-03 14:04:54.161520173 +0100
>> +++ gcc/vr-values.c     2019-10-11 15:41:20.380576059 +0100
>> @@ -263,14 +263,14 @@ symbolic_range_based_on_p (value_range_b
>>    bool neg, min_has_symbol, max_has_symbol;
>>    tree inv;
>>
>> -  if (is_gimple_min_invariant (vr->min ()))
>> +  if (constant_range_value_p (vr->min ()))
>>      min_has_symbol = false;
>>    else if (get_single_symbol (vr->min (), &neg, &inv) == sym)
>>      min_has_symbol = true;
>>    else
>>      return false;
>>
>> -  if (is_gimple_min_invariant (vr->max ()))
>> +  if (constant_range_value_p (vr->max ()))
>>      max_has_symbol = false;
>>    else if (get_single_symbol (vr->max (), &neg, &inv) == sym)
>>      max_has_symbol = true;
>> @@ -409,7 +409,7 @@ valid_value_p (tree expr)
>>      return (TREE_CODE (TREE_OPERAND (expr, 0)) == SSA_NAME
>>             && TREE_CODE (TREE_OPERAND (expr, 1)) == INTEGER_CST);
>>
>> -  return is_gimple_min_invariant (expr);
>> +  return constant_range_value_p (expr);
>>  }
>>
>>  /* If OP has a value range with a single constant value return that,
>> @@ -419,7 +419,7 @@ valid_value_p (tree expr)
>>  tree
>>  vr_values::op_with_constant_singleton_value_range (tree op)
>>  {
>> -  if (is_gimple_min_invariant (op))
>> +  if (constant_range_value_p (op))
>>      return op;
>>
>>    if (TREE_CODE (op) != SSA_NAME)
>> @@ -778,14 +778,14 @@ vr_values::extract_range_from_binary_exp
>>    value_range_base vr0, vr1;
>>    if (TREE_CODE (op0) == SSA_NAME)
>>      vr0 = *(get_value_range (op0));
>> -  else if (is_gimple_min_invariant (op0))
>> +  else if (constant_range_value_p (op0))
>>      vr0.set (op0);
>>    else
>>      vr0.set_varying (TREE_TYPE (op0));
>>
>>    if (TREE_CODE (op1) == SSA_NAME)
>>      vr1 = *(get_value_range (op1));
>> -  else if (is_gimple_min_invariant (op1))
>> +  else if (constant_range_value_p (op1))
>>      vr1.set (op1);
>>    else
>>      vr1.set_varying (TREE_TYPE (op1));
>> @@ -855,11 +855,11 @@ vr_values::extract_range_from_binary_exp
>>        value_range n_vr1;
>>
>>        /* Try with VR0 and [-INF, OP1].  */
>> -      if (is_gimple_min_invariant (minus_p ? vr0.max () : vr0.min ()))
>> +      if (constant_range_value_p (minus_p ? vr0.max () : vr0.min ()))
>>         n_vr1.set (VR_RANGE, vrp_val_min (expr_type), op1);
>>
>>        /* Try with VR0 and [OP1, +INF].  */
>> -      else if (is_gimple_min_invariant (minus_p ? vr0.min () : vr0.max ()))
>> +      else if (constant_range_value_p (minus_p ? vr0.min () : vr0.max ()))
>>         n_vr1.set (VR_RANGE, op1, vrp_val_max (expr_type));
>>
>>        /* Try with VR0 and [OP1, OP1].  */
>> @@ -879,11 +879,11 @@ vr_values::extract_range_from_binary_exp
>>        value_range n_vr0;
>>
>>        /* Try with [-INF, OP0] and VR1.  */
>> -      if (is_gimple_min_invariant (minus_p ? vr1.max () : vr1.min ()))
>> +      if (constant_range_value_p (minus_p ? vr1.max () : vr1.min ()))
>>         n_vr0.set (VR_RANGE, vrp_val_min (expr_type), op0);
>>
>>        /* Try with [OP0, +INF] and VR1.  */
>> -      else if (is_gimple_min_invariant (minus_p ? vr1.min (): vr1.max ()))
>> +      else if (constant_range_value_p (minus_p ? vr1.min (): vr1.max ()))
>>         n_vr0.set (VR_RANGE, op0, vrp_val_max (expr_type));
>>
>>        /* Try with [OP0, OP0] and VR1.  */
>> @@ -926,7 +926,7 @@ vr_values::extract_range_from_unary_expr
>>       a new value range with the operand to simplify processing.  */
>>    if (TREE_CODE (op0) == SSA_NAME)
>>      vr0 = *(get_value_range (op0));
>> -  else if (is_gimple_min_invariant (op0))
>> +  else if (constant_range_value_p (op0))
>>      vr0.set (op0);
>>    else
>>      vr0.set_varying (type);
>> @@ -948,7 +948,7 @@ vr_values::extract_range_from_cond_expr
>>    const value_range *vr0 = &tem0;
>>    if (TREE_CODE (op0) == SSA_NAME)
>>      vr0 = get_value_range (op0);
>> -  else if (is_gimple_min_invariant (op0))
>> +  else if (constant_range_value_p (op0))
>>      tem0.set (op0);
>>    else
>>      tem0.set_varying (TREE_TYPE (op0));
>> @@ -958,7 +958,7 @@ vr_values::extract_range_from_cond_expr
>>    const value_range *vr1 = &tem1;
>>    if (TREE_CODE (op1) == SSA_NAME)
>>      vr1 = get_value_range (op1);
>> -  else if (is_gimple_min_invariant (op1))
>> +  else if (constant_range_value_p (op1))
>>      tem1.set (op1);
>>    else
>>      tem1.set_varying (TREE_TYPE (op1));
>> @@ -987,7 +987,7 @@ vr_values::extract_range_from_comparison
>>          its type may be different from _Bool.  Convert VAL to EXPR's
>>          type.  */
>>        val = fold_convert (type, val);
>> -      if (is_gimple_min_invariant (val))
>> +      if (constant_range_value_p (val))
>>         vr->set (val);
>>        else
>>         vr->update (VR_RANGE, val, val);
>> @@ -1479,7 +1479,7 @@ vr_values::extract_range_from_assignment
>>                                    gimple_assign_rhs1 (stmt),
>>                                    gimple_assign_rhs2 (stmt));
>>    else if (get_gimple_rhs_class (code) == GIMPLE_SINGLE_RHS
>> -          && is_gimple_min_invariant (gimple_assign_rhs1 (stmt)))
>> +          && constant_range_value_p (gimple_assign_rhs1 (stmt)))
>>      vr->set (gimple_assign_rhs1 (stmt));
>>    else
>>      vr->set_varying (TREE_TYPE (gimple_assign_lhs (stmt)));
>> @@ -1756,7 +1756,7 @@ vr_values::adjust_range_with_scev (value
>>    chrec = instantiate_parameters (loop, analyze_scalar_evolution (loop, var));
>>
>>    /* Like in PR19590, scev can return a constant function.  */
>> -  if (is_gimple_min_invariant (chrec))
>> +  if (constant_range_value_p (chrec))
>>      {
>>        vr->set (chrec);
>>        return;
>> @@ -1779,7 +1779,7 @@ vr_values::adjust_range_with_scev (value
>>       a simple expression, compare_values and possibly other functions
>>       in tree-vrp won't be able to handle it.  */
>>    if (step == NULL_TREE
>> -      || !is_gimple_min_invariant (step)
>> +      || !constant_range_value_p (step)
>>        || !valid_value_p (init))
>>      return;
>>
>> @@ -1841,7 +1841,7 @@ vr_values::adjust_range_with_scev (value
>>
>>                   if (TREE_CODE (init) == SSA_NAME)
>>                     initvr = *(get_value_range (init));
>> -                 else if (is_gimple_min_invariant (init))
>> +                 else if (constant_range_value_p (init))
>>                     initvr.set (init);
>>                   else
>>                     return;
>> @@ -1995,7 +1995,7 @@ vrp_valueize (tree name)
>>        const value_range *vr = x_vr_values->get_value_range (name);
>>        if (vr->kind () == VR_RANGE
>>           && (TREE_CODE (vr->min ()) == SSA_NAME
>> -             || is_gimple_min_invariant (vr->min ()))
>> +             || constant_range_value_p (vr->min ()))
>>           && vrp_operand_equal_p (vr->min (), vr->max ()))
>>         return vr->min ();
>>      }
>> @@ -2077,7 +2077,7 @@ vr_values::vrp_visit_assignment_or_call
>>               extract_range_from_ssa_name (vr, tem);
>>               return;
>>             }
>> -         else if (is_gimple_min_invariant (tem))
>> +         else if (constant_range_value_p (tem))
>>             {
>>               vr->set (tem);
>>               return;
>> @@ -2475,7 +2475,7 @@ vr_values::vrp_evaluate_conditional (tre
>>        enum warn_strict_overflow_code wc;
>>        const char* warnmsg;
>>
>> -      if (is_gimple_min_invariant (ret))
>> +      if (constant_range_value_p (ret))
>>         {
>>           wc = WARN_STRICT_OVERFLOW_CONDITIONAL;
>>           warnmsg = G_("assuming signed overflow does not occur when "
>> @@ -2517,7 +2517,7 @@ vr_values::vrp_evaluate_conditional (tre
>>           && INTEGRAL_TYPE_P (type)
>>           && vrp_val_is_min (vr0->min ())
>>           && vrp_val_is_max (vr0->max ())
>> -         && is_gimple_min_invariant (op1))
>> +         && constant_range_value_p (op1))
>>         {
>>           location_t location;
>>
>> @@ -3361,14 +3361,14 @@ vr_values::simplify_bit_ops_using_ranges
>>
>>    if (TREE_CODE (op0) == SSA_NAME)
>>      vr0 = *(get_value_range (op0));
>> -  else if (is_gimple_min_invariant (op0))
>> +  else if (constant_range_value_p (op0))
>>      vr0.set (op0);
>>    else
>>      return false;
>>
>>    if (TREE_CODE (op1) == SSA_NAME)
>>      vr1 = *(get_value_range (op1));
>> -  else if (is_gimple_min_invariant (op1))
>> +  else if (constant_range_value_p (op1))
>>      vr1.set (op1);
>>    else
>>      return false;
>> @@ -3481,7 +3481,7 @@ test_for_singularity (enum tree_code con
>>        /* If the new min/max values have converged to a single value,
>>          then there is only one value which can satisfy the condition,
>>          return that value.  */
>> -      if (operand_equal_p (min, max, 0) && is_gimple_min_invariant (min))
>> +      if (operand_equal_p (min, max, 0) && constant_range_value_p (min))
>>         return min;
>>      }
>>    return NULL;
>> @@ -3554,7 +3554,7 @@ vr_values::simplify_cond_using_ranges_1
>>        && cond_code != EQ_EXPR
>>        && TREE_CODE (op0) == SSA_NAME
>>        && INTEGRAL_TYPE_P (TREE_TYPE (op0))
>> -      && is_gimple_min_invariant (op1))
>> +      && constant_range_value_p (op1))
>>      {
>>        const value_range *vr = get_value_range (op0);
>>

Re: Add a constant_range_value_p function (PR 92033)

[Richard Biener]

On October 14, 2019 2:32:43 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
>Richard Biener <richard.guenther@gmail.com> writes:
>> On Fri, Oct 11, 2019 at 4:42 PM Richard Sandiford
>> <richard.sandiford@arm.com> wrote:
>>>
>>> The range-tracking code has a pretty hard-coded assumption that
>>> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
>>> ADDR_EXPR".  It seems better to add a predicate specifically for
>>> that rather than contiually fight cases in which it can't handle
>>> other invariants.
>>>
>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>>
>> ICK.  Nobody is going to remember this new restriction and
>> constant_range_value_p reads like constant_value_range_p ;)
>>
>> Btw, is_gimple_invariant_address shouldn't have been exported,
>> it's only use could have used is_gimple_min_invariant...
>
>What do you think we should do instead?

Just handle POLY_INT_CST in a few place to quickly enough drop to varying. 

Richard. 

>Richard
>
>>
>> Richard.
>>
>>> Richard
>>>
>>>
>>> 2019-10-11  Richard Sandiford  <richard.sandiford@arm.com>
>>>
>>> gcc/
>>>         PR tree-optimization/92033
>>>         * tree-vrp.h (constant_range_value_p): Declare.
>>>         * tree-vrp.c (constant_range_value_p): New function.
>>>         (value_range_base::symbolic_p,
>value_range_base::singleton_p)
>>>         (get_single_symbol, compare_values_warnv, intersect_ranges)
>>>         (value_range_base::normalize_symbolics): Use it instead of
>>>         is_gimple_min_invariant.
>>>         (simplify_stmt_for_jump_threading): Likewise.
>>>         * vr-values.c (symbolic_range_based_on_p, valid_value_p):
>Likewise.
>>>         (vr_values::op_with_constant_singleton_value_range):
>Likewise.
>>>         (vr_values::extract_range_from_binary_expr): Likewise.
>>>         (vr_values::extract_range_from_unary_expr): Likewise.
>>>         (vr_values::extract_range_from_cond_expr): Likewise.
>>>         (vr_values::extract_range_from_comparison): Likewise.
>>>         (vr_values::extract_range_from_assignment): Likewise.
>>>         (vr_values::adjust_range_with_scev, vrp_valueize): Likewise.
>>>         (vr_values::vrp_visit_assignment_or_call): Likewise.
>>>         (vr_values::vrp_evaluate_conditional): Likewise.
>>>         (vr_values::simplify_bit_ops_using_ranges): Likewise.
>>>         (test_for_singularity): Likewise.
>>>         (vr_values::simplify_cond_using_ranges_1): Likewise.
>>>
>>> Index: gcc/tree-vrp.h
>>> ===================================================================
>>> --- gcc/tree-vrp.h      2019-10-08 09:23:31.282533990 +0100
>>> +++ gcc/tree-vrp.h      2019-10-11 15:41:20.380576059 +0100
>>> @@ -284,6 +284,7 @@ value_range_base::supports_type_p (tree
>>>    return false;
>>>  }
>>>
>>> +extern bool constant_range_value_p (const_tree);
>>>  extern void register_edge_assert_for (tree, edge, enum tree_code,
>>>                                       tree, tree, vec<assert_info>
>&);
>>>  extern bool stmt_interesting_for_vrp (gimple *);
>>> Index: gcc/tree-vrp.c
>>> ===================================================================
>>> --- gcc/tree-vrp.c      2019-10-08 09:23:31.282533990 +0100
>>> +++ gcc/tree-vrp.c      2019-10-11 15:41:20.380576059 +0100
>>> @@ -78,6 +78,18 @@ ranges_from_anti_range (const value_rang
>>>     for still active basic-blocks.  */
>>>  static sbitmap *live;
>>>
>>> +/* Return true if VALUE is considered constant for range tracking.
>>> +   This is stricter than is_gimple_min_invariant and should be
>>> +   used instead of it in range-related code.  */
>>> +
>>> +bool
>>> +constant_range_value_p (const_tree value)
>>> +{
>>> +  return (TREE_CODE (value) == INTEGER_CST
>>> +         || (TREE_CODE (value) == ADDR_EXPR
>>> +             && is_gimple_invariant_address (value)));
>>> +}
>>> +
>>>  void
>>>  value_range::set_equiv (bitmap equiv)
>>>  {
>>> @@ -273,8 +285,8 @@ value_range_base::symbolic_p () const
>>>  {
>>>    return (!varying_p ()
>>>           && !undefined_p ()
>>> -         && (!is_gimple_min_invariant (m_min)
>>> -             || !is_gimple_min_invariant (m_max)));
>>> +         && (!constant_range_value_p (m_min)
>>> +             || !constant_range_value_p (m_max)));
>>>  }
>>>
>>>  /* NOTE: This is not the inverse of symbolic_p because the range
>>> @@ -388,7 +400,7 @@ value_range_base::singleton_p (tree *res
>>>      }
>>>    if (m_kind == VR_RANGE
>>>        && vrp_operand_equal_p (min (), max ())
>>> -      && is_gimple_min_invariant (min ()))
>>> +      && constant_range_value_p (min ()))
>>>      {
>>>        if (result)
>>>          *result = min ();
>>> @@ -953,13 +965,13 @@ get_single_symbol (tree t, bool *neg, tr
>>>        || TREE_CODE (t) == POINTER_PLUS_EXPR
>>>        || TREE_CODE (t) == MINUS_EXPR)
>>>      {
>>> -      if (is_gimple_min_invariant (TREE_OPERAND (t, 0)))
>>> +      if (constant_range_value_p (TREE_OPERAND (t, 0)))
>>>         {
>>>           neg_ = (TREE_CODE (t) == MINUS_EXPR);
>>>           inv_ = TREE_OPERAND (t, 0);
>>>           t = TREE_OPERAND (t, 1);
>>>         }
>>> -      else if (is_gimple_min_invariant (TREE_OPERAND (t, 1)))
>>> +      else if (constant_range_value_p (TREE_OPERAND (t, 1)))
>>>         {
>>>           neg_ = false;
>>>           inv_ = TREE_OPERAND (t, 1);
>>> @@ -1106,8 +1118,8 @@ compare_values_warnv (tree val1, tree va
>>>                       TYPE_SIGN (TREE_TYPE (val1)));
>>>      }
>>>
>>> -  const bool cst1 = is_gimple_min_invariant (val1);
>>> -  const bool cst2 = is_gimple_min_invariant (val2);
>>> +  const bool cst1 = constant_range_value_p (val1);
>>> +  const bool cst2 = constant_range_value_p (val2);
>>>
>>>    /* If one is of the form '[-]NAME + CST' and the other is
>constant, then
>>>       it might be possible to say something depending on the
>constants.  */
>>> @@ -5785,7 +5797,7 @@ intersect_ranges (enum value_range_kind
>>>       correct estimate unless VR1 is a constant singleton range
>>>       in which case we choose that.  */
>>>    if (vr1type == VR_RANGE
>>> -      && is_gimple_min_invariant (vr1min)
>>> +      && constant_range_value_p (vr1min)
>>>        && vrp_operand_equal_p (vr1min, vr1max))
>>>      {
>>>        *vr0type = vr1type;
>>> @@ -6045,8 +6057,8 @@ value_range_base::normalize_symbolics ()
>>>    if (varying_p () || undefined_p ())
>>>      return *this;
>>>    tree ttype = type ();
>>> -  bool min_symbolic = !is_gimple_min_invariant (min ());
>>> -  bool max_symbolic = !is_gimple_min_invariant (max ());
>>> +  bool min_symbolic = !constant_range_value_p (min ());
>>> +  bool max_symbolic = !constant_range_value_p (max ());
>>>    if (!min_symbolic && !max_symbolic)
>>>      return normalize_addresses ();
>>>
>>> @@ -6432,7 +6444,7 @@ simplify_stmt_for_jump_threading (gimple
>>>  {
>>>    /* First see if the conditional is in the hash table.  */
>>>    tree cached_lhs = avail_exprs_stack->lookup_avail_expr (stmt,
>false, true);
>>> -  if (cached_lhs && is_gimple_min_invariant (cached_lhs))
>>> +  if (cached_lhs && constant_range_value_p (cached_lhs))
>>>      return cached_lhs;
>>>
>>>    vr_values *vr_values = x_vr_values;
>>> Index: gcc/vr-values.c
>>> ===================================================================
>>> --- gcc/vr-values.c     2019-10-03 14:04:54.161520173 +0100
>>> +++ gcc/vr-values.c     2019-10-11 15:41:20.380576059 +0100
>>> @@ -263,14 +263,14 @@ symbolic_range_based_on_p (value_range_b
>>>    bool neg, min_has_symbol, max_has_symbol;
>>>    tree inv;
>>>
>>> -  if (is_gimple_min_invariant (vr->min ()))
>>> +  if (constant_range_value_p (vr->min ()))
>>>      min_has_symbol = false;
>>>    else if (get_single_symbol (vr->min (), &neg, &inv) == sym)
>>>      min_has_symbol = true;
>>>    else
>>>      return false;
>>>
>>> -  if (is_gimple_min_invariant (vr->max ()))
>>> +  if (constant_range_value_p (vr->max ()))
>>>      max_has_symbol = false;
>>>    else if (get_single_symbol (vr->max (), &neg, &inv) == sym)
>>>      max_has_symbol = true;
>>> @@ -409,7 +409,7 @@ valid_value_p (tree expr)
>>>      return (TREE_CODE (TREE_OPERAND (expr, 0)) == SSA_NAME
>>>             && TREE_CODE (TREE_OPERAND (expr, 1)) == INTEGER_CST);
>>>
>>> -  return is_gimple_min_invariant (expr);
>>> +  return constant_range_value_p (expr);
>>>  }
>>>
>>>  /* If OP has a value range with a single constant value return
>that,
>>> @@ -419,7 +419,7 @@ valid_value_p (tree expr)
>>>  tree
>>>  vr_values::op_with_constant_singleton_value_range (tree op)
>>>  {
>>> -  if (is_gimple_min_invariant (op))
>>> +  if (constant_range_value_p (op))
>>>      return op;
>>>
>>>    if (TREE_CODE (op) != SSA_NAME)
>>> @@ -778,14 +778,14 @@ vr_values::extract_range_from_binary_exp
>>>    value_range_base vr0, vr1;
>>>    if (TREE_CODE (op0) == SSA_NAME)
>>>      vr0 = *(get_value_range (op0));
>>> -  else if (is_gimple_min_invariant (op0))
>>> +  else if (constant_range_value_p (op0))
>>>      vr0.set (op0);
>>>    else
>>>      vr0.set_varying (TREE_TYPE (op0));
>>>
>>>    if (TREE_CODE (op1) == SSA_NAME)
>>>      vr1 = *(get_value_range (op1));
>>> -  else if (is_gimple_min_invariant (op1))
>>> +  else if (constant_range_value_p (op1))
>>>      vr1.set (op1);
>>>    else
>>>      vr1.set_varying (TREE_TYPE (op1));
>>> @@ -855,11 +855,11 @@ vr_values::extract_range_from_binary_exp
>>>        value_range n_vr1;
>>>
>>>        /* Try with VR0 and [-INF, OP1].  */
>>> -      if (is_gimple_min_invariant (minus_p ? vr0.max () : vr0.min
>()))
>>> +      if (constant_range_value_p (minus_p ? vr0.max () : vr0.min
>()))
>>>         n_vr1.set (VR_RANGE, vrp_val_min (expr_type), op1);
>>>
>>>        /* Try with VR0 and [OP1, +INF].  */
>>> -      else if (is_gimple_min_invariant (minus_p ? vr0.min () :
>vr0.max ()))
>>> +      else if (constant_range_value_p (minus_p ? vr0.min () :
>vr0.max ()))
>>>         n_vr1.set (VR_RANGE, op1, vrp_val_max (expr_type));
>>>
>>>        /* Try with VR0 and [OP1, OP1].  */
>>> @@ -879,11 +879,11 @@ vr_values::extract_range_from_binary_exp
>>>        value_range n_vr0;
>>>
>>>        /* Try with [-INF, OP0] and VR1.  */
>>> -      if (is_gimple_min_invariant (minus_p ? vr1.max () : vr1.min
>()))
>>> +      if (constant_range_value_p (minus_p ? vr1.max () : vr1.min
>()))
>>>         n_vr0.set (VR_RANGE, vrp_val_min (expr_type), op0);
>>>
>>>        /* Try with [OP0, +INF] and VR1.  */
>>> -      else if (is_gimple_min_invariant (minus_p ? vr1.min ():
>vr1.max ()))
>>> +      else if (constant_range_value_p (minus_p ? vr1.min ():
>vr1.max ()))
>>>         n_vr0.set (VR_RANGE, op0, vrp_val_max (expr_type));
>>>
>>>        /* Try with [OP0, OP0] and VR1.  */
>>> @@ -926,7 +926,7 @@ vr_values::extract_range_from_unary_expr
>>>       a new value range with the operand to simplify processing.  */
>>>    if (TREE_CODE (op0) == SSA_NAME)
>>>      vr0 = *(get_value_range (op0));
>>> -  else if (is_gimple_min_invariant (op0))
>>> +  else if (constant_range_value_p (op0))
>>>      vr0.set (op0);
>>>    else
>>>      vr0.set_varying (type);
>>> @@ -948,7 +948,7 @@ vr_values::extract_range_from_cond_expr
>>>    const value_range *vr0 = &tem0;
>>>    if (TREE_CODE (op0) == SSA_NAME)
>>>      vr0 = get_value_range (op0);
>>> -  else if (is_gimple_min_invariant (op0))
>>> +  else if (constant_range_value_p (op0))
>>>      tem0.set (op0);
>>>    else
>>>      tem0.set_varying (TREE_TYPE (op0));
>>> @@ -958,7 +958,7 @@ vr_values::extract_range_from_cond_expr
>>>    const value_range *vr1 = &tem1;
>>>    if (TREE_CODE (op1) == SSA_NAME)
>>>      vr1 = get_value_range (op1);
>>> -  else if (is_gimple_min_invariant (op1))
>>> +  else if (constant_range_value_p (op1))
>>>      tem1.set (op1);
>>>    else
>>>      tem1.set_varying (TREE_TYPE (op1));
>>> @@ -987,7 +987,7 @@ vr_values::extract_range_from_comparison
>>>          its type may be different from _Bool.  Convert VAL to
>EXPR's
>>>          type.  */
>>>        val = fold_convert (type, val);
>>> -      if (is_gimple_min_invariant (val))
>>> +      if (constant_range_value_p (val))
>>>         vr->set (val);
>>>        else
>>>         vr->update (VR_RANGE, val, val);
>>> @@ -1479,7 +1479,7 @@ vr_values::extract_range_from_assignment
>>>                                    gimple_assign_rhs1 (stmt),
>>>                                    gimple_assign_rhs2 (stmt));
>>>    else if (get_gimple_rhs_class (code) == GIMPLE_SINGLE_RHS
>>> -          && is_gimple_min_invariant (gimple_assign_rhs1 (stmt)))
>>> +          && constant_range_value_p (gimple_assign_rhs1 (stmt)))
>>>      vr->set (gimple_assign_rhs1 (stmt));
>>>    else
>>>      vr->set_varying (TREE_TYPE (gimple_assign_lhs (stmt)));
>>> @@ -1756,7 +1756,7 @@ vr_values::adjust_range_with_scev (value
>>>    chrec = instantiate_parameters (loop, analyze_scalar_evolution
>(loop, var));
>>>
>>>    /* Like in PR19590, scev can return a constant function.  */
>>> -  if (is_gimple_min_invariant (chrec))
>>> +  if (constant_range_value_p (chrec))
>>>      {
>>>        vr->set (chrec);
>>>        return;
>>> @@ -1779,7 +1779,7 @@ vr_values::adjust_range_with_scev (value
>>>       a simple expression, compare_values and possibly other
>functions
>>>       in tree-vrp won't be able to handle it.  */
>>>    if (step == NULL_TREE
>>> -      || !is_gimple_min_invariant (step)
>>> +      || !constant_range_value_p (step)
>>>        || !valid_value_p (init))
>>>      return;
>>>
>>> @@ -1841,7 +1841,7 @@ vr_values::adjust_range_with_scev (value
>>>
>>>                   if (TREE_CODE (init) == SSA_NAME)
>>>                     initvr = *(get_value_range (init));
>>> -                 else if (is_gimple_min_invariant (init))
>>> +                 else if (constant_range_value_p (init))
>>>                     initvr.set (init);
>>>                   else
>>>                     return;
>>> @@ -1995,7 +1995,7 @@ vrp_valueize (tree name)
>>>        const value_range *vr = x_vr_values->get_value_range (name);
>>>        if (vr->kind () == VR_RANGE
>>>           && (TREE_CODE (vr->min ()) == SSA_NAME
>>> -             || is_gimple_min_invariant (vr->min ()))
>>> +             || constant_range_value_p (vr->min ()))
>>>           && vrp_operand_equal_p (vr->min (), vr->max ()))
>>>         return vr->min ();
>>>      }
>>> @@ -2077,7 +2077,7 @@ vr_values::vrp_visit_assignment_or_call
>>>               extract_range_from_ssa_name (vr, tem);
>>>               return;
>>>             }
>>> -         else if (is_gimple_min_invariant (tem))
>>> +         else if (constant_range_value_p (tem))
>>>             {
>>>               vr->set (tem);
>>>               return;
>>> @@ -2475,7 +2475,7 @@ vr_values::vrp_evaluate_conditional (tre
>>>        enum warn_strict_overflow_code wc;
>>>        const char* warnmsg;
>>>
>>> -      if (is_gimple_min_invariant (ret))
>>> +      if (constant_range_value_p (ret))
>>>         {
>>>           wc = WARN_STRICT_OVERFLOW_CONDITIONAL;
>>>           warnmsg = G_("assuming signed overflow does not occur when
>"
>>> @@ -2517,7 +2517,7 @@ vr_values::vrp_evaluate_conditional (tre
>>>           && INTEGRAL_TYPE_P (type)
>>>           && vrp_val_is_min (vr0->min ())
>>>           && vrp_val_is_max (vr0->max ())
>>> -         && is_gimple_min_invariant (op1))
>>> +         && constant_range_value_p (op1))
>>>         {
>>>           location_t location;
>>>
>>> @@ -3361,14 +3361,14 @@ vr_values::simplify_bit_ops_using_ranges
>>>
>>>    if (TREE_CODE (op0) == SSA_NAME)
>>>      vr0 = *(get_value_range (op0));
>>> -  else if (is_gimple_min_invariant (op0))
>>> +  else if (constant_range_value_p (op0))
>>>      vr0.set (op0);
>>>    else
>>>      return false;
>>>
>>>    if (TREE_CODE (op1) == SSA_NAME)
>>>      vr1 = *(get_value_range (op1));
>>> -  else if (is_gimple_min_invariant (op1))
>>> +  else if (constant_range_value_p (op1))
>>>      vr1.set (op1);
>>>    else
>>>      return false;
>>> @@ -3481,7 +3481,7 @@ test_for_singularity (enum tree_code con
>>>        /* If the new min/max values have converged to a single
>value,
>>>          then there is only one value which can satisfy the
>condition,
>>>          return that value.  */
>>> -      if (operand_equal_p (min, max, 0) && is_gimple_min_invariant
>(min))
>>> +      if (operand_equal_p (min, max, 0) && constant_range_value_p
>(min))
>>>         return min;
>>>      }
>>>    return NULL;
>>> @@ -3554,7 +3554,7 @@ vr_values::simplify_cond_using_ranges_1
>>>        && cond_code != EQ_EXPR
>>>        && TREE_CODE (op0) == SSA_NAME
>>>        && INTEGRAL_TYPE_P (TREE_TYPE (op0))
>>> -      && is_gimple_min_invariant (op1))
>>> +      && constant_range_value_p (op1))
>>>      {
>>>        const value_range *vr = get_value_range (op0);
>>>

Re: Add a constant_range_value_p function (PR 92033)

[Aldy Hernandez]

On 10/14/19 12:38 PM, Richard Biener wrote:
> On October 14, 2019 2:32:43 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
>> Richard Biener <richard.guenther@gmail.com> writes:
>>> On Fri, Oct 11, 2019 at 4:42 PM Richard Sandiford
>>> <richard.sandiford@arm.com> wrote:
>>>>
>>>> The range-tracking code has a pretty hard-coded assumption that
>>>> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
>>>> ADDR_EXPR".  It seems better to add a predicate specifically for
>>>> that rather than contiually fight cases in which it can't handle
>>>> other invariants.
>>>>
>>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>>>
>>> ICK.  Nobody is going to remember this new restriction and
>>> constant_range_value_p reads like constant_value_range_p ;)
>>>
>>> Btw, is_gimple_invariant_address shouldn't have been exported,
>>> it's only use could have used is_gimple_min_invariant...
>>
>> What do you think we should do instead?
> 
> Just handle POLY_INT_CST in a few place to quickly enough drop to varying.

I don't care either way, but I had originally suggested:

> I was going to suggest we normalize ranges to numerics completely before folding.  That is, replacing normalize_addresses() here:
> 
>   *vr = op->fold_range (expr_type,
>             vr0.normalize_addresses (),
>             vr1.normalize_addresses ());
> 
> ...into normalize_symbolics().

This will allow ranges of POLY_INTs to live throughout, but will get 
dropped right before any arithmetic on them.  At the least, it's a two 
line change (assuming there are no hidden gotchas ;-)).

Aldy

Re: Add a constant_range_value_p function (PR 92033)

[Richard Sandiford]

Richard Biener <richard.guenther@gmail.com> writes:
> On October 14, 2019 2:32:43 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
>>Richard Biener <richard.guenther@gmail.com> writes:
>>> On Fri, Oct 11, 2019 at 4:42 PM Richard Sandiford
>>> <richard.sandiford@arm.com> wrote:
>>>>
>>>> The range-tracking code has a pretty hard-coded assumption that
>>>> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
>>>> ADDR_EXPR".  It seems better to add a predicate specifically for
>>>> that rather than contiually fight cases in which it can't handle
>>>> other invariants.
>>>>
>>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>>>
>>> ICK.  Nobody is going to remember this new restriction and
>>> constant_range_value_p reads like constant_value_range_p ;)
>>>
>>> Btw, is_gimple_invariant_address shouldn't have been exported,
>>> it's only use could have used is_gimple_min_invariant...
>>
>>What do you think we should do instead?
>
> Just handle POLY_INT_CST in a few place to quickly enough drop to varying. 

OK, how about this?  Aldy's suggestion would be fine by me too,
but I thought I'd try this first given Aldy's queasiness about
allowing POLY_INT_CSTs further in.

The main case in which this gives useful ranges is a lower bound
of A + B * X becoming A when B >= 0.  E.g.:

  (1) [32 + 16X, 100] -> [32, 100]
  (2) [32 + 16X, 32 + 16X] -> [32, MAX]

But the same thing can be useful for the upper bound with negative
X coefficients.

We can revisit this later if keeping a singleton range for (2)
would be better.

Tested as before.

Richard


2019-10-15  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	PR middle-end/92033
	* poly-int.h (constant_lower_bound_with_limit): New function.
	(constant_upper_bound_with_limit): Likewise.
	* doc/poly-int.texi: Document them.
	* tree-vrp.c (value_range_base::set): Convert POLY_INT_CST bounds
	into the worst-case INTEGER_CST bounds.

Index: gcc/poly-int.h
===================================================================
--- gcc/poly-int.h	2019-07-10 19:41:26.395898027 +0100
+++ gcc/poly-int.h	2019-10-15 11:30:14.099625553 +0100
@@ -1528,6 +1528,29 @@ constant_lower_bound (const poly_int_pod
   return a.coeffs[0];
 }
 
+/* Return the constant lower bound of A, given that it is no less than B.  */
+
+template<unsigned int N, typename Ca, typename Cb>
+inline POLY_CONST_COEFF (Ca, Cb)
+constant_lower_bound_with_limit (const poly_int_pod<N, Ca> &a, const Cb &b)
+{
+  if (known_ge (a, b))
+    return a.coeffs[0];
+  return b;
+}
+
+/* Return the constant upper bound of A, given that it is no greater
+   than B.  */
+
+template<unsigned int N, typename Ca, typename Cb>
+inline POLY_CONST_COEFF (Ca, Cb)
+constant_upper_bound_with_limit (const poly_int_pod<N, Ca> &a, const Cb &b)
+{
+  if (known_le (a, b))
+    return a.coeffs[0];
+  return b;
+}
+
 /* Return a value that is known to be no greater than A and B.  This
    will be the greatest lower bound for some indeterminate values but
    not necessarily for all.  */
Index: gcc/doc/poly-int.texi
===================================================================
--- gcc/doc/poly-int.texi	2019-03-08 18:14:25.333011645 +0000
+++ gcc/doc/poly-int.texi	2019-10-15 11:30:14.099625553 +0100
@@ -803,6 +803,18 @@ the assertion is known to hold.
 @item constant_lower_bound (@var{a})
 Assert that @var{a} is nonnegative and return the smallest value it can have.
 
+@item constant_lower_bound_with_limit (@var{a}, @var{b})
+Return the least value @var{a} can have, given that the context in
+which @var{a} appears guarantees that the answer is no less than @var{b}.
+In other words, the caller is asserting that @var{a} is greater than or
+equal to @var{b} even if @samp{known_ge (@var{a}, @var{b})} doesn't hold.
+
+@item constant_upper_bound_with_limit (@var{a}, @var{b})
+Return the greatest value @var{a} can have, given that the context in
+which @var{a} appears guarantees that the answer is no greater than @var{b}.
+In other words, the caller is asserting that @var{a} is less than or equal
+to @var{b} even if @samp{known_le (@var{a}, @var{b})} doesn't hold.
+
 @item lower_bound (@var{a}, @var{b})
 Return a value that is always less than or equal to both @var{a} and @var{b}.
 It will be the greatest such value for some indeterminate values
Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	2019-10-14 09:04:54.515259320 +0100
+++ gcc/tree-vrp.c	2019-10-15 11:30:14.099625553 +0100
@@ -727,6 +727,24 @@ value_range_base::set (enum value_range_
       return;
     }
 
+  /* Convert POLY_INT_CST bounds into worst-case INTEGER_CST bounds.  */
+  if (POLY_INT_CST_P (min))
+    {
+      tree type_min = vrp_val_min (TREE_TYPE (min), true);
+      widest_int lb
+	= constant_lower_bound_with_limit (wi::to_poly_widest (min),
+					   wi::to_widest (type_min));
+      min = wide_int_to_tree (TREE_TYPE (min), lb);
+    }
+  if (POLY_INT_CST_P (max))
+    {
+      tree type_max = vrp_val_max (TREE_TYPE (max), true);
+      widest_int ub
+	= constant_upper_bound_with_limit (wi::to_poly_widest (max),
+					   wi::to_widest (type_max));
+      max = wide_int_to_tree (TREE_TYPE (max), ub);
+    }
+
   /* Nothing to canonicalize for symbolic ranges.  */
   if (TREE_CODE (min) != INTEGER_CST
       || TREE_CODE (max) != INTEGER_CST)

Re: Add a constant_range_value_p function (PR 92033)

[Richard Biener]

On Tue, Oct 15, 2019 at 12:35 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Richard Biener <richard.guenther@gmail.com> writes:
> > On October 14, 2019 2:32:43 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
> >>Richard Biener <richard.guenther@gmail.com> writes:
> >>> On Fri, Oct 11, 2019 at 4:42 PM Richard Sandiford
> >>> <richard.sandiford@arm.com> wrote:
> >>>>
> >>>> The range-tracking code has a pretty hard-coded assumption that
> >>>> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
> >>>> ADDR_EXPR".  It seems better to add a predicate specifically for
> >>>> that rather than contiually fight cases in which it can't handle
> >>>> other invariants.
> >>>>
> >>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
> >>>
> >>> ICK.  Nobody is going to remember this new restriction and
> >>> constant_range_value_p reads like constant_value_range_p ;)
> >>>
> >>> Btw, is_gimple_invariant_address shouldn't have been exported,
> >>> it's only use could have used is_gimple_min_invariant...
> >>
> >>What do you think we should do instead?
> >
> > Just handle POLY_INT_CST in a few place to quickly enough drop to varying.
>
> OK, how about this?  Aldy's suggestion would be fine by me too,
> but I thought I'd try this first given Aldy's queasiness about
> allowing POLY_INT_CSTs further in.
>
> The main case in which this gives useful ranges is a lower bound
> of A + B * X becoming A when B >= 0.  E.g.:
>
>   (1) [32 + 16X, 100] -> [32, 100]
>   (2) [32 + 16X, 32 + 16X] -> [32, MAX]
>
> But the same thing can be useful for the upper bound with negative
> X coefficients.
>
> We can revisit this later if keeping a singleton range for (2)
> would be better.
>
> Tested as before.

Works for me.

Richard.

> Richard
>
>
> 2019-10-15  Richard Sandiford  <richard.sandiford@arm.com>
>
> gcc/
>         PR middle-end/92033
>         * poly-int.h (constant_lower_bound_with_limit): New function.
>         (constant_upper_bound_with_limit): Likewise.
>         * doc/poly-int.texi: Document them.
>         * tree-vrp.c (value_range_base::set): Convert POLY_INT_CST bounds
>         into the worst-case INTEGER_CST bounds.
>
> Index: gcc/poly-int.h
> ===================================================================
> --- gcc/poly-int.h      2019-07-10 19:41:26.395898027 +0100
> +++ gcc/poly-int.h      2019-10-15 11:30:14.099625553 +0100
> @@ -1528,6 +1528,29 @@ constant_lower_bound (const poly_int_pod
>    return a.coeffs[0];
>  }
>
> +/* Return the constant lower bound of A, given that it is no less than B.  */
> +
> +template<unsigned int N, typename Ca, typename Cb>
> +inline POLY_CONST_COEFF (Ca, Cb)
> +constant_lower_bound_with_limit (const poly_int_pod<N, Ca> &a, const Cb &b)
> +{
> +  if (known_ge (a, b))
> +    return a.coeffs[0];
> +  return b;
> +}
> +
> +/* Return the constant upper bound of A, given that it is no greater
> +   than B.  */
> +
> +template<unsigned int N, typename Ca, typename Cb>
> +inline POLY_CONST_COEFF (Ca, Cb)
> +constant_upper_bound_with_limit (const poly_int_pod<N, Ca> &a, const Cb &b)
> +{
> +  if (known_le (a, b))
> +    return a.coeffs[0];
> +  return b;
> +}
> +
>  /* Return a value that is known to be no greater than A and B.  This
>     will be the greatest lower bound for some indeterminate values but
>     not necessarily for all.  */
> Index: gcc/doc/poly-int.texi
> ===================================================================
> --- gcc/doc/poly-int.texi       2019-03-08 18:14:25.333011645 +0000
> +++ gcc/doc/poly-int.texi       2019-10-15 11:30:14.099625553 +0100
> @@ -803,6 +803,18 @@ the assertion is known to hold.
>  @item constant_lower_bound (@var{a})
>  Assert that @var{a} is nonnegative and return the smallest value it can have.
>
> +@item constant_lower_bound_with_limit (@var{a}, @var{b})
> +Return the least value @var{a} can have, given that the context in
> +which @var{a} appears guarantees that the answer is no less than @var{b}.
> +In other words, the caller is asserting that @var{a} is greater than or
> +equal to @var{b} even if @samp{known_ge (@var{a}, @var{b})} doesn't hold.
> +
> +@item constant_upper_bound_with_limit (@var{a}, @var{b})
> +Return the greatest value @var{a} can have, given that the context in
> +which @var{a} appears guarantees that the answer is no greater than @var{b}.
> +In other words, the caller is asserting that @var{a} is less than or equal
> +to @var{b} even if @samp{known_le (@var{a}, @var{b})} doesn't hold.
> +
>  @item lower_bound (@var{a}, @var{b})
>  Return a value that is always less than or equal to both @var{a} and @var{b}.
>  It will be the greatest such value for some indeterminate values
> Index: gcc/tree-vrp.c
> ===================================================================
> --- gcc/tree-vrp.c      2019-10-14 09:04:54.515259320 +0100
> +++ gcc/tree-vrp.c      2019-10-15 11:30:14.099625553 +0100
> @@ -727,6 +727,24 @@ value_range_base::set (enum value_range_
>        return;
>      }
>
> +  /* Convert POLY_INT_CST bounds into worst-case INTEGER_CST bounds.  */
> +  if (POLY_INT_CST_P (min))
> +    {
> +      tree type_min = vrp_val_min (TREE_TYPE (min), true);
> +      widest_int lb
> +       = constant_lower_bound_with_limit (wi::to_poly_widest (min),
> +                                          wi::to_widest (type_min));
> +      min = wide_int_to_tree (TREE_TYPE (min), lb);
> +    }
> +  if (POLY_INT_CST_P (max))
> +    {
> +      tree type_max = vrp_val_max (TREE_TYPE (max), true);
> +      widest_int ub
> +       = constant_upper_bound_with_limit (wi::to_poly_widest (max),
> +                                          wi::to_widest (type_max));
> +      max = wide_int_to_tree (TREE_TYPE (max), ub);
> +    }
> +
>    /* Nothing to canonicalize for symbolic ranges.  */
>    if (TREE_CODE (min) != INTEGER_CST
>        || TREE_CODE (max) != INTEGER_CST)

Re: Add a constant_range_value_p function (PR 92033)

[Christophe Lyon]

On Tue, 15 Oct 2019 at 12:36, Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Richard Biener <richard.guenther@gmail.com> writes:
> > On October 14, 2019 2:32:43 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
> >>Richard Biener <richard.guenther@gmail.com> writes:
> >>> On Fri, Oct 11, 2019 at 4:42 PM Richard Sandiford
> >>> <richard.sandiford@arm.com> wrote:
> >>>>
> >>>> The range-tracking code has a pretty hard-coded assumption that
> >>>> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
> >>>> ADDR_EXPR".  It seems better to add a predicate specifically for
> >>>> that rather than contiually fight cases in which it can't handle
> >>>> other invariants.
> >>>>
> >>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
> >>>
> >>> ICK.  Nobody is going to remember this new restriction and
> >>> constant_range_value_p reads like constant_value_range_p ;)
> >>>
> >>> Btw, is_gimple_invariant_address shouldn't have been exported,
> >>> it's only use could have used is_gimple_min_invariant...
> >>
> >>What do you think we should do instead?
> >
> > Just handle POLY_INT_CST in a few place to quickly enough drop to varying.
>
> OK, how about this?  Aldy's suggestion would be fine by me too,
> but I thought I'd try this first given Aldy's queasiness about
> allowing POLY_INT_CSTs further in.
>
> The main case in which this gives useful ranges is a lower bound
> of A + B * X becoming A when B >= 0.  E.g.:
>
>   (1) [32 + 16X, 100] -> [32, 100]
>   (2) [32 + 16X, 32 + 16X] -> [32, MAX]
>
> But the same thing can be useful for the upper bound with negative
> X coefficients.
>
> We can revisit this later if keeping a singleton range for (2)
> would be better.
>
> Tested as before.
>
> Richard
>
>
Hi Richard,

This patch did improve aarch64 results quite a lot, however, there are
still a few failures that used to pass circa r276650:
    gcc.target/aarch64/sve/loop_add_6.c -march=armv8.2-a+sve
scan-assembler \\tfsub\\tz[0-9]+\\.d, p[0-7]/m
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tadd\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b,
z[0-9]+\\.b\\n 1
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tadd\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
z[0-9]+\\.d\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tadd\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
z[0-9]+\\.h\\n 1
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tadd\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
z[0-9]+\\.s\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tand\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b,
z[0-9]+\\.b\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tand\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
z[0-9]+\\.d\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tand\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
z[0-9]+\\.h\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tand\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
z[0-9]+\\.s\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\teor\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b,
z[0-9]+\\.b\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\teor\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
z[0-9]+\\.d\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\teor\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
z[0-9]+\\.h\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\teor\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
z[0-9]+\\.s\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tfadd\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
z[0-9]+\\.d\\n 1
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tfadd\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
z[0-9]+\\.h\\n 1
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\tfadd\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
z[0-9]+\\.s\\n 1
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\torr\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b,
z[0-9]+\\.b\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\torr\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
z[0-9]+\\.d\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\torr\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
z[0-9]+\\.h\\n 2
    gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
scan-assembler-times \\torr\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
z[0-9]+\\.s\\n 2
    gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
scan-assembler-times \\tfsub\\tz[0-9]+\\.d, p[0-7]/m 1
    gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
scan-assembler-times \\tfsub\\tz[0-9]+\\.s, p[0-7]/m 1
    gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
scan-assembler-times \\tsub\\tz[0-9]+\\.b, p[0-7]/m 1
    gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
scan-assembler-times \\tsub\\tz[0-9]+\\.d, p[0-7]/m 2
    gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
scan-assembler-times \\tsub\\tz[0-9]+\\.h, p[0-7]/m 1
    gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
scan-assembler-times \\tsub\\tz[0-9]+\\.s, p[0-7]/m 2

Just to make sure you are aware of these :-)

Thanks,

Christophe

> 2019-10-15  Richard Sandiford  <richard.sandiford@arm.com>
>
> gcc/
>         PR middle-end/92033
>         * poly-int.h (constant_lower_bound_with_limit): New function.
>         (constant_upper_bound_with_limit): Likewise.
>         * doc/poly-int.texi: Document them.
>         * tree-vrp.c (value_range_base::set): Convert POLY_INT_CST bounds
>         into the worst-case INTEGER_CST bounds.
>
> Index: gcc/poly-int.h
> ===================================================================
> --- gcc/poly-int.h      2019-07-10 19:41:26.395898027 +0100
> +++ gcc/poly-int.h      2019-10-15 11:30:14.099625553 +0100
> @@ -1528,6 +1528,29 @@ constant_lower_bound (const poly_int_pod
>    return a.coeffs[0];
>  }
>
> +/* Return the constant lower bound of A, given that it is no less than B.  */
> +
> +template<unsigned int N, typename Ca, typename Cb>
> +inline POLY_CONST_COEFF (Ca, Cb)
> +constant_lower_bound_with_limit (const poly_int_pod<N, Ca> &a, const Cb &b)
> +{
> +  if (known_ge (a, b))
> +    return a.coeffs[0];
> +  return b;
> +}
> +
> +/* Return the constant upper bound of A, given that it is no greater
> +   than B.  */
> +
> +template<unsigned int N, typename Ca, typename Cb>
> +inline POLY_CONST_COEFF (Ca, Cb)
> +constant_upper_bound_with_limit (const poly_int_pod<N, Ca> &a, const Cb &b)
> +{
> +  if (known_le (a, b))
> +    return a.coeffs[0];
> +  return b;
> +}
> +
>  /* Return a value that is known to be no greater than A and B.  This
>     will be the greatest lower bound for some indeterminate values but
>     not necessarily for all.  */
> Index: gcc/doc/poly-int.texi
> ===================================================================
> --- gcc/doc/poly-int.texi       2019-03-08 18:14:25.333011645 +0000
> +++ gcc/doc/poly-int.texi       2019-10-15 11:30:14.099625553 +0100
> @@ -803,6 +803,18 @@ the assertion is known to hold.
>  @item constant_lower_bound (@var{a})
>  Assert that @var{a} is nonnegative and return the smallest value it can have.
>
> +@item constant_lower_bound_with_limit (@var{a}, @var{b})
> +Return the least value @var{a} can have, given that the context in
> +which @var{a} appears guarantees that the answer is no less than @var{b}.
> +In other words, the caller is asserting that @var{a} is greater than or
> +equal to @var{b} even if @samp{known_ge (@var{a}, @var{b})} doesn't hold.
> +
> +@item constant_upper_bound_with_limit (@var{a}, @var{b})
> +Return the greatest value @var{a} can have, given that the context in
> +which @var{a} appears guarantees that the answer is no greater than @var{b}.
> +In other words, the caller is asserting that @var{a} is less than or equal
> +to @var{b} even if @samp{known_le (@var{a}, @var{b})} doesn't hold.
> +
>  @item lower_bound (@var{a}, @var{b})
>  Return a value that is always less than or equal to both @var{a} and @var{b}.
>  It will be the greatest such value for some indeterminate values
> Index: gcc/tree-vrp.c
> ===================================================================
> --- gcc/tree-vrp.c      2019-10-14 09:04:54.515259320 +0100
> +++ gcc/tree-vrp.c      2019-10-15 11:30:14.099625553 +0100
> @@ -727,6 +727,24 @@ value_range_base::set (enum value_range_
>        return;
>      }
>
> +  /* Convert POLY_INT_CST bounds into worst-case INTEGER_CST bounds.  */
> +  if (POLY_INT_CST_P (min))
> +    {
> +      tree type_min = vrp_val_min (TREE_TYPE (min), true);
> +      widest_int lb
> +       = constant_lower_bound_with_limit (wi::to_poly_widest (min),
> +                                          wi::to_widest (type_min));
> +      min = wide_int_to_tree (TREE_TYPE (min), lb);
> +    }
> +  if (POLY_INT_CST_P (max))
> +    {
> +      tree type_max = vrp_val_max (TREE_TYPE (max), true);
> +      widest_int ub
> +       = constant_upper_bound_with_limit (wi::to_poly_widest (max),
> +                                          wi::to_widest (type_max));
> +      max = wide_int_to_tree (TREE_TYPE (max), ub);
> +    }
> +
>    /* Nothing to canonicalize for symbolic ranges.  */
>    if (TREE_CODE (min) != INTEGER_CST
>        || TREE_CODE (max) != INTEGER_CST)

Re: Add a constant_range_value_p function (PR 92033)

[Richard Sandiford]

Christophe Lyon <christophe.lyon@linaro.org> writes:
> On Tue, 15 Oct 2019 at 12:36, Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> Richard Biener <richard.guenther@gmail.com> writes:
>> > On October 14, 2019 2:32:43 PM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
>> >>Richard Biener <richard.guenther@gmail.com> writes:
>> >>> On Fri, Oct 11, 2019 at 4:42 PM Richard Sandiford
>> >>> <richard.sandiford@arm.com> wrote:
>> >>>>
>> >>>> The range-tracking code has a pretty hard-coded assumption that
>> >>>> is_gimple_min_invariant is equivalent to "INTEGER_CST or invariant
>> >>>> ADDR_EXPR".  It seems better to add a predicate specifically for
>> >>>> that rather than contiually fight cases in which it can't handle
>> >>>> other invariants.
>> >>>>
>> >>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>> >>>
>> >>> ICK.  Nobody is going to remember this new restriction and
>> >>> constant_range_value_p reads like constant_value_range_p ;)
>> >>>
>> >>> Btw, is_gimple_invariant_address shouldn't have been exported,
>> >>> it's only use could have used is_gimple_min_invariant...
>> >>
>> >>What do you think we should do instead?
>> >
>> > Just handle POLY_INT_CST in a few place to quickly enough drop to varying.
>>
>> OK, how about this?  Aldy's suggestion would be fine by me too,
>> but I thought I'd try this first given Aldy's queasiness about
>> allowing POLY_INT_CSTs further in.
>>
>> The main case in which this gives useful ranges is a lower bound
>> of A + B * X becoming A when B >= 0.  E.g.:
>>
>>   (1) [32 + 16X, 100] -> [32, 100]
>>   (2) [32 + 16X, 32 + 16X] -> [32, MAX]
>>
>> But the same thing can be useful for the upper bound with negative
>> X coefficients.
>>
>> We can revisit this later if keeping a singleton range for (2)
>> would be better.
>>
>> Tested as before.
>>
>> Richard
>>
>>
> Hi Richard,
>
> This patch did improve aarch64 results quite a lot, however, there are
> still a few failures that used to pass circa r276650:
>     gcc.target/aarch64/sve/loop_add_6.c -march=armv8.2-a+sve
> scan-assembler \\tfsub\\tz[0-9]+\\.d, p[0-7]/m
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\tadd\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b,
> z[0-9]+\\.b\\n 1
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\tadd\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
> z[0-9]+\\.d\\n 2
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\tadd\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
> z[0-9]+\\.h\\n 1
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\tadd\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
> z[0-9]+\\.s\\n 2
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\tand\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b,
> z[0-9]+\\.b\\n 2
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\tand\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
> z[0-9]+\\.d\\n 2
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\tand\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
> z[0-9]+\\.h\\n 2
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\tand\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
> z[0-9]+\\.s\\n 2
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\teor\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b,
> z[0-9]+\\.b\\n 2
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\teor\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
> z[0-9]+\\.d\\n 2
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\teor\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
> z[0-9]+\\.h\\n 2
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\teor\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
> z[0-9]+\\.s\\n 2
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\tfadd\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
> z[0-9]+\\.d\\n 1
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\tfadd\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
> z[0-9]+\\.h\\n 1
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\tfadd\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
> z[0-9]+\\.s\\n 1
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\torr\\tz[0-9]+\\.b, p[0-7]/m, z[0-9]+\\.b,
> z[0-9]+\\.b\\n 2
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\torr\\tz[0-9]+\\.d, p[0-7]/m, z[0-9]+\\.d,
> z[0-9]+\\.d\\n 2
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\torr\\tz[0-9]+\\.h, p[0-7]/m, z[0-9]+\\.h,
> z[0-9]+\\.h\\n 2
>     gcc.target/aarch64/sve/reduc_1.c -march=armv8.2-a+sve
> scan-assembler-times \\torr\\tz[0-9]+\\.s, p[0-7]/m, z[0-9]+\\.s,
> z[0-9]+\\.s\\n 2
>     gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
> scan-assembler-times \\tfsub\\tz[0-9]+\\.d, p[0-7]/m 1
>     gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
> scan-assembler-times \\tfsub\\tz[0-9]+\\.s, p[0-7]/m 1
>     gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
> scan-assembler-times \\tsub\\tz[0-9]+\\.b, p[0-7]/m 1
>     gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
> scan-assembler-times \\tsub\\tz[0-9]+\\.d, p[0-7]/m 2
>     gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
> scan-assembler-times \\tsub\\tz[0-9]+\\.h, p[0-7]/m 1
>     gcc.target/aarch64/sve/reduc_5.c -march=armv8.2-a+sve
> scan-assembler-times \\tsub\\tz[0-9]+\\.s, p[0-7]/m 2
>
> Just to make sure you are aware of these :-)

Yeah, aware of them :-)  These were UNRESOLVED before the patch due
to the VRP ICEs.  I assume they're from the recent reduction changes,
but I haven't had chance to look at them in detail yet.

Thanks,
Richard