Re: [PATCH] manage dom-walk_data initialization and finalization with constructors and destructors

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Sep 4, 2013 at 5:16 AM,  <tsaunders@mozilla.com> wrote:
> From: Trevor Saunders <tsaunders@mozilla.com>
>
> bootstrapped on x86_64-unknown-linux-gnu with same test results as unpatched r202185 ok?

That looks like a not too useful part-C++-ification of domwalk.  A proper
C++-ification would include the use of functors and possibly templating it
on the user specific data type.

So, what do you think is the advantage (other than dropping two or three lines
of code in the users)?

Richard.

>
> * compare-elim.c (find_comparisons_in_bb): adjust
> * domwalk.c (init_walk_dominator_tree): Convert to dom_walk_data constructor.
>   (fini_walk_dominator_tree): Convert to dom_walk_data destructor.
> * domwalk.h (dom_walk_data::dom_walk_data): declare
>   (dom_walk_data::~dom_walk_data): declare
>   (init_walk_dominator_tree): remove
>   (fini_walk_dominator_tree): remove
> * fwprop.c (build_single_def_use_links): adjust
> * gimple-ssa-strength-reduction.c (execute_strength_reduction): adjust
> * graphite-sese-to-poly.c (build_sese_conditions_after): adjust
> * tree-into-ssa.c (rewrite_blocks): adjust
>   (mark_def_site_blocks): adjust
> * tree-ssa-dom.c (tree_ssa_dominator_optimize): adjust
> * tree-ssa-dse.c (tree_ssa_dse): adjust
> * tree-ssa-loop-im.c (determine_invariantness): adjust
>   (move_computations): adjust
> * tree-ssa-phiopt.c (get_non_trapping): adjust
> * tree-ssa-pre.c (eliminate): adjust
> * tree-ssa-strlen.c (tree_ssa_strlen): adjust
> * tree-ssa-uncprop.c (tree_ssa_uncprop): adjust
>
> diff --git a/gcc/compare-elim.c b/gcc/compare-elim.c
> index e907376..8a1e68f 100644
> --- a/gcc/compare-elim.c
> +++ b/gcc/compare-elim.c
> @@ -403,17 +403,13 @@ find_comparisons_in_bb (struct dom_walk_data *data ATTRIBUTE_UNUSED,
>  static void
>  find_comparisons (void)
>  {
> -  struct dom_walk_data data;
> +  struct dom_walk_data data (CDI_DOMINATORS);
>
> -  memset (&data, 0, sizeof(data));
> -  data.dom_direction = CDI_DOMINATORS;
>    data.before_dom_children = find_comparisons_in_bb;
>
>    calculate_dominance_info (CDI_DOMINATORS);
>
> -  init_walk_dominator_tree (&data);
>    walk_dominator_tree (&data, ENTRY_BLOCK_PTR);
> -  fini_walk_dominator_tree (&data);
>
>    clear_aux_for_blocks ();
>    free_dominance_info (CDI_DOMINATORS);
> diff --git a/gcc/domwalk.c b/gcc/domwalk.c
> index 8c1ddc6..bc7a93f 100644
> --- a/gcc/domwalk.c
> +++ b/gcc/domwalk.c
> @@ -261,22 +261,26 @@ walk_dominator_tree (struct dom_walk_data *walk_data, basic_block bb)
>    free (worklist);
>  }
>
> -void
> -init_walk_dominator_tree (struct dom_walk_data *walk_data)
> +dom_walk_data::dom_walk_data (cdi_direction direction)
> +  : dom_direction(direction),
> +  initialize_block_local_data (NULL),
> +    before_dom_children (NULL),
> +  after_dom_children (NULL),
> +  global_data (NULL),
> +    block_local_data_size (0)
>  {
> -  walk_data->free_block_data.create (0);
> -  walk_data->block_data_stack.create (0);
> +  free_block_data.create (0);
> +  block_data_stack.create (0);
>  }
>
> -void
> -fini_walk_dominator_tree (struct dom_walk_data *walk_data)
> +dom_walk_data::~dom_walk_data ()
>  {
> -  if (walk_data->initialize_block_local_data)
> +  if (initialize_block_local_data)
>      {
> -      while (walk_data->free_block_data.length () > 0)
> -       free (walk_data->free_block_data.pop ());
> +      while (free_block_data.length () > 0)
> +       free (free_block_data.pop ());
>      }
>
> -  walk_data->free_block_data.release ();
> -  walk_data->block_data_stack.release ();
> +  free_block_data.release ();
> +  block_data_stack.release ();
>  }
> diff --git a/gcc/domwalk.h b/gcc/domwalk.h
> index 54b7f3c..94986c7 100644
> --- a/gcc/domwalk.h
> +++ b/gcc/domwalk.h
> @@ -26,6 +26,10 @@ typedef void *void_p;
>
>  struct dom_walk_data
>  {
> +public:
> +  dom_walk_data (cdi_direction);
> +  ~dom_walk_data ();
> +
>    /* This is the direction of the dominator tree we want to walk.  i.e.,
>       if it is set to CDI_DOMINATORS, then we walk the dominator tree,
>       if it is set to CDI_POST_DOMINATORS, then we walk the post
> @@ -70,5 +74,3 @@ struct dom_walk_data
>  };
>
>  void walk_dominator_tree (struct dom_walk_data *, basic_block);
> -void init_walk_dominator_tree (struct dom_walk_data *);
> -void fini_walk_dominator_tree (struct dom_walk_data *);
> diff --git a/gcc/fwprop.c b/gcc/fwprop.c
> index 8fe02ac..e10754b 100644
> --- a/gcc/fwprop.c
> +++ b/gcc/fwprop.c
> @@ -269,8 +269,6 @@ single_def_use_leave_block (struct dom_walk_data *walk_data ATTRIBUTE_UNUSED,
>  static void
>  build_single_def_use_links (void)
>  {
> -  struct dom_walk_data walk_data;
> -
>    /* We use the multiple definitions problem to compute our restricted
>       use-def chains.  */
>    df_set_flags (DF_EQ_NOTES);
> @@ -291,14 +289,12 @@ build_single_def_use_links (void)
>
>    /* Walk the dominator tree looking for single reaching definitions
>       dominating the uses.  This is similar to how SSA form is built.  */
> -  walk_data.dom_direction = CDI_DOMINATORS;
> +  dom_walk_data walk_data (CDI_DOMINATORS);
>    walk_data.initialize_block_local_data = NULL;
>    walk_data.before_dom_children = single_def_use_enter_block;
>    walk_data.after_dom_children = single_def_use_leave_block;
>
> -  init_walk_dominator_tree (&walk_data);
>    walk_dominator_tree (&walk_data, ENTRY_BLOCK_PTR);
> -  fini_walk_dominator_tree (&walk_data);
>
>    BITMAP_FREE (local_lr);
>    BITMAP_FREE (local_md);
> diff --git a/gcc/gimple-ssa-strength-reduction.c b/gcc/gimple-ssa-strength-reduction.c
> index e85e629..240b190 100644
> --- a/gcc/gimple-ssa-strength-reduction.c
> +++ b/gcc/gimple-ssa-strength-reduction.c
> @@ -3429,8 +3429,6 @@ analyze_candidates_and_replace (void)
>  static unsigned
>  execute_strength_reduction (void)
>  {
> -  struct dom_walk_data walk_data;
> -
>    /* Create the obstack where candidates will reside.  */
>    gcc_obstack_init (&cand_obstack);
>
> @@ -3451,13 +3449,12 @@ execute_strength_reduction (void)
>    loop_optimizer_init (AVOID_CFG_MODIFICATIONS);
>
>    /* Set up callbacks for the generic dominator tree walker.  */
> -  walk_data.dom_direction = CDI_DOMINATORS;
> +  dom_walk_data walk_data (CDI_DOMINATORS);
>    walk_data.initialize_block_local_data = NULL;
>    walk_data.before_dom_children = find_candidates_in_block;
>    walk_data.after_dom_children = NULL;
>    walk_data.global_data = NULL;
>    walk_data.block_local_data_size = 0;
> -  init_walk_dominator_tree (&walk_data);
>
>    /* Walk the CFG in predominator order looking for strength reduction
>       candidates.  */
> @@ -3472,8 +3469,6 @@ execute_strength_reduction (void)
>    /* Analyze costs and make appropriate replacements.  */
>    analyze_candidates_and_replace ();
>
> -  /* Free resources.  */
> -  fini_walk_dominator_tree (&walk_data);
>    loop_optimizer_finalize ();
>    base_cand_map.dispose ();
>    obstack_free (&chain_obstack, NULL);
> diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
> index c4c3eb4..431431c 100644
> --- a/gcc/graphite-sese-to-poly.c
> +++ b/gcc/graphite-sese-to-poly.c
> @@ -1289,7 +1289,6 @@ build_sese_conditions_after (struct dom_walk_data *dw_data,
>  static void
>  build_sese_conditions (sese region)
>  {
> -  struct dom_walk_data walk_data;
>    vec<gimple> conditions;
>    conditions.create (3);
>    vec<gimple> cases;
> @@ -1300,16 +1299,14 @@ build_sese_conditions (sese region)
>    data.cases = &cases;
>    data.region = region;
>
> -  walk_data.dom_direction = CDI_DOMINATORS;
> +  dom_walk_data walk_data (CDI_DOMINATORS);
>    walk_data.initialize_block_local_data = NULL;
>    walk_data.before_dom_children = build_sese_conditions_before;
>    walk_data.after_dom_children = build_sese_conditions_after;
>    walk_data.global_data = &data;
>    walk_data.block_local_data_size = 0;
>
> -  init_walk_dominator_tree (&walk_data);
>    walk_dominator_tree (&walk_data, SESE_ENTRY_BB (region));
> -  fini_walk_dominator_tree (&walk_data);
>
>    conditions.release ();
>    cases.release ();
> diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
> index 33d4ba8..30015e5 100644
> --- a/gcc/tree-into-ssa.c
> +++ b/gcc/tree-into-ssa.c
> @@ -2183,15 +2183,11 @@ rewrite_update_leave_block (struct dom_walk_data *walk_data ATTRIBUTE_UNUSED,
>  static void
>  rewrite_blocks (basic_block entry, enum rewrite_mode what)
>  {
> -  struct dom_walk_data walk_data;
> -
>    /* Rewrite all the basic blocks in the program.  */
>    timevar_push (TV_TREE_SSA_REWRITE_BLOCKS);
>
>    /* Setup callbacks for the generic dominator tree walker.  */
> -  memset (&walk_data, 0, sizeof (walk_data));
> -
> -  walk_data.dom_direction = CDI_DOMINATORS;
> +  dom_walk_data walk_data (CDI_DOMINATORS);
>
>    if (what == REWRITE_ALL)
>      {
> @@ -2208,16 +2204,10 @@ rewrite_blocks (basic_block entry, enum rewrite_mode what)
>
>    block_defs_stack.create (10);
>
> -  /* Initialize the dominator walker.  */
> -  init_walk_dominator_tree (&walk_data);
> -
>    /* Recursively walk the dominator tree rewriting each statement in
>       each basic block.  */
>    walk_dominator_tree (&walk_data, entry);
>
> -  /* Finalize the dominator walker.  */
> -  fini_walk_dominator_tree (&walk_data);
> -
>    /* Debugging dumps.  */
>    if (dump_file && (dump_flags & TDF_STATS))
>      {
> @@ -2261,12 +2251,11 @@ mark_def_sites_block (struct dom_walk_data *walk_data, basic_block bb)
>  static void
>  mark_def_site_blocks (void)
>  {
> -  struct dom_walk_data walk_data;
>    struct mark_def_sites_global_data mark_def_sites_global_data;
>
>    /* Setup callbacks for the generic dominator tree walker to find and
>       mark definition sites.  */
> -  walk_data.dom_direction = CDI_DOMINATORS;
> +  dom_walk_data walk_data (CDI_DOMINATORS);
>    walk_data.initialize_block_local_data = NULL;
>    walk_data.before_dom_children = mark_def_sites_block;
>    walk_data.after_dom_children = NULL;
> @@ -2280,15 +2269,9 @@ mark_def_site_blocks (void)
>    /* We do not have any local data.  */
>    walk_data.block_local_data_size = 0;
>
> -  /* Initialize the dominator walker.  */
> -  init_walk_dominator_tree (&walk_data);
> -
>    /* Recursively walk the dominator tree.  */
>    walk_dominator_tree (&walk_data, ENTRY_BLOCK_PTR);
>
> -  /* Finalize the dominator walker.  */
> -  fini_walk_dominator_tree (&walk_data);
> -
>    /* We no longer need this bitmap, clear and free it.  */
>    BITMAP_FREE (mark_def_sites_global_data.kills);
>  }
> diff --git a/gcc/tree-ssa-dom.c b/gcc/tree-ssa-dom.c
> index 691e6f9..7800aae 100644
> --- a/gcc/tree-ssa-dom.c
> +++ b/gcc/tree-ssa-dom.c
> @@ -782,10 +782,6 @@ free_all_edge_infos (void)
>  static unsigned int
>  tree_ssa_dominator_optimize (void)
>  {
> -  struct dom_walk_data walk_data;
> -
> -  memset (&opt_stats, 0, sizeof (opt_stats));
> -
>    /* Create our hash tables.  */
>    avail_exprs.create (1024);
>    avail_exprs_stack.create (20);
> @@ -793,7 +789,7 @@ tree_ssa_dominator_optimize (void)
>    need_eh_cleanup = BITMAP_ALLOC (NULL);
>
>    /* Setup callbacks for the generic dominator tree walker.  */
> -  walk_data.dom_direction = CDI_DOMINATORS;
> +  dom_walk_data walk_data (CDI_DOMINATORS);
>    walk_data.initialize_block_local_data = NULL;
>    walk_data.before_dom_children = dom_opt_enter_block;
>    walk_data.after_dom_children = dom_opt_leave_block;
> @@ -803,9 +799,6 @@ tree_ssa_dominator_optimize (void)
>    walk_data.global_data = NULL;
>    walk_data.block_local_data_size = 0;
>
> -  /* Now initialize the dominator walker.  */
> -  init_walk_dominator_tree (&walk_data);
> -
>    calculate_dominance_info (CDI_DOMINATORS);
>    cfg_altered = false;
>
> @@ -897,9 +890,6 @@ tree_ssa_dominator_optimize (void)
>    /* Delete our main hashtable.  */
>    avail_exprs.dispose ();
>
> -  /* And finalize the dominator walker.  */
> -  fini_walk_dominator_tree (&walk_data);
> -
>    /* Free asserted bitmaps and stacks.  */
>    BITMAP_FREE (need_eh_cleanup);
>
> diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
> index 6578758..819e26f 100644
> --- a/gcc/tree-ssa-dse.c
> +++ b/gcc/tree-ssa-dse.c
> @@ -313,8 +313,6 @@ dse_enter_block (struct dom_walk_data *walk_data ATTRIBUTE_UNUSED,
>  static unsigned int
>  tree_ssa_dse (void)
>  {
> -  struct dom_walk_data walk_data;
> -
>    need_eh_cleanup = BITMAP_ALLOC (NULL);
>
>    renumber_gimple_stmt_uids ();
> @@ -328,7 +326,7 @@ tree_ssa_dse (void)
>
>    /* Dead store elimination is fundamentally a walk of the post-dominator
>       tree and a backwards walk of statements within each block.  */
> -  walk_data.dom_direction = CDI_POST_DOMINATORS;
> +  dom_walk_data walk_data (CDI_POST_DOMINATORS);
>    walk_data.initialize_block_local_data = NULL;
>    walk_data.before_dom_children = dse_enter_block;
>    walk_data.after_dom_children = NULL;
> @@ -336,15 +334,9 @@ tree_ssa_dse (void)
>    walk_data.block_local_data_size = 0;
>    walk_data.global_data = NULL;
>
> -  /* Initialize the dominator walker.  */
> -  init_walk_dominator_tree (&walk_data);
> -
>    /* Recursively walk the dominator tree.  */
>    walk_dominator_tree (&walk_data, EXIT_BLOCK_PTR);
>
> -  /* Finalize the dominator walker.  */
> -  fini_walk_dominator_tree (&walk_data);
> -
>    /* Removal of stores may make some EH edges dead.  Purge such edges from
>       the CFG as needed.  */
>    if (!bitmap_empty_p (need_eh_cleanup))
> diff --git a/gcc/tree-ssa-loop-im.c b/gcc/tree-ssa-loop-im.c
> index e5e502b..e5aba3b 100644
> --- a/gcc/tree-ssa-loop-im.c
> +++ b/gcc/tree-ssa-loop-im.c
> @@ -1185,15 +1185,10 @@ determine_invariantness_stmt (struct dom_walk_data *dw_data ATTRIBUTE_UNUSED,
>  static void
>  determine_invariantness (void)
>  {
> -  struct dom_walk_data walk_data;
> -
> -  memset (&walk_data, 0, sizeof (struct dom_walk_data));
> -  walk_data.dom_direction = CDI_DOMINATORS;
> +  dom_walk_data walk_data (CDI_DOMINATORS);
>    walk_data.before_dom_children = determine_invariantness_stmt;
>
> -  init_walk_dominator_tree (&walk_data);
>    walk_dominator_tree (&walk_data, ENTRY_BLOCK_PTR);
> -  fini_walk_dominator_tree (&walk_data);
>  }
>
>  /* Hoist the statements in basic block BB out of the loops prescribed by
> @@ -1337,17 +1332,13 @@ move_computations_stmt (struct dom_walk_data *dw_data,
>  static unsigned int
>  move_computations (void)
>  {
> -  struct dom_walk_data walk_data;
> +  dom_walk_data walk_data (CDI_DOMINATORS);
>    unsigned int todo = 0;
>
> -  memset (&walk_data, 0, sizeof (struct dom_walk_data));
>    walk_data.global_data = &todo;
> -  walk_data.dom_direction = CDI_DOMINATORS;
>    walk_data.before_dom_children = move_computations_stmt;
>
> -  init_walk_dominator_tree (&walk_data);
>    walk_dominator_tree (&walk_data, ENTRY_BLOCK_PTR);
> -  fini_walk_dominator_tree (&walk_data);
>
>    gsi_commit_edge_inserts ();
>    if (need_ssa_update_p (cfun))
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index ddcd040..83102ec 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -1428,7 +1428,6 @@ static struct pointer_set_t *
>  get_non_trapping (void)
>  {
>    struct pointer_set_t *nontrap;
> -  struct dom_walk_data walk_data;
>
>    nt_call_phase = 0;
>    nontrap = pointer_set_create ();
> @@ -1439,6 +1438,7 @@ get_non_trapping (void)
>
>    /* Setup callbacks for the generic dominator tree walker.  */
>    nontrap_set = nontrap;
> +  dom_walk_data walk_data (CDI_DOMINATORS);
>    walk_data.dom_direction = CDI_DOMINATORS;
>    walk_data.initialize_block_local_data = NULL;
>    walk_data.before_dom_children = nt_init_block;
> @@ -1446,9 +1446,7 @@ get_non_trapping (void)
>    walk_data.global_data = NULL;
>    walk_data.block_local_data_size = 0;
>
> -  init_walk_dominator_tree (&walk_data);
>    walk_dominator_tree (&walk_data, ENTRY_BLOCK_PTR);
> -  fini_walk_dominator_tree (&walk_data);
>    seen_ssa_names.dispose ();
>
>    clear_aux_for_blocks ();
> diff --git a/gcc/tree-ssa-pre.c b/gcc/tree-ssa-pre.c
> index 56b0573..04768d3 100644
> --- a/gcc/tree-ssa-pre.c
> +++ b/gcc/tree-ssa-pre.c
> @@ -4416,7 +4416,6 @@ eliminate_leave_block (dom_walk_data *, basic_block)
>  static unsigned int
>  eliminate (void)
>  {
> -  struct dom_walk_data walk_data;
>    gimple_stmt_iterator gsi;
>    gimple stmt;
>    unsigned i;
> @@ -4430,15 +4429,13 @@ eliminate (void)
>    el_avail.create (0);
>    el_avail_stack.create (0);
>
> -  walk_data.dom_direction = CDI_DOMINATORS;
> +  dom_walk_data walk_data (CDI_DOMINATORS);
>    walk_data.initialize_block_local_data = NULL;
>    walk_data.before_dom_children = eliminate_bb;
>    walk_data.after_dom_children = eliminate_leave_block;
>    walk_data.global_data = NULL;
>    walk_data.block_local_data_size = 0;
> -  init_walk_dominator_tree (&walk_data);
>    walk_dominator_tree (&walk_data, ENTRY_BLOCK_PTR);
> -  fini_walk_dominator_tree (&walk_data);
>
>    el_avail.release ();
>    el_avail_stack.release ();
> diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
> index 5c21b92..6d97467 100644
> --- a/gcc/tree-ssa-strlen.c
> +++ b/gcc/tree-ssa-strlen.c
> @@ -2040,8 +2040,6 @@ strlen_leave_block (struct dom_walk_data *walk_data ATTRIBUTE_UNUSED,
>  static unsigned int
>  tree_ssa_strlen (void)
>  {
> -  struct dom_walk_data walk_data;
> -
>    ssa_ver_to_stridx.safe_grow_cleared (num_ssa_names);
>    max_stridx = 1;
>    strinfo_pool = create_alloc_pool ("strinfo_struct pool",
> @@ -2051,22 +2049,16 @@ tree_ssa_strlen (void)
>
>    /* String length optimization is implemented as a walk of the dominator
>       tree and a forward walk of statements within each block.  */
> -  walk_data.dom_direction = CDI_DOMINATORS;
> +  dom_walk_data walk_data (CDI_DOMINATORS);
>    walk_data.initialize_block_local_data = NULL;
>    walk_data.before_dom_children = strlen_enter_block;
>    walk_data.after_dom_children = strlen_leave_block;
>    walk_data.block_local_data_size = 0;
>    walk_data.global_data = NULL;
>
> -  /* Initialize the dominator walker.  */
> -  init_walk_dominator_tree (&walk_data);
> -
>    /* Recursively walk the dominator tree.  */
>    walk_dominator_tree (&walk_data, ENTRY_BLOCK_PTR);
>
> -  /* Finalize the dominator walker.  */
> -  fini_walk_dominator_tree (&walk_data);
> -
>    ssa_ver_to_stridx.release ();
>    free_alloc_pool (strinfo_pool);
>    if (decl_to_stridxlist_htab.is_created ())
> diff --git a/gcc/tree-ssa-uncprop.c b/gcc/tree-ssa-uncprop.c
> index 837c4ea..780a27f 100644
> --- a/gcc/tree-ssa-uncprop.c
> +++ b/gcc/tree-ssa-uncprop.c
> @@ -366,7 +366,6 @@ record_equiv (tree value, tree equivalence)
>  static unsigned int
>  tree_ssa_uncprop (void)
>  {
> -  struct dom_walk_data walk_data;
>    basic_block bb;
>
>    associate_equivalences_with_edges ();
> @@ -379,24 +378,18 @@ tree_ssa_uncprop (void)
>       dominance information.  */
>    calculate_dominance_info (CDI_DOMINATORS);
>
> +  dom_walk_data walk_data (CDI_DOMINATORS);
>    /* Setup callbacks for the generic dominator tree walker.  */
> -  walk_data.dom_direction = CDI_DOMINATORS;
>    walk_data.initialize_block_local_data = NULL;
>    walk_data.before_dom_children = uncprop_enter_block;
>    walk_data.after_dom_children = uncprop_leave_block;
>    walk_data.global_data = NULL;
>    walk_data.block_local_data_size = 0;
>
> -  /* Now initialize the dominator walker.  */
> -  init_walk_dominator_tree (&walk_data);
> -
>    /* Recursively walk the dominator tree undoing unprofitable
>       constant/copy propagations.  */
>    walk_dominator_tree (&walk_data, ENTRY_BLOCK_PTR);
>
> -  /* Finalize and clean up.  */
> -  fini_walk_dominator_tree (&walk_data);
> -
>    /* EQUIV_STACK should already be empty at this point, so we just
>       need to empty elements out of the hash table, free EQUIV_STACK,
>       and cleanup the AUX field on the edges.  */
> --
> 1.8.4.rc3
>