On Fri, Nov 17, 2017 at 8:41 AM, Jeff Law <l...@redhat.com> wrote:
> This patch introduces the evrp_range_analyzer class. This is the class
> we're going to be able to embed into existing dominator walkers to
> provide them with context sensitive range analysis.
>
> The bulk of the class is extracted from the evrp_dom_walker class. A
> few minor notes.
>
> Setup of the reachable/executable state for blocks/edges moves into the
> evrp_range_analyzer class's constructor from execute_early_vrp. It
> seems to bit better in the analyzer class as it sets state the analyzer
> uses internally.
>
> The analyzer has a enter/leave methods to call as well as a
> per-statement method to call. I went back and forth several times on
> whether or not to have a per-statement method call or hide the
> per-statement stuff behind the enter method. I could be easily
> convinced to return to the latter as it has a simpler interface for the
> clients.
>
> I suspect evrp_range_analyzer::get_value_range which is currently
> private and delegates to the embedded vr_values class to likely become a
> public member that continues to delegate to the embedded vr_values.
> Essentially it's the query interface for embedded range analysis.
> There's a couple other cleanups that need to happen with the goal that
> the vr_values data member can become private.
>
> Bootstrapped and regression tested on x86_64. OK for the trunk?
Ok.
Thanks,
Richard.
> jeff
>
>
>
> * gimple-ssa-evrp.c (class evrp_range_analyzer): New class extracted
> from evrp_dom_walker class. Various methods moved into new class.
> (evrp_range_analyzer::evrp_range_analyzer): Constructor for new class.
> (evrp_range_analyzer::enter): New method.
> (evrp_range_analyzer::leave): New method.
> (evrp_dom_walker): Remove delegators no longer needed by this class.
> Replace vr_values data member with evrp_range_analyzer
>
> commit 45facb8e6d15937db1135310dd9ff85f85c17d81
> Author: Jeff Law <l...@torsion.usersys.redhat.com>
> Date: Fri Nov 17 02:05:27 2017 -0500
>
> Introduce range_analyzer class
>
> diff --git a/gcc/gimple-ssa-evrp.c b/gcc/gimple-ssa-evrp.c
> index 7b92beb..bb56696 100644
> --- a/gcc/gimple-ssa-evrp.c
> +++ b/gcc/gimple-ssa-evrp.c
> @@ -55,6 +55,72 @@ evrp_folder::get_value (tree op)
> return vr_values->op_with_constant_singleton_value_range (op);
> }
>
> +class evrp_range_analyzer
> +{
> + public:
> + evrp_range_analyzer (void);
> + ~evrp_range_analyzer (void) { stack.release (); }
> +
> + void enter (basic_block);
> + void leave (basic_block);
> + void record_ranges_from_stmt (gimple *);
> +
> + class vr_values vr_values;
> +
> + private:
> + 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);
> + void record_ranges_from_incoming_edge (basic_block);
> + void record_ranges_from_phis (basic_block);
> +
> + /* We do not allow copying this object or initializing one from another.
> */
> + evrp_range_analyzer (const evrp_range_analyzer &);
> + evrp_range_analyzer& operator= (const evrp_range_analyzer &);
> +
> + /* Cond_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);
> }
> +};
> +
> +evrp_range_analyzer::evrp_range_analyzer () : stack (10)
> +{
> + edge e;
> + edge_iterator ei;
> + basic_block bb;
> +
> + FOR_EACH_BB_FN (bb, cfun)
> + {
> + bb->flags &= ~BB_VISITED;
> + FOR_EACH_EDGE (e, ei, bb->preds)
> + e->flags |= EDGE_EXECUTABLE;
> + }
> +}
> +
> +
> /* evrp_dom_walker visits the basic blocks in the dominance order and set
> the Value Ranges (VR) for SSA_NAMEs in the scope. Use this VR to
> discover more VRs. */
> @@ -62,8 +130,7 @@ evrp_folder::get_value (tree op)
> class evrp_dom_walker : public dom_walker
> {
> public:
> - evrp_dom_walker ()
> - : dom_walker (CDI_DOMINATORS), stack (10)
> + evrp_dom_walker () : dom_walker (CDI_DOMINATORS)
> {
> need_eh_cleanup = BITMAP_ALLOC (NULL);
> }
> @@ -76,21 +143,11 @@ public:
> void cleanup (void);
>
> private:
> - 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);
> -
> - void record_ranges_from_incoming_edge (basic_block);
> - void record_ranges_from_phis (basic_block);
> - void record_ranges_from_stmt (gimple *);
> -
> - /* Cond_stack holds the old VR. */
> - auto_vec<std::pair <tree, value_range*> > stack;
> bitmap need_eh_cleanup;
> auto_vec<gimple *> stmts_to_fixup;
> auto_vec<gimple *> stmts_to_remove;
>
> - class vr_values vr_values;
> + class evrp_range_analyzer evrp_range_analyzer;
>
> /* We do not allow copying this object or initializing one from another.
> */
> evrp_dom_walker (const evrp_dom_walker &);
> @@ -98,40 +155,25 @@ public:
>
> /* 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 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);
> }
> - 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); }
> + { return evrp_range_analyzer.vr_values.get_value_range (op); }
> tree op_with_constant_singleton_value_range (tree op)
> - { return vr_values.op_with_constant_singleton_value_range (op); }
> - 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 simplify_cond_using_ranges_2 (gcond *stmt)
> - { vr_values.simplify_cond_using_ranges_2 (stmt); }
> + { return
> evrp_range_analyzer.vr_values.op_with_constant_singleton_value_range (op); }
> void vrp_visit_cond_stmt (gcond *cond, edge *e)
> - { vr_values.vrp_visit_cond_stmt (cond, e); }
> + { evrp_range_analyzer.vr_values.vrp_visit_cond_stmt (cond, e); }
> };
>
> -/* Find new range for NAME such that (OP CODE LIMIT) is true. */
> +void
> +evrp_range_analyzer::enter (basic_block bb)
> +{
> + stack.safe_push (std::make_pair (NULL_TREE, (value_range *)NULL));
> + record_ranges_from_incoming_edge (bb);
> + record_ranges_from_phis (bb);
> +}
>
> +/* Find new range for NAME such that (OP CODE LIMIT) is true. */
> value_range *
> -evrp_dom_walker::try_find_new_range (tree name,
> - tree op, tree_code code, tree limit)
> +evrp_range_analyzer::try_find_new_range (tree name,
> + tree op, tree_code code, tree limit)
> {
> value_range vr = VR_INITIALIZER;
> value_range *old_vr = get_value_range (name);
> @@ -158,10 +200,8 @@ evrp_dom_walker::try_find_new_range (tree name,
> then derive ranges implied by traversing that edge. */
>
> void
> -evrp_dom_walker::record_ranges_from_incoming_edge (basic_block bb)
> +evrp_range_analyzer::record_ranges_from_incoming_edge (basic_block bb)
> {
> -/* See if there is any new scope is entered with new VR and set that VR to
> - ssa_name before visiting the statements in the scope. */
> edge pred_e = single_pred_edge_ignoring_loop_edges (bb, false);
> if (pred_e)
> {
> @@ -210,10 +250,8 @@ evrp_dom_walker::record_ranges_from_incoming_edge
> (basic_block bb)
> }
> }
>
> -/* Record ranges from any PHI nodes at the start of basic block BB. */
> -
> void
> -evrp_dom_walker::record_ranges_from_phis (basic_block bb)
> +evrp_range_analyzer::record_ranges_from_phis (basic_block bb)
> {
> /* Visit PHI stmts and discover any new VRs possible. */
> bool has_unvisited_preds = false;
> @@ -234,15 +272,10 @@ evrp_dom_walker::record_ranges_from_phis (basic_block
> bb)
> tree lhs = PHI_RESULT (phi);
> if (virtual_operand_p (lhs))
> continue;
> +
> value_range vr_result = VR_INITIALIZER;
> bool interesting = stmt_interesting_for_vrp (phi);
> - if (interesting && dump_file && (dump_flags & TDF_DETAILS))
> - {
> - fprintf (dump_file, "Visiting PHI node ");
> - print_gimple_stmt (dump_file, phi, 0);
> - }
> - if (!has_unvisited_preds
> - && interesting)
> + if (!has_unvisited_preds && interesting)
> extract_range_from_phi_node (phi, &vr_result);
> else
> {
> @@ -255,7 +288,7 @@ evrp_dom_walker::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);
> + adjust_range_with_scev (&vr_result, l, phi, lhs);
> }
> update_value_range (lhs, &vr_result);
>
> @@ -284,7 +317,7 @@ evrp_dom_walker::record_ranges_from_phis (basic_block bb)
> /* Record any ranges created by statement STMT. */
>
> void
> -evrp_dom_walker::record_ranges_from_stmt (gimple *stmt)
> +evrp_range_analyzer::record_ranges_from_stmt (gimple *stmt)
> {
> tree output = NULL_TREE;
>
> @@ -303,8 +336,8 @@ evrp_dom_walker::record_ranges_from_stmt (gimple *stmt)
> if (INTEGRAL_TYPE_P (TREE_TYPE (output)))
> {
> if ((vr.type == VR_RANGE || vr.type == VR_ANTI_RANGE)
> - && (TREE_CODE (vr.min) == INTEGER_CST)
> - && (TREE_CODE (vr.max) == INTEGER_CST))
> + && (TREE_CODE (vr.min) == INTEGER_CST)
> + && (TREE_CODE (vr.max) == INTEGER_CST))
> set_range_info (output, vr.type,
> wi::to_wide (vr.min),
> wi::to_wide (vr.max));
> @@ -375,9 +408,7 @@ evrp_dom_walker::before_dom_children (basic_block bb)
> if (dump_file && (dump_flags & TDF_DETAILS))
> fprintf (dump_file, "Visiting BB%d\n", bb->index);
>
> - stack.safe_push (std::make_pair (NULL_TREE, (value_range *)NULL));
> - record_ranges_from_incoming_edge (bb);
> - record_ranges_from_phis (bb);
> + evrp_range_analyzer.enter (bb);
>
> for (gphi_iterator gpi = gsi_start_phis (bb);
> !gsi_end_p (gpi); gsi_next (&gpi))
> @@ -414,7 +445,8 @@ evrp_dom_walker::before_dom_children (basic_block bb)
> print_gimple_stmt (dump_file, stmt, 0);
> }
>
> - record_ranges_from_stmt (stmt);
> + evrp_range_analyzer.record_ranges_from_stmt (stmt);
> +
> if (gcond *cond = dyn_cast <gcond *> (stmt))
> {
> vrp_visit_cond_stmt (cond, &taken_edge);
> @@ -435,10 +467,10 @@ evrp_dom_walker::before_dom_children (basic_block bb)
> output = get_output_for_vrp (stmt);
> if (output)
> {
> + tree val;
> vr = *get_value_range (output);
>
> /* Mark stmts whose output we fully propagate for removal. */
> - tree val;
> if ((vr.type == VR_RANGE || vr.type == VR_ANTI_RANGE)
> && (val = op_with_constant_singleton_value_range (output))
> && may_propagate_copy (output, val)
> @@ -453,7 +485,7 @@ 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 = &vr_values;
> + 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)
> @@ -511,10 +543,16 @@ evrp_dom_walker::before_dom_children (basic_block bb)
> return taken_edge;
> }
>
> +void
> +evrp_dom_walker::after_dom_children (basic_block bb)
> +{
> + evrp_range_analyzer.leave (bb);
> +}
> +
> /* Restore/pop VRs valid only for BB when we leave BB. */
>
> void
> -evrp_dom_walker::after_dom_children (basic_block bb ATTRIBUTE_UNUSED)
> +evrp_range_analyzer::leave (basic_block bb ATTRIBUTE_UNUSED)
> {
> gcc_checking_assert (!stack.is_empty ());
> while (stack.last ().first != NULL_TREE)
> @@ -525,7 +563,7 @@ evrp_dom_walker::after_dom_children (basic_block bb
> ATTRIBUTE_UNUSED)
> /* Push the Value Range of VAR to the stack and update it with new VR. */
>
> void
> -evrp_dom_walker::push_value_range (tree var, value_range *vr)
> +evrp_range_analyzer::push_value_range (tree var, value_range *vr)
> {
> if (dump_file && (dump_flags & TDF_DETAILS))
> {
> @@ -542,7 +580,7 @@ evrp_dom_walker::push_value_range (tree var, value_range
> *vr)
> /* Pop the Value Range from the vrp_stack and update VAR with it. */
>
> value_range *
> -evrp_dom_walker::pop_value_range (tree var)
> +evrp_range_analyzer::pop_value_range (tree var)
> {
> value_range *vr = stack.last ().second;
> gcc_checking_assert (var == stack.last ().first);
> @@ -567,7 +605,7 @@ evrp_dom_walker::cleanup (void)
> if (dump_file)
> {
> fprintf (dump_file, "\nValue ranges after Early VRP:\n\n");
> - vr_values.dump_all_value_ranges (dump_file);
> + evrp_range_analyzer.vr_values.dump_all_value_ranges (dump_file);
> fprintf (dump_file, "\n");
> }
>
> @@ -613,10 +651,6 @@ evrp_dom_walker::cleanup (void)
> static unsigned int
> execute_early_vrp ()
> {
> - edge e;
> - edge_iterator ei;
> - basic_block bb;
> -
> /* Ideally this setup code would move into the ctor for the dominator
> walk. However, this setup can change the number of blocks which
> invalidates the internal arrays that are set up by the dominator
> @@ -625,12 +659,6 @@ execute_early_vrp ()
> rewrite_into_loop_closed_ssa (NULL, TODO_update_ssa);
> scev_initialize ();
> calculate_dominance_info (CDI_DOMINATORS);
> - FOR_EACH_BB_FN (bb, cfun)
> - {
> - bb->flags &= ~BB_VISITED;
> - FOR_EACH_EDGE (e, ei, bb->preds)
> - e->flags |= EDGE_EXECUTABLE;
> - }
>
> /* Walk stmts in dominance order and propagate VRP. */
> evrp_dom_walker walker;
>
