On Sat, 2020-01-18 at 18:42 +0100, Jan Hubicka wrote:
> > > > On Mon, 2020-01-13 at 11:23 +0800, luoxhu wrote:
> > > > On 2020/1/10 19:08, Jan Hubicka wrote:
> > > > > OK. You will need to do the obvious updates for Martin's
> > > > > patch
> > > > > which turned some member functions into static functions.
> > > > >
> > > > > Honza
> > > >
> > > > Thanks a lot! Rebased & updated, will commit below patch
> > > > shortly
> > > > when git push is ready.
> > > >
> > > >
> > > > v8:
> > > > 1. Rebase to master with Martin's static function (r280043)
> > > > comments
> > > > merge.
> > > > Boostrap/testsuite/SPEC2017 tested pass on Power8-LE.
> > > > 2. TODO:
> > > > 2.1. C++ devirt for multiple speculative call targets.
> > > > 2.2. ipa-icf ipa_merge_profiles refine with COMDAT inline
> > > > testcase.
> > >
> > > [...]
> > >
> > > > diff --git a/gcc/ipa-profile.c b/gcc/ipa-profile.c
> > >
> > > [...]
> > >
> > > > +static ipa_profile_call_summaries *call_sums = NULL;
> > >
> > > [...]
> > >
> > > > static void
> > > > ipa_profile_generate_summary (void)
> > > > @@ -169,7 +261,10 @@ ipa_profile_generate_summary (void)
> > > > basic_block bb;
> > > >
> > > > hash_table<histogram_hash> hashtable (10);
> > > > -
> > > > +
> > > > + gcc_checking_assert (!call_sums);
> > > > + call_sums = new ipa_profile_call_summaries (symtab);
> > > > +
> > >
> > > [...]
> > >
> > > Unfortunately, this assertion is failing for most of the
> > > testcases in
> > > jit.dg, reducing the number of PASS results in jit.sum from 10473
> > > down
> > > to 3254 in my builds.
> > >
> > >
> > > Counter-intuitively, "jit" is not in --enable-languages=all (as
> > > it also
> > > needs --enable-host-shared at configure time, which slows down
> > > the
> > > built compiler code).
> > >
> > >
> > > The jit code expects to be able to invoke the compiler code more
> > > than
> > > once within the same process, purging all state.
> > >
> > > It looks like this "call_sums" state needs deleting and resetting
> > > to
> > > NULL after the compiler has run (or else we'll likely get an ICE
> > > due to
> > > using old symtab/call summaries in subsequent in-process runs of
> > > the
> > > compiler).
> > >
> > > Is there a natural place to do that within the IPA hooks?
> > >
> > >
> > > Alternatively the following patch seems to fix things (not yet
> > > fully
> > > tested though); hopefully it seems sane.
> >
> > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
> > fixes
> > the issue with jit.sum.
> >
> > OK for master?
> >
> > gcc/ChangeLog:
> > PR ipa/93315
> > * ipa-profile.c (ipa_profile_c_finalize): New function.
> > * toplev.c (toplev::finalize): Call it.
> > * toplev.h (ipa_profile_c_finalize): New decl.
>
> Other similar summaries are freed at the end of execute method. I
> think
> that we probably want to do the same for consistency as well.
> Advantage is that this releases memory prior late
> compilation/streaming.
Thanks; here's an updated, much simpler patch, which does it at the
end of ipa_profile (which is effectively the execute method).
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
fixes the issue with jit.sum:
Changes to jit.sum
------------------
FAIL: 69->0 (-69)
PASS: 3254->10471 (+7217)
UNRESOLVED: 1->0 (-1)
OK for master?
> I think for all these summaries we have leak for -flto compilation
> where
> we do not call execute methods and thus we do not free the summaries.
> Is this problem for jit?
>
> Honza
If we do, then, if I understand correctly, this would only affect
someone who tried to use libgccjit to generate .o files with -flto,
repeatedly, within a single process. I don't know of anyone doing
that, and if that's broken, that would be a separate, pre-existing,
bug, I think.
Dave
gcc/ChangeLog:
PR ipa/93315
* ipa-profile.c (ipa_profile): Delete call_sums and set it to
NULL on exit.
---
gcc/ipa-profile.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/gcc/ipa-profile.c b/gcc/ipa-profile.c
index 03272f20987..a69ba0c373a 100644
--- a/gcc/ipa-profile.c
+++ b/gcc/ipa-profile.c
@@ -1023,6 +1023,9 @@ ipa_profile (void)
if (dump_file && (dump_flags & TDF_DETAILS))
symtab->dump (dump_file);
+ delete call_sums;
+ call_sums = NULL;
+
return 0;
}
--
2.21.0
