| Message ID | mpteezj738t.fsf@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add a constant_range_value_p function (PR 92033) | expand |
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); > >
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
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
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); >
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
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); >>
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
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); >>>
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
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)
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)
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)
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
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);