Richard Biener <richard.guent...@gmail.com> writes: > On Fri, Oct 11, 2019 at 4:42 PM Richard Sandiford > <richard.sandif...@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.sandif...@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); >>