Do not leak memory when streaming in ssa names

[Richard Biener]

On Mon, 23 Nov 2020, Jan Hubicka wrote:

> > On Mon, 23 Nov 2020, Jan Hubicka wrote:
> > 
> > > > On Mon, 23 Nov 2020, Jan Hubicka wrote:
> > > > 
> > > > > Hi,
> > > > > tree-streamer-in currently calls init_tree_ssa that calls init_ssanames
> > > > > that allocates space in ssa_names array for 50 names.  Later it streams
> > > > > in the count and calls init_tree_ssa again, this time with correct
> > > > > count, which is rounded up to 50 and allocated again leaking the first
> > > > > array.
> > > > > 
> > > > > This patch fixes it by adding extra argument to init_tree_ssa specifing
> > > > > whether ssa names should be initialized.  There are few places where we
> > > > > initialize ssa and only lto streaming is special, so i think extra arg
> > > > > works well here.
> > > > > 
> > > > > I am not quite sure about the 50MB default.  I think it was made before
> > > > > ggc_free was implemented (so resizing vectors was expensive) and when we
> > > > > had only one function in SSA form at a time.  Most of functions in C++
> > > > > will probably need fewer than 50 SSA names until inlining.
> > > > > 
> > > > > This saves 21MB of WPA linking libxul.
> > > > 
> > > > Why do we input SSA names at WPA?
> > > 
> > > To ICF compare function bodies. It tends to load tons of small bodies.
> > > We also may load body when merging BB profile etc.
> > > > 
> > > > > Bootstrapping/regtesting x86_64-linux, OK if it suceeds?
> > > > 
> > > > I think calling init_ssanames again is bogus.  Please simply
> > > > do that in input_ssa_names after we know how many we need
> > > > adding a number of SSA names to reserve argument to init_tree_ssa
> > > > and passing that down instead.
> > > 
> > > I tried that originally but it hit ICE in init_ssa_operands.  Seems that
> > > reloacating them is last change I need though.  Does this look better?
> > 
> > Patch below looks completely unrelated but is OK as well if it passes
> > testing ;)
> 
> Glad to hear since it is in mainline for a while :)
> Here is correct pathc. Sorry.

LGTM.

Thanks,
Richard.

> 	* lto-streamer-in.c (input_cfg): Do not init ssa operands.
> 	(input_function): Do not init tree_ssa and set in_ssa_p.
> 	(input_ssa_names): Do it here.
> 	* tree-ssa.c (init_tree_ssa): Add additional SIZE parameter, default
> 	to 0
> 	* tree-ssanames.c (init_ssanames): Do not round size up to 50, allocate
> 	precisely.
> diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c
> index 64037f74ad3..a20d64b0089 100644
> --- a/gcc/lto-streamer-in.c
> +++ b/gcc/lto-streamer-in.c
> @@ -1017,7 +1017,6 @@ input_cfg (class lto_input_block *ib, class data_in *data_in,
>    int index;
>  
>    init_empty_tree_cfg_for_function (fn);
> -  init_ssa_operands (fn);
>  
>    profile_status_for_fn (fn) = streamer_read_enum (ib, profile_status_d,
>  						   PROFILE_LAST);
> @@ -1144,7 +1143,9 @@ input_ssa_names (class lto_input_block *ib, class data_in *data_in,
>    unsigned int i, size;
>  
>    size = streamer_read_uhwi (ib);
> -  init_ssanames (fn, size);
> +  init_tree_ssa (fn, size);
> +  cfun->gimple_df->in_ssa_p = true;
> +  init_ssa_operands (fn);
>  
>    i = streamer_read_uhwi (ib);
>    while (i)
> @@ -1384,9 +1385,6 @@ input_function (tree fn_decl, class data_in *data_in,
>  
>    push_struct_function (fn_decl);
>    fn = DECL_STRUCT_FUNCTION (fn_decl);
> -  init_tree_ssa (fn);
> -  /* We input IL in SSA form.  */
> -  cfun->gimple_df->in_ssa_p = true;
>  
>    gimple_register_cfg_hooks ();
>  
> diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c
> index b44361f8244..24a5154620d 100644
> --- a/gcc/tree-ssa.c
> +++ b/gcc/tree-ssa.c
> @@ -1218,15 +1218,16 @@ err:
>  #  pragma GCC diagnostic pop
>  #endif
>  
> -/* Initialize global DFA and SSA structures.  */
> +/* Initialize global DFA and SSA structures.
> +   If SIZE is non-zero allocated ssa names array of a given size.  */
>  
>  void
> -init_tree_ssa (struct function *fn)
> +init_tree_ssa (struct function *fn, int size)
>  {
>    fn->gimple_df = ggc_cleared_alloc<gimple_df> ();
>    fn->gimple_df->default_defs = hash_table<ssa_name_hasher>::create_ggc (20);
>    pt_solution_reset (&fn->gimple_df->escaped);
> -  init_ssanames (fn, 0);
> +  init_ssanames (fn, size);
>  }
>  
>  /* Deallocate memory associated with SSA data structures for FNDECL.  */
> diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
> index 6ac97fe39c4..ec4681f85cb 100644
> --- a/gcc/tree-ssanames.c
> +++ b/gcc/tree-ssanames.c
> @@ -77,10 +77,10 @@ unsigned int ssa_name_nodes_created;
>  void
>  init_ssanames (struct function *fn, int size)
>  {
> -  if (size < 50)
> -    size = 50;
> -
> -  vec_alloc (SSANAMES (fn), size);
> +  if (!size)
> +    vec_alloc (SSANAMES (fn), 50);
> +  else
> +    vec_safe_reserve (SSANAMES (fn), size, true);
>  
>    /* Version 0 is special, so reserve the first slot in the table.  Though
>       currently unused, we may use version 0 in alias analysis as part of
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend