On Fri, Nov 17, 2017 at 5:49 AM, Jeff Law <[email protected]> wrote:
> So with the major reorganization bits in place it's time to start
> cleaning up.
>
> This patch is primarily concerned with cleanups to the evrp_dom_walker
> class.
>
> We pull a blob of code from execute_early_vrp into a new member
> function. That subsequently allows various data members to become private.
>
> We declare a private copy constructor and move assignment operator to
> ensure that the evrp_dom_walker class is never copied.
>
> I'd like to pull more of execute_early_vrp into the class ctor/dtor.
> But those operations can change the number of basic blocks, and the dom
> walker class is unhappy -- it sizes internal arrays based on the number
> of basic blocks. Ugh.
>
> Anyway, small steps.
>
> Bootstrapped and regression tested on x86_64. OK for the trunk?
Ok.
Richard.
> Jeff
>
> * gimple-ssa-evrp.c (evrp_dom_walker): Add cleanup method.
> Add private copy constructor and move assignment operators.
> Privatize methods and class data where trivially possible.
> (evrp_dom_walker::cleanup): New function, extracted from
> execute_early_vrp. Simplify access to class data.
>
> diff --git a/gcc/gimple-ssa-evrp.c b/gcc/gimple-ssa-evrp.c
> index 13ba31d..952b31b 100644
> --- a/gcc/gimple-ssa-evrp.c
> +++ b/gcc/gimple-ssa-evrp.c
> @@ -73,6 +73,9 @@ public:
> }
> virtual edge before_dom_children (basic_block);
> virtual void after_dom_children (basic_block);
> + 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);
> @@ -85,6 +88,10 @@ public:
>
> class vr_values vr_values;
>
> + /* We do not allow copying this object or initializing one from another.
> */
> + evrp_dom_walker (const evrp_dom_walker &);
> + evrp_dom_walker& operator= (const evrp_dom_walker &);
> +
> /* Temporary delegators. */
> value_range *get_value_range (const_tree op)
> { return vr_values.get_value_range (op); }
> @@ -509,44 +516,22 @@ evrp_dom_walker::pop_value_range (tree var)
> return vr;
> }
>
> +/* Perform any cleanups after the main phase of EVRP has completed. */
>
> -/* Main entry point for the early vrp pass which is a simplified
> non-iterative
> - version of vrp where basic blocks are visited in dominance order. Value
> - ranges discovered in early vrp will also be used by ipa-vrp. */
> -
> -static unsigned int
> -execute_early_vrp ()
> +void
> +evrp_dom_walker::cleanup (void)
> {
> - edge e;
> - edge_iterator ei;
> - basic_block bb;
> -
> - loop_optimizer_init (LOOPS_NORMAL | LOOPS_HAVE_RECORDED_EXITS);
> - 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;
> - walker.walk (ENTRY_BLOCK_PTR_FOR_FN (cfun));
> -
> if (dump_file)
> {
> fprintf (dump_file, "\nValue ranges after Early VRP:\n\n");
> - walker.vr_values.dump_all_value_ranges (dump_file);
> + vr_values.dump_all_value_ranges (dump_file);
> fprintf (dump_file, "\n");
> }
>
> /* Remove stmts in reverse order to make debug stmt creation possible. */
> - while (! walker.stmts_to_remove.is_empty ())
> + while (! stmts_to_remove.is_empty ())
> {
> - gimple *stmt = walker.stmts_to_remove.pop ();
> + gimple *stmt = stmts_to_remove.pop ();
> if (dump_file && dump_flags & TDF_DETAILS)
> {
> fprintf (dump_file, "Removing dead stmt ");
> @@ -564,18 +549,50 @@ execute_early_vrp ()
> }
> }
>
> - if (!bitmap_empty_p (walker.need_eh_cleanup))
> - gimple_purge_all_dead_eh_edges (walker.need_eh_cleanup);
> + if (!bitmap_empty_p (need_eh_cleanup))
> + gimple_purge_all_dead_eh_edges (need_eh_cleanup);
>
> /* Fixup stmts that became noreturn calls. This may require splitting
> blocks and thus isn't possible during the dominator walk. Do this
> in reverse order so we don't inadvertedly remove a stmt we want to
> fixup by visiting a dominating now noreturn call first. */
> - while (!walker.stmts_to_fixup.is_empty ())
> + while (!stmts_to_fixup.is_empty ())
> {
> - gimple *stmt = walker.stmts_to_fixup.pop ();
> + gimple *stmt = stmts_to_fixup.pop ();
> fixup_noreturn_call (stmt);
> }
> +}
> +
> +/* Main entry point for the early vrp pass which is a simplified
> non-iterative
> + version of vrp where basic blocks are visited in dominance order. Value
> + ranges discovered in early vrp will also be used by ipa-vrp. */
> +
> +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
> + walker. */
> + loop_optimizer_init (LOOPS_NORMAL | LOOPS_HAVE_RECORDED_EXITS);
> + 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;
> + walker.walk (ENTRY_BLOCK_PTR_FOR_FN (cfun));
> + walker.cleanup ();
>
> scev_finalize ();
> loop_optimizer_finalize ();
>
