Add a constant_range_value_p function (PR 92033)

Aldy Hernandez aldyh@redhat.com
Sun Oct 13 16:22:00 GMT 2019


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



More information about the Gcc-patches mailing list