On Fri, Nov 17, 2017 at 5:49 AM, Jeff Law <l...@redhat.com> 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 (); >