On Mon, Nov 09, 2015 at 01:11:48PM +0100, Richard Biener wrote:
> On Mon, Nov 9, 2015 at 12:22 PM, Martin Liška <mli...@suse.cz> wrote:
> > Hi.
> >
> > This is follow-up of changes that Richi started on Friday.
> >
> > Patch can bootstrap on x86_64-linux-pc and regression tests are running.
> >
> > Ready for trunk?
> 
>         * tree-ssa-dom.c (free_edge_info): Make the function extern.
> ...
>         * tree-ssa.h (free_edge_info): Declare function extern.
> 
> declare this in tree-ssa-threadupdate.h instead and renaming it to
> sth less "public", like free_dom_edge_info.
> 
> diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
> index fff62de..eb6b7df 100644
> --- a/gcc/ifcvt.c
> +++ b/gcc/ifcvt.c
> @@ -3161,6 +3161,8 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
>        set_used_flags (targets[i]);
>      }
> 
> +  temporaries.release ();
> +
>    set_used_flags (cond);
>    set_used_flags (x);
>    set_used_flags (y);
> @@ -3194,6 +3196,7 @@ noce_convert_multiple_sets (struct noce_if_info 
> *if_info)
>      }
> 
>    num_updated_if_blocks++;
> +  targets.release ();
>    return TRUE;
> 
> suspiciously look like candidates for an auto_vec<> (didn't check).

I was about to say the same thing after a little checking (maybe the
region one in tree-ssa-threadupdate.c to, but didn't check that)

> 
> @@ -1240,6 +1240,7 @@ lra_create_live_ranges_1 (bool all_p, bool dead_insn_p)
>    dead_set = sparseset_alloc (max_regno);
>    unused_set = sparseset_alloc (max_regno);
>    curr_point = 0;
> +  point_freq_vec.release ();
>    point_freq_vec.create (get_max_uid () * 2);
> 
> a truncate (0) instead of a release () should be cheaper, avoiding the
> re-allocation.

yeah, or even change it to just grow the array, afaict it doesn't expect
the array to be cleared?

> @@ -674,6 +674,10 @@ sra_deinitialize (void)
>    assign_link_pool.release ();
>    obstack_free (&name_obstack, NULL);
> 
> +  for (hash_map<tree, auto_vec<access_p> >::iterator it =
> +       base_access_vec->begin (); it != base_access_vec->end (); ++it)
> +    (*it).second.release ();
> +
>    delete base_access_vec;
> 
> I wonder if the better fix is to provide a proper free method for the 
> hash_map?
> A hash_map with 'auto_vec' looks suspicous - eventually a proper release
> was intented here via default_hash_map_traits <>?

in fact I would expect that already works, but apparently not, so I'd
say that's the bug.

Trev

> 
> Anyway, most of the things above can be improved as followup of course.
> 
> Thanks,
> Richard.
> 
> > Thanks,
> > Martin

Reply via email to