From: Jan Hubicka <hubicka@ucw.cz>
To: Richard Biener <rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: Do not leak memory when streaming in ssa names
Date: Mon, 23 Nov 2020 13:56:47 +0100 [thread overview]
Message-ID: <20201123125647.GC66486@kam.mff.cuni.cz> (raw)
In-Reply-To: <nycvar.YFH.7.77.849.2011231249310.7048@jbgna.fhfr.qr>
> 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.
* 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
next prev parent reply other threads:[~2020-11-23 12:56 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-23 12:18 Jan Hubicka
2020-11-23 12:27 ` Richard Biener
2020-11-23 12:43 ` Jan Hubicka
2020-11-23 12:49 ` Richard Biener
2020-11-23 12:56 ` Jan Hubicka [this message]
2020-11-23 13:18 ` Jan Hubicka
2020-11-23 13:18 ` Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201123125647.GC66486@kam.mff.cuni.cz \
--to=hubicka@ucw.cz \
--cc=gcc-patches@gcc.gnu.org \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).