PING: Fwd: [PATCH 1/2] Add statement context to get_value_range.

Andrew MacLeod amacleod@redhat.com
Fri Aug 14 16:03:50 GMT 2020


On 8/11/20 7:53 AM, Aldy Hernandez via Gcc-patches wrote:
> ---------- Forwarded message ---------
> From: Aldy Hernandez <aldyh@redhat.com>
> Date: Tue, Aug 4, 2020, 13:55
> Subject: [PATCH 1/2] Add statement context to get_value_range.
> To: <gcc-patches@gcc.gnu.org>
> Cc: <law@redhat.com>, Aldy Hernandez <aldyh@redhat.com>
>
>
> This is in line with the statement context that we have for get_value()
> in the substitute_and_fold_engine class.
> ---
>   gcc/vr-values.c | 64 ++++++++++++++++++++++++++-----------------------
>   gcc/vr-values.h | 14 +++++------
>   2 files changed, 41 insertions(+), 37 deletions(-)
>
> diff --git a/gcc/vr-values.c b/gcc/vr-values.c
> index 511342f2f13..9002d87c14b 100644
> --- a/gcc/vr-values.c
> +++ b/gcc/vr-values.c
> @@ -147,7 +147,8 @@ vr_values::get_lattice_entry (const_tree var)
>      return NULL.  Otherwise create an empty range if none existed for VAR.
> */
>
>   const value_range_equiv *
> -vr_values::get_value_range (const_tree var)
> +vr_values::get_value_range (const_tree var,
> +                           gimple *stmt ATTRIBUTE_UNUSED)
>   {
>     /* If we have no recorded ranges, then return NULL.  */
>     if (!vr_value)
> @@ -450,7 +451,7 @@ simplify_using_ranges::op_with_boolean_value_range_p
> (tree op)
>
>     /* ?? Errr, this should probably check for [0,0] and [1,1] as well
>        as [0,1].  */
> -  const value_range *vr = get_value_range (op);
> +  const value_range *vr = get_value_range (op, NULL);
>     return *vr == value_range (build_zero_cst (TREE_TYPE (op)),
>                               build_one_cst (TREE_TYPE (op)));
>   }

I think if we are adding "gimple *stmt" as a parameter, we should make 
if default to NULL...  Then we won't have to change all the callers that 
don't have a need for it.
I get that it helped us find all the places where stmts were 
available/needed originally, but I think that need is no longer relevant 
and we can revert to making it a default parameter now.

further more, I don't think it should be a ATTRIBUTE_UNUSED, and then 
pass a NULL further down :)  we should be able to pass stmt.

> @@ -972,12 +973,13 @@ vr_values::extract_range_from_cond_expr
> (value_range_equiv *vr, gassign *stmt)
>
>   void
>   vr_values::extract_range_from_comparison (value_range_equiv *vr,
> +                                         gimple *stmt,
>                                            enum tree_code code,
>                                            tree type, tree op0, tree op1)

Now that we are passing stmt in, and there is only one use of this 
function, I think you can kill the final 4 parameters and just get them 
in the function itself...

>   {
>     bool sop;
>     tree val
> -    = simplifier.vrp_evaluate_conditional_warnv_with_ops (code, op0, op1,
> +    = simplifier.vrp_evaluate_conditional_warnv_with_ops (stmt, code, op0,
> op1,
>                                                            false, &sop,
> NULL);
>     if (val)
>       {
> @@ -1008,14 +1010,14 @@ check_for_binary_op_overflow (vr_values *store,
>   {
>     value_range vr0, vr1;
>     if (TREE_CODE (op0) == SSA_NAME)
> -    vr0 = *store->get_value_range (op0);
> +    vr0 = *store->get_value_range (op0, NULL);
>     else if (TREE_CODE (op0) == INTEGER_CST)
>       vr0.set (op0);
>     else
>       vr0.set_varying (TREE_TYPE (op0));
>
>     if (TREE_CODE (op1) == SSA_NAME)
> -    vr1 = *store->get_value_range (op1);
> +    vr1 = *store->get_value_range (op1, NULL);
>     else if (TREE_CODE (op1) == INTEGER_CST)
>       vr1.set (op1);
>     else
> @@ -1472,7 +1474,7 @@ vr_values::extract_range_from_assignment
> (value_range_equiv *vr, gassign *stmt)
>     else if (code == COND_EXPR)
>       extract_range_from_cond_expr (vr, stmt);
>     else if (TREE_CODE_CLASS (code) == tcc_comparison)
> -    extract_range_from_comparison (vr, gimple_assign_rhs_code (stmt),
> +    extract_range_from_comparison (vr, stmt, gimple_assign_rhs_code (stmt),
>                                     gimple_expr_type (stmt),
>                                     gimple_assign_rhs1 (stmt),
>                                     gimple_assign_rhs2 (stmt));
> @@ -1805,7 +1807,7 @@ vr_values::adjust_range_with_scev (value_range_equiv
> *vr, class loop *loop,
>     if (TREE_CODE (step) == INTEGER_CST
>         && is_gimple_val (init)
>         && (TREE_CODE (init) != SSA_NAME
> -         || get_value_range (init)->kind () == VR_RANGE))
> +         || get_value_range (init, stmt)->kind () == VR_RANGE))
>       {
>         widest_int nit;
>
> @@ -1838,7 +1840,7 @@ vr_values::adjust_range_with_scev (value_range_equiv
> *vr, class loop *loop,
>                    value_range initvr;
>
>                    if (TREE_CODE (init) == SSA_NAME)
> -                   initvr = *(get_value_range (init));
> +                   initvr = *(get_value_range (init, stmt));
>                    else if (is_gimple_min_invariant (init))
>                      initvr.set (init);
>                    else
> @@ -2090,7 +2092,7 @@ const value_range_equiv *
>   simplify_using_ranges::get_vr_for_comparison (int i, value_range_equiv
> *tem)
>   {
>     /* Shallow-copy equiv bitmap.  */
> -  const value_range_equiv *vr = get_value_range (ssa_name (i));
> +  const value_range_equiv *vr = get_value_range (ssa_name (i), NULL);
>
>     /* If name N_i does not have a valid range, use N_i as its own
>        range.  This allows us to compare against names that may
> @@ -2115,7 +2117,7 @@ simplify_using_ranges::compare_name_with_value
>                                   bool *strict_overflow_p, bool use_equiv_p)
>   {
>     /* Get the set of equivalences for VAR.  */
> -  bitmap e = get_value_range (var)->equiv ();
> +  bitmap e = get_value_range (var, NULL)->equiv ();
>
>     /* Start at -1.  Set it to 0 if we do a comparison without relying
>        on overflow, or 1 if all comparisons rely on overflow.  */
> @@ -2195,8 +2197,8 @@ simplify_using_ranges::compare_names (enum tree_code
> comp, tree n1, tree n2,
>   {
>     /* Compare the ranges of every name equivalent to N1 against the
>        ranges of every name equivalent to N2.  */
> -  bitmap e1 = get_value_range (n1)->equiv ();
> -  bitmap e2 = get_value_range (n2)->equiv ();
> +  bitmap e1 = get_value_range (n1, NULL)->equiv ();
> +  bitmap e2 = get_value_range (n2, NULL)->equiv ();
>
>     /* Use the fake bitmaps if e1 or e2 are not available.  */
>     static bitmap s_e1 = NULL, s_e2 = NULL;
> @@ -2308,8 +2310,8 @@
> simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops_using_ranges
>       (enum tree_code code, tree op0, tree op1, bool * strict_overflow_p)
>   {
>     const value_range_equiv *vr0, *vr1;
> -  vr0 = (TREE_CODE (op0) == SSA_NAME) ? get_value_range (op0) : NULL;
> -  vr1 = (TREE_CODE (op1) == SSA_NAME) ? get_value_range (op1) : NULL;
> +  vr0 = (TREE_CODE (op0) == SSA_NAME) ? get_value_range (op0, NULL) : NULL;
> +  vr1 = (TREE_CODE (op1) == SSA_NAME) ? get_value_range (op1, NULL) : NULL;
>
>     tree res = NULL_TREE;
>     if (vr0 && vr1)
> @@ -2326,7 +2328,8 @@
> simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops_using_ranges
>
>   tree
>   simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops
> -                                               (enum tree_code code,
> +                                               (gimple *stmt,
> +                                                enum tree_code code,
>                                                   tree op0, tree op1,
>                                                   bool use_equiv_p,
>                                                   bool *strict_overflow_p,

I was really hoping that by passing stmt in here, we could avoid passing 
code, op1 and op2 as well... but unfortunately, with further digging it 
seems that there are issues with VRP .   in particular there are places 
which tweak  op1 and op2 before being passed for consideration...   
specifically  vrp_evaluate_conditional  calls here and has incoming 
tweaks.  they should be fine as far as ranger interaction goes, but 
prevents us from condensing those parameters... :-(

So that is OK for now. Perhaps when we get into replacing VRP, we can 
clean this up more.



> @@ -2387,7 +2390,7 @@
> simplify_using_ranges::vrp_evaluate_conditional_warnv_with_ops
>              }
>            else
>              gcc_unreachable ();
> -         const value_range_equiv *vr0 = get_value_range (op0);
> +         const value_range_equiv *vr0 = get_value_range (op0, stmt);
>            /* If vro, the range for OP0 to pass the overflow test, has
>               no intersection with *vr0, OP0's known range, then the
>               overflow test can't pass, so return the node for false.
> @@ -2449,8 +2452,8 @@ simplify_using_ranges::vrp_evaluate_conditional
> (tree_code code, tree op0,
>       return NULL_TREE;
>
>     sop = false;
> -  ret = vrp_evaluate_conditional_warnv_with_ops (code, op0, op1, true,
> &sop,
> -                                                &only_ranges);
> +  ret = vrp_evaluate_conditional_warnv_with_ops (stmt, code, op0, op1,
> true,
> +                                                &sop, &only_ranges);
>
>     if (ret && sop)
>       {
> @@ -2493,7 +2496,7 @@ simplify_using_ranges::vrp_evaluate_conditional
> (tree_code code, tree op0,
>           always fold regardless of the value of OP0.  If -Wtype-limits
>           was specified, emit a warning.  */
>         tree type = TREE_TYPE (op0);
> -      const value_range_equiv *vr0 = get_value_range (op0);
> +      const value_range_equiv *vr0 = get_value_range (op0, stmt);
>
>         if (vr0->varying_p ()
>            && INTEGRAL_TYPE_P (type)
> @@ -2544,7 +2547,7 @@ simplify_using_ranges::vrp_visit_cond_stmt (gcond
> *stmt, edge *taken_edge_p)
>            fprintf (dump_file, "\t");
>            print_generic_expr (dump_file, use);
>            fprintf (dump_file, ": ");
> -         dump_value_range (dump_file, get_value_range (use));
> +         dump_value_range (dump_file, get_value_range (use, stmt));
>          }
>
>         fprintf (dump_file, "\n");
> @@ -2594,7 +2597,8 @@ simplify_using_ranges::vrp_visit_cond_stmt (gcond
> *stmt, edge *taken_edge_p)
>        4 more predicates folded in SPEC.  */
>
>     bool sop;
> -  val = vrp_evaluate_conditional_warnv_with_ops (gimple_cond_code (stmt),
> +  val = vrp_evaluate_conditional_warnv_with_ops (stmt,
> +                                                gimple_cond_code (stmt),
>                                                   gimple_cond_lhs (stmt),
>                                                   gimple_cond_rhs (stmt),
>                                                   false, &sop, NULL);
> @@ -3119,7 +3123,7 @@
> simplify_using_ranges::simplify_div_or_mod_using_ranges
>       }
>     else
>       {
> -      vr = get_value_range (op0);
> +      vr = get_value_range (op0, stmt);
>         if (range_int_cst_p (vr))
>          {
>            op0min = vr->min ();
> @@ -3130,7 +3134,7 @@
> simplify_using_ranges::simplify_div_or_mod_using_ranges
>     if (rhs_code == TRUNC_MOD_EXPR
>         && TREE_CODE (op1) == SSA_NAME)
>       {
> -      const value_range_equiv *vr1 = get_value_range (op1);
> +      const value_range_equiv *vr1 = get_value_range (op1, stmt);
>         if (range_int_cst_p (vr1))
>          op1min = vr1->min ();
>       }
> @@ -3279,7 +3283,7 @@ simplify_using_ranges::simplify_abs_using_ranges
> (gimple_stmt_iterator *gsi,
>                                                    gimple *stmt)
>   {
>     tree op = gimple_assign_rhs1 (stmt);
> -  const value_range *vr = get_value_range (op);
> +  const value_range *vr = get_value_range (op, stmt);
>
>     if (vr)
>       {
> @@ -3369,14 +3373,14 @@ simplify_using_ranges::simplify_bit_ops_using_ranges
>     wide_int mask;
>
>     if (TREE_CODE (op0) == SSA_NAME)
> -    vr0 = *(get_value_range (op0));
> +    vr0 = *(get_value_range (op0, stmt));
>     else if (is_gimple_min_invariant (op0))
>       vr0.set (op0);
>     else
>       return false;
>
>     if (TREE_CODE (op1) == SSA_NAME)
> -    vr1 = *(get_value_range (op1));
> +    vr1 = *(get_value_range (op1, stmt));
>     else if (is_gimple_min_invariant (op1))
>       vr1.set (op1);
>     else
> @@ -3595,7 +3599,7 @@ simplify_using_ranges::simplify_cond_using_ranges_1
> (gcond *stmt)
>         && INTEGRAL_TYPE_P (TREE_TYPE (op0))
>         && is_gimple_min_invariant (op1))
>       {
> -      const value_range *vr = get_value_range (op0);
> +      const value_range *vr = get_value_range (op0, stmt);
>
>         /* If we have range information for OP0, then we might be
>           able to simplify this conditional. */
> @@ -3739,7 +3743,7 @@ simplify_using_ranges::simplify_switch_using_ranges
> (gswitch *stmt)
>
>     if (TREE_CODE (op) == SSA_NAME)
>       {
> -      vr = get_value_range (op);
> +      vr = get_value_range (op, stmt);
>
>         /* We can only handle integer ranges.  */
>         if (vr->varying_p ()
> @@ -4032,7 +4036,7 @@
> simplify_using_ranges::simplify_float_conversion_using_ranges
>                                           gimple *stmt)
>   {
>     tree rhs1 = gimple_assign_rhs1 (stmt);
> -  const value_range *vr = get_value_range (rhs1);
> +  const value_range *vr = get_value_range (rhs1, stmt);
>     scalar_float_mode fltmode
>       = SCALAR_FLOAT_TYPE_MODE (TREE_TYPE (gimple_assign_lhs (stmt)));
>     scalar_int_mode mode;
> @@ -4196,7 +4200,7 @@
> simplify_using_ranges::simplify_internal_call_using_ranges
>   bool
>   simplify_using_ranges::two_valued_val_range_p (tree var, tree *a, tree *b)
>   {
> -  value_range vr = *get_value_range (var);
> +  value_range vr = *get_value_range (var, NULL);
>     vr.normalize_symbolics ();
>     if (vr.varying_p () || vr.undefined_p ())
>       return false;
> diff --git a/gcc/vr-values.h b/gcc/vr-values.h
> index 62a20218c6d..3fbea9bd69f 100644
> --- a/gcc/vr-values.h
> +++ b/gcc/vr-values.h
> @@ -38,12 +38,12 @@ public:
>     // ?? These should be cleaned, merged, and made private.
>     tree vrp_evaluate_conditional (tree_code, tree, tree, gimple *);
>     void vrp_visit_cond_stmt (gcond *, edge *);
> -  tree vrp_evaluate_conditional_warnv_with_ops (enum tree_code,
> +  tree vrp_evaluate_conditional_warnv_with_ops (gimple *stmt, enum
> tree_code,
>                                                  tree, tree, bool,
>                                                  bool *, bool *);
>
>   private:
> -  const value_range_equiv *get_value_range (const_tree op);
> +  const value_range_equiv *get_value_range (const_tree op, gimple *stmt);
>     bool simplify_truth_ops_using_ranges (gimple_stmt_iterator *, gimple *);
>     bool simplify_div_or_mod_using_ranges (gimple_stmt_iterator *, gimple *);
>     bool simplify_abs_using_ranges (gimple_stmt_iterator *, gimple *);
> @@ -101,7 +101,7 @@ class vr_values
>     vr_values (void);
>     ~vr_values (void);
>
> -  const value_range_equiv *get_value_range (const_tree);
> +  const value_range_equiv *get_value_range (const_tree, gimple * = NULL);
>     void set_vr_value (tree, value_range_equiv *);
>     value_range_equiv *swap_vr_value (tree, value_range_equiv *);
>
> @@ -140,8 +140,8 @@ class vr_values
>     void extract_range_from_unary_expr (value_range_equiv *, enum tree_code,
>                                        tree, tree);
>     void extract_range_from_cond_expr (value_range_equiv *, gassign *);
> -  void extract_range_from_comparison (value_range_equiv *, enum tree_code,
> -                                     tree, tree, tree);
> +  void extract_range_from_comparison (value_range_equiv *, gimple *,
> +                                     enum tree_code, tree, tree, tree);
>     void vrp_visit_assignment_or_call (gimple*, tree *, value_range_equiv *);
>     void vrp_visit_switch_stmt (gswitch *, edge *);
>
> @@ -167,9 +167,9 @@ class vr_values
>   };
>
>   inline const value_range_equiv *
> -simplify_using_ranges::get_value_range (const_tree op)
> +simplify_using_ranges::get_value_range (const_tree op, gimple *stmt)
>   {
> -  return store->get_value_range (op);
> +  return store->get_value_range (op, stmt);
>   }
>
>   extern tree get_output_for_vrp (gimple *);



More information about the Gcc-patches mailing list