On 8/11/20 7:53 AM, Aldy Hernandez via Gcc-patches wrote:
---------- Forwarded message ---------
From: Aldy Hernandez <al...@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: <l...@redhat.com>, Aldy Hernandez <al...@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 *);

Reply via email to