On Sat, Nov 18, 2017 at 9:39 AM, Jeff Law <l...@redhat.com> wrote:
>
> A batch of random cleanups in the various vrp related bits.
>
> The goal here is to start banging some of the classes into shape.
> Included in this patch:
>
>
> For evrp_folder:
> Accepts vr_values in its ctor to attach.
> Uses DISABLE_COPY_AND_ASSIGN
>
> For evrp_dom_walker:
> Has the evrp_folder attached to it to avoid continually
> recreating an evrp_folder instance.
> Delegators removed, created free value_range_constant_singleton
> along the way to help remove one of them.
>
> For evrp_range_analyzer:
> The attached vr_values member is now private.
> 3 delegators are kept and made part of the public API
> The remaining half-dozen or so delegators are gone
>
> For vr_values:
> Privatizes several data members
> Privatizes many methods
>
>
> I'm still not at all happy with management of the attached vr_values
> class instance. I was going to use our unique_ptr, but without examples
> my attempts failed. I'll have to give it another whirl when I'm fresher.
>
> Bootstrapped and regression tested on x86_64.
>
> OK for the trunk once the vr_values attachment code is cleaned up and it
> goes through another round of testing?
Ok.
Thanks,
Richard.
> jeff
>
>
>
>
>
> * gimple-ssa-evrp-analyze.c (evrp_range_analyzer::evrp_range_analyzer)
> Initialize vr_values.
> (evrp_range_analyzer::try_find_new_range): Call methods attached to
> vr_values via vr_values class instance rather than delegators.
> (evrp_range_analyzer::record_ranges_from_phis): Likewise.
> (evrp_range_analyzer::record_ranges_from_stmt): Likewise.
> (evrp_range_analyzer::push_value_range): Likewise.
> (evrp_range_analyzer::pop_value_range): Likewise.
> * gimple-ssa-evrp-analyze.h (class evrp_range_analyzer): Remove
> most delegators. Those remaining are exposed as public interfaces.
> Make vr_values a pointer.
> (evrp_range_analyzer::~evrp_range_analyzer): Conditionally
> delete the attached vr_values.
> (evrp_range_analyzer::get_vr_value): New method.
> * gimple-ssa-evrp.c (class evrp_folder): Use DISABLE_COPY_AND_ASSIGN.
> (evrp_folder::evrp_folder): New ctor to initialize vr_values.
> (class evrp_dom_walker): Attach evrp_folder class, initialize
> it in the ctor. Remove temporary delegators.
> (evrp_dom_walker::before_dom_children): Call methods in attached
> evrp_range_analyzer class via class instance pointer. Use
> free value_range_constant_singleton to remove need for
> op_with_constant_singleton_value delegator method. Do not
> create a vrp_prop class instance for every call! Narrow
> scope of a couple variables.
> (evrp_dom_walker::cleanup): Call methods in attached
> evrp_range_analyzer class via class instance pointer.
> * vr-values.h (class vr_values): Privatize many methods and
> data members.
>
>
> diff --git a/gcc/gimple-ssa-evrp-analyze.c b/gcc/gimple-ssa-evrp-analyze.c
> index 9e581834d08..c3877791a5e 100644
> --- a/gcc/gimple-ssa-evrp-analyze.c
> +++ b/gcc/gimple-ssa-evrp-analyze.c
> @@ -53,6 +53,7 @@ evrp_range_analyzer::evrp_range_analyzer () : stack (10)
> FOR_EACH_EDGE (e, ei, bb->preds)
> e->flags |= EDGE_EXECUTABLE;
> }
> + vr_values = new class vr_values;
> }
>
> void
> @@ -73,8 +74,8 @@ evrp_range_analyzer::try_find_new_range (tree name,
> value_range *old_vr = get_value_range (name);
>
> /* Discover VR when condition is true. */
> - extract_range_for_var_from_comparison_expr (name, code, op,
> - limit, &vr);
> + vr_values->extract_range_for_var_from_comparison_expr (name, code, op,
> + limit, &vr);
> /* If we found any usable VR, set the VR to ssa_name and create a
> PUSH old value in the stack with the old VR. */
> if (vr.type == VR_RANGE || vr.type == VR_ANTI_RANGE)
> @@ -83,7 +84,7 @@ evrp_range_analyzer::try_find_new_range (tree name,
> && vrp_operand_equal_p (old_vr->min, vr.min)
> && vrp_operand_equal_p (old_vr->max, vr.max))
> return NULL;
> - value_range *new_vr = vr_values.vrp_value_range_pool.allocate ();
> + value_range *new_vr = vr_values->vrp_value_range_pool.allocate ();
> *new_vr = vr;
> return new_vr;
> }
> @@ -167,7 +168,7 @@ evrp_range_analyzer::record_ranges_from_phis (basic_block
> bb)
> value_range vr_result = VR_INITIALIZER;
> bool interesting = stmt_interesting_for_vrp (phi);
> if (!has_unvisited_preds && interesting)
> - extract_range_from_phi_node (phi, &vr_result);
> + vr_values->extract_range_from_phi_node (phi, &vr_result);
> else
> {
> set_value_range_to_varying (&vr_result);
> @@ -179,9 +180,9 @@ evrp_range_analyzer::record_ranges_from_phis (basic_block
> bb)
> if (interesting
> && (l = loop_containing_stmt (phi))
> && l->header == gimple_bb (phi))
> - adjust_range_with_scev (&vr_result, l, phi, lhs);
> + vr_values->adjust_range_with_scev (&vr_result, l, phi, lhs);
> }
> - update_value_range (lhs, &vr_result);
> + vr_values->update_value_range (lhs, &vr_result);
>
> /* Set the SSA with the value range. */
> if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
> @@ -216,11 +217,11 @@ evrp_range_analyzer::record_ranges_from_stmt (gimple
> *stmt)
> {
> edge taken_edge;
> value_range vr = VR_INITIALIZER;
> - extract_range_from_stmt (stmt, &taken_edge, &output, &vr);
> + vr_values->extract_range_from_stmt (stmt, &taken_edge, &output, &vr);
> if (output
> && (vr.type == VR_RANGE || vr.type == VR_ANTI_RANGE))
> {
> - update_value_range (output, &vr);
> + vr_values->update_value_range (output, &vr);
>
> /* Set the SSA with the value range. */
> if (INTEGRAL_TYPE_P (TREE_TYPE (output)))
> @@ -240,10 +241,10 @@ evrp_range_analyzer::record_ranges_from_stmt (gimple
> *stmt)
> set_ptr_nonnull (output);
> }
> else
> - set_defs_to_varying (stmt);
> + vr_values->set_defs_to_varying (stmt);
> }
> else
> - set_defs_to_varying (stmt);
> + vr_values->set_defs_to_varying (stmt);
>
> /* See if we can derive a range for any of STMT's operands. */
> tree op;
> @@ -319,7 +320,7 @@ evrp_range_analyzer::push_value_range (tree var,
> value_range *vr)
> fprintf (dump_file, "\n");
> }
> stack.safe_push (std::make_pair (var, get_value_range (var)));
> - set_vr_value (var, vr);
> + vr_values->set_vr_value (var, vr);
> }
>
> /* Pop the Value Range from the vrp_stack and update VAR with it. */
> @@ -337,7 +338,7 @@ evrp_range_analyzer::pop_value_range (tree var)
> dump_value_range (dump_file, vr);
> fprintf (dump_file, "\n");
> }
> - set_vr_value (var, vr);
> + vr_values->set_vr_value (var, vr);
> stack.pop ();
> return vr;
> }
> diff --git a/gcc/gimple-ssa-evrp-analyze.h b/gcc/gimple-ssa-evrp-analyze.h
> index 2e6c609c45b..b60bba848cc 100644
> --- a/gcc/gimple-ssa-evrp-analyze.h
> +++ b/gcc/gimple-ssa-evrp-analyze.h
> @@ -24,16 +24,46 @@ class evrp_range_analyzer
> {
> public:
> evrp_range_analyzer (void);
> - ~evrp_range_analyzer (void) { stack.release (); }
> + ~evrp_range_analyzer (void)
> + {
> + if (vr_values)
> + delete vr_values;
> + stack.release ();
> + }
>
> void enter (basic_block);
> void leave (basic_block);
> void record_ranges_from_stmt (gimple *);
>
> - class vr_values vr_values;
> + /* Main interface to retrieve range information. */
> + value_range *get_value_range (const_tree op)
> + { return vr_values->get_value_range (op); }
> +
> + /* Dump all the current value ranges. This is primarily
> + a debugging interface. */
> + void dump_all_value_ranges (FILE *fp)
> + { vr_values->dump_all_value_ranges (fp); }
> +
> + /* A bit of a wart. This should ideally go away. */
> + void vrp_visit_cond_stmt (gcond *cond, edge *e)
> + { return vr_values->vrp_visit_cond_stmt (cond, e); }
> +
> + /* Get the underlying vr_values class instance. If TRANSFER is
> + true, then we are transferring ownership. Else we keep ownership.
> +
> + This should be converted to a unique_ptr. */
> + class vr_values *get_vr_values (bool transfer)
> + {
> + class vr_values *x = vr_values;
> + if (transfer)
> + vr_values = NULL;
> + return x;
> + }
>
> private:
> DISABLE_COPY_AND_ASSIGN (evrp_range_analyzer);
> + class vr_values *vr_values;
> +
> void push_value_range (tree var, value_range *vr);
> value_range *pop_value_range (tree var);
> value_range *try_find_new_range (tree, tree op, tree_code code, tree
> limit);
> @@ -42,30 +72,6 @@ class evrp_range_analyzer
>
> /* STACK holds the old VR. */
> auto_vec<std::pair <tree, value_range*> > stack;
> -
> - /* Temporary delegators. */
> - value_range *get_value_range (const_tree op)
> - { return vr_values.get_value_range (op); }
> - bool update_value_range (const_tree op, value_range *vr)
> - { return vr_values.update_value_range (op, vr); }
> - void extract_range_from_phi_node (gphi *phi, value_range *vr)
> - { vr_values.extract_range_from_phi_node (phi, vr); }
> - void adjust_range_with_scev (value_range *vr, struct loop *loop,
> - gimple *stmt, tree var)
> - { vr_values.adjust_range_with_scev (vr, loop, stmt, var); }
> - void extract_range_from_stmt (gimple *stmt, edge *taken_edge_p,
> - tree *output_p, value_range *vr)
> - { vr_values.extract_range_from_stmt (stmt, taken_edge_p, output_p, vr); }
> - void set_defs_to_varying (gimple *stmt)
> - { return vr_values.set_defs_to_varying (stmt); }
> - void set_vr_value (tree name, value_range *vr)
> - { vr_values.set_vr_value (name, vr); }
> - void extract_range_for_var_from_comparison_expr (tree var,
> - enum tree_code cond_code,
> - tree op, tree limit,
> - value_range *vr_p)
> - { vr_values.extract_range_for_var_from_comparison_expr (var, cond_code,
> - op, limit, vr_p);
> }
> };
>
> #endif /* GCC_GIMPLE_SSA_EVRP_ANALYZE_H */
> diff --git a/gcc/gimple-ssa-evrp.c b/gcc/gimple-ssa-evrp.c
> index 27a983dd9ae..a554cf9e834 100644
> --- a/gcc/gimple-ssa-evrp.c
> +++ b/gcc/gimple-ssa-evrp.c
> @@ -46,8 +46,11 @@ class evrp_folder : public substitute_and_fold_engine
> {
> public:
> tree get_value (tree) FINAL OVERRIDE;
> -
> + evrp_folder (class vr_values *vr_values_) : vr_values (vr_values_) { }
> class vr_values *vr_values;
> +
> + private:
> + DISABLE_COPY_AND_ASSIGN (evrp_folder);
> };
>
> tree
> @@ -63,7 +66,9 @@ evrp_folder::get_value (tree op)
> class evrp_dom_walker : public dom_walker
> {
> public:
> - evrp_dom_walker () : dom_walker (CDI_DOMINATORS)
> + evrp_dom_walker ()
> + : dom_walker (CDI_DOMINATORS),
> + evrp_folder (evrp_range_analyzer.get_vr_values (false))
> {
> need_eh_cleanup = BITMAP_ALLOC (NULL);
> }
> @@ -82,14 +87,7 @@ public:
> auto_vec<gimple *> stmts_to_remove;
>
> class evrp_range_analyzer evrp_range_analyzer;
> -
> - /* Temporary delegators. */
> - value_range *get_value_range (const_tree op)
> - { return evrp_range_analyzer.vr_values.get_value_range (op); }
> - tree op_with_constant_singleton_value_range (tree op)
> - { return
> evrp_range_analyzer.vr_values.op_with_constant_singleton_value_range (op); }
> - void vrp_visit_cond_stmt (gcond *cond, edge *e)
> - { evrp_range_analyzer.vr_values.vrp_visit_cond_stmt (cond, e); }
> + class evrp_folder evrp_folder;
> };
>
> edge
> @@ -108,8 +106,9 @@ evrp_dom_walker::before_dom_children (basic_block bb)
> if (virtual_operand_p (lhs))
> continue;
>
> + value_range *vr = evrp_range_analyzer.get_value_range (lhs);
> /* Mark PHIs whose lhs we fully propagate for removal. */
> - tree val = op_with_constant_singleton_value_range (lhs);
> + tree val = value_range_constant_singleton (vr);
> if (val && may_propagate_copy (lhs, val))
> {
> stmts_to_remove.safe_push (phi);
> @@ -139,7 +138,7 @@ evrp_dom_walker::before_dom_children (basic_block bb)
>
> if (gcond *cond = dyn_cast <gcond *> (stmt))
> {
> - vrp_visit_cond_stmt (cond, &taken_edge);
> + evrp_range_analyzer.vrp_visit_cond_stmt (cond, &taken_edge);
> if (taken_edge)
> {
> if (taken_edge->flags & EDGE_TRUE_VALUE)
> @@ -153,16 +152,15 @@ evrp_dom_walker::before_dom_children (basic_block bb)
> }
> else if (stmt_interesting_for_vrp (stmt))
> {
> - value_range vr = VR_INITIALIZER;
> output = get_output_for_vrp (stmt);
> if (output)
> {
> tree val;
> - vr = *get_value_range (output);
> + value_range *vr = evrp_range_analyzer.get_value_range (output);
>
> /* Mark stmts whose output we fully propagate for removal. */
> - if ((vr.type == VR_RANGE || vr.type == VR_ANTI_RANGE)
> - && (val = op_with_constant_singleton_value_range (output))
> + if ((vr->type == VR_RANGE || vr->type == VR_ANTI_RANGE)
> + && (val = value_range_constant_singleton (vr))
> && may_propagate_copy (output, val)
> && !stmt_could_throw_p (stmt)
> && !gimple_has_side_effects (stmt))
> @@ -174,8 +172,6 @@ evrp_dom_walker::before_dom_children (basic_block bb)
> }
>
> /* Try folding stmts with the VR discovered. */
> - class evrp_folder evrp_folder;
> - evrp_folder.vr_values = &evrp_range_analyzer.vr_values;
> bool did_replace = evrp_folder.replace_uses_in (stmt);
> if (fold_stmt (&gsi, follow_single_use_edges)
> || did_replace)
> @@ -222,7 +218,8 @@ evrp_dom_walker::before_dom_children (basic_block bb)
> if (TREE_CODE (arg) != SSA_NAME
> || virtual_operand_p (arg))
> continue;
> - tree val = op_with_constant_singleton_value_range (arg);
> + value_range *vr = evrp_range_analyzer.get_value_range (arg);
> + tree val = value_range_constant_singleton (vr);
> if (val && may_propagate_copy (arg, val))
> propagate_value (use_p, val);
> }
> @@ -245,7 +242,7 @@ evrp_dom_walker::cleanup (void)
> if (dump_file)
> {
> fprintf (dump_file, "\nValue ranges after Early VRP:\n\n");
> - evrp_range_analyzer.vr_values.dump_all_value_ranges (dump_file);
> + evrp_range_analyzer.dump_all_value_ranges (dump_file);
> fprintf (dump_file, "\n");
> }
>
> diff --git a/gcc/vr-values.h b/gcc/vr-values.h
> index 24f013a9402..9eeebedfaed 100644
> --- a/gcc/vr-values.h
> +++ b/gcc/vr-values.h
> @@ -40,80 +40,81 @@ class vr_values
> vr_values (void);
> ~vr_values (void);
>
> - /* Value range array. After propagation, VR_VALUE[I] holds the range
> - of values that SSA name N_I may take. */
> - unsigned int num_vr_values;
> - value_range **vr_value;
> - bool values_propagated;
> + value_range *get_value_range (const_tree);
>
> - /* For a PHI node which sets SSA name N_I, VR_COUNTS[I] holds the
> - number of executable edges we saw the last time we visited the
> - node. */
> - int *vr_phi_edge_counts;
> + void set_vr_value (tree, value_range *);
> + void set_defs_to_varying (gimple *);
> + bool update_value_range (const_tree, value_range *);
> + tree op_with_constant_singleton_value_range (tree);
> + void adjust_range_with_scev (value_range *, struct loop *, gimple *, tree);
> + tree vrp_evaluate_conditional (tree_code, tree, tree, gimple *);
> + void dump_all_value_ranges (FILE *);
> +
> + void extract_range_for_var_from_comparison_expr (tree, enum tree_code,
> + tree, tree, value_range *);
> + void extract_range_from_phi_node (gphi *, value_range *);
> + void extract_range_basic (value_range *, gimple *);
> + void extract_range_from_assignment (value_range *, gassign *);
> + void extract_range_from_stmt (gimple *, edge *, tree *, value_range *);
> +
> + void vrp_visit_cond_stmt (gcond *, edge *);
> +
> + void simplify_cond_using_ranges_2 (gcond *);
> + bool simplify_stmt_using_ranges (gimple_stmt_iterator *);
> +
> + /* This probably belongs in the lattice rather than in here. */
> + bool values_propagated;
>
> /* Allocation pools for tree-vrp allocations. */
> object_allocator<value_range> vrp_value_range_pool;
> - bitmap_obstack vrp_equiv_obstack;
>
> - value_range *get_value_range (const_tree);
> - void set_vr_value (tree, value_range *);
> -
> - void set_defs_to_varying (gimple *);
> - bool update_value_range (const_tree, value_range *);
> + private:
> + bitmap_obstack vrp_equiv_obstack;
> void add_equivalence (bitmap *, const_tree);
> bool vrp_stmt_computes_nonzero (gimple *);
> - tree op_with_constant_singleton_value_range (tree);
> bool op_with_boolean_value_range_p (tree);
> bool check_for_binary_op_overflow (enum tree_code, tree, tree, tree, bool
> *);
> - void adjust_range_with_scev (value_range *, struct loop *, gimple *, tree);
> value_range get_vr_for_comparison (int);
> tree compare_name_with_value (enum tree_code, tree, tree, bool *, bool);
> tree compare_names (enum tree_code, tree, tree, bool *);
> bool two_valued_val_range_p (tree, tree *, tree *);
> -
> tree vrp_evaluate_conditional_warnv_with_ops_using_ranges (enum tree_code,
> tree, tree,
> bool *);
> tree vrp_evaluate_conditional_warnv_with_ops (enum tree_code,
> tree, tree, bool,
> bool *, bool *);
> - tree vrp_evaluate_conditional (tree_code, tree, tree, gimple *);
> -
> -
> - void dump_all_value_ranges (FILE *);
> -
> - void extract_range_for_var_from_comparison_expr (tree, enum tree_code,
> - tree, tree, value_range *);
> void extract_range_from_assert (value_range *, tree);
> void extract_range_from_ssa_name (value_range *, tree);
> void extract_range_from_binary_expr (value_range *, enum tree_code,
> tree, tree, tree);
> void extract_range_from_unary_expr (value_range *, enum tree_code,
> tree, tree);
> - void extract_range_from_phi_node (gphi *, value_range *);
> void extract_range_from_cond_expr (value_range *, gassign *);
> - void extract_range_basic (value_range *, gimple *);
> - void extract_range_from_assignment (value_range *, gassign *);
> - void extract_range_from_stmt (gimple *, edge *, tree *, value_range *);
> void extract_range_from_comparison (value_range *, enum tree_code,
> tree, tree, tree);
> -
> void vrp_visit_assignment_or_call (gimple*, tree *, value_range *);
> void vrp_visit_switch_stmt (gswitch *, edge *);
> - void vrp_visit_cond_stmt (gcond *, edge *);
> -
> 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 *);
> bool simplify_bit_ops_using_ranges (gimple_stmt_iterator *, gimple *);
> bool simplify_min_or_max_using_ranges (gimple_stmt_iterator *, gimple *);
> bool simplify_cond_using_ranges_1 (gcond *);
> - void simplify_cond_using_ranges_2 (gcond *);
> bool simplify_switch_using_ranges (gswitch *);
> bool simplify_float_conversion_using_ranges (gimple_stmt_iterator *,
> gimple *);
> bool simplify_internal_call_using_ranges (gimple_stmt_iterator *, gimple
> *);
> - bool simplify_stmt_using_ranges (gimple_stmt_iterator *);
> +
> + /* Value range array. After propagation, VR_VALUE[I] holds the range
> + of values that SSA name N_I may take. */
> + unsigned int num_vr_values;
> + value_range **vr_value;
> +
> + /* For a PHI node which sets SSA name N_I, VR_COUNTS[I] holds the
> + number of executable edges we saw the last time we visited the
> + node. */
> + int *vr_phi_edge_counts;
> };
>
> #define VR_INITIALIZER { VR_UNDEFINED, NULL_TREE, NULL_TREE, NULL }
>
