On Tue, Nov 25, 2014 at 09:59:46PM +0100, Jan Hubicka wrote:
> > From: Trevor Saunders <tsaund...@mozilla.com>
> >
> > Hi,
> >
> > the interesting symbol in the test case for pr61324 is __GLOBAL__sub_I_s.
> > It
> > refers to nothing, and is called by nothing, however it is kept (I believe
> > because of -fkeep-inline-functions). That means ipa_comdats never tries to
> > put
>
> Aha, that explans why it is around.
>
> > it in a comdat, and so it never ends up in the hash table. It seems like
> > the
> > simplest solution is to just check if symbol is not in the map before
> > trying to
> > get the comdat it should go in, but another approach might be to keep
> > separate
> > hash maps for comdat functions and functions that can't be in any comdat,
> > and
> > then iterate over only the functions that belong in a comdat.
>
> Well, -fkeep-inline-functions promise you that you can call any inline
> function from debugger.
> I suppose in this case you also want to be able to call static functions.
> Comdat pass may
> bundle the function into comdat that is later optimized away by linker, so I
> would say we
> just want to disable the whole comdat pass when -fkeep-inline-functions is
> used?
that doesn't seem totally crazy, maybe it makes sense to handle things
like attribute used, and noreorder similarly?
> Patch for that is preapproved.
Ok then, I'll take a look at the other similar looking bugs with
ipa_comdats and see what I find.
Trev
> Honza
> >
> > boottstrapped + regtested x86_64-unknown-linux-gnu, ok?
> >
> > Trev
> >
> > gcc/
> >
> > * ipa-comdats.c (ipa_commdats): check if map contains symbol before
> > trying to put symbol in a comdat.
> >
> > diff --git a/gcc/ipa-comdats.c b/gcc/ipa-comdats.c
> > index af2aef8..8695a7e 100644
> > --- a/gcc/ipa-comdats.c
> > +++ b/gcc/ipa-comdats.c
> > @@ -327,18 +327,18 @@ ipa_comdats (void)
> > && !symbol->alias
> > && symbol->real_symbol_p ())
> > {
> > - tree group = *map.get (symbol);
> > + tree *group = map.get (symbol);
> >
> > - if (group == error_mark_node)
> > + if (!group || *group == error_mark_node)
> > continue;
> > if (dump_file)
> > {
> > fprintf (dump_file, "Localizing symbol\n");
> > symbol->dump (dump_file);
> > - fprintf (dump_file, "To group: %s\n", IDENTIFIER_POINTER (group));
> > + fprintf (dump_file, "To group: %s\n", IDENTIFIER_POINTER
> > (*group));
> > }
> > symbol->call_for_symbol_and_aliases (set_comdat_group,
> > - *comdat_head_map.get (group),
> > + *comdat_head_map.get (*group),
> > true);
> > }
> > }
> > diff --git a/gcc/testsuite/g++.dg/pr61324.C b/gcc/testsuite/g++.dg/pr61324.C
> > new file mode 100644
> > index 0000000..6102574
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/pr61324.C
> > @@ -0,0 +1,13 @@
> > +// { dg-do compile }
> > +// { dg-options "-O -fkeep-inline-functions -fno-use-cxa-atexit" }
> > +void foo ();
> > +
> > +struct S
> > +{
> > + ~S ()
> > + {
> > + foo ();
> > + }
> > +};
> > +
> > +S s;
> > --
> > 2.1.3
