Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-22 Thread Jan Hubicka
> commit be1b04c77a420288e29c48c07e68c3ec87dd5b24 > Author: Jason Merrill > Date: Thu Jan 12 14:04:42 2012 -0500 > > PR c++/41090 > Add -fdeclone-ctor-dtor. > gcc/cp/ > * optimize.c (can_alias_cdtor, populate_clone_array): Split out > from maybe_clone_body. > (

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-20 Thread Jason Merrill
On 12/13/2013 10:32 AM, Jan Hubicka wrote: On 12/13/2013 05:58 AM, Jan Hubicka wrote: Moreover when we turn comdat_local to false, we need to recompute also function it is inlined into. I don't see why. If function A calls function B, which calls comdat-local function C, A can be inlined, so

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-13 Thread Martin Jambor
Hi, On Wed, Dec 11, 2013 at 02:55:26PM +0100, Jan Hubicka wrote: > > On 12/10/2013 04:48 AM, Jan Hubicka wrote: > > >The case where I played with local comdats was the following. > > >I made cgraph_body_availability to get context argument (i.e. instead of > > >saying > > >if something is availab

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-13 Thread Jan Hubicka
> On 12/13/2013 05:58 AM, Jan Hubicka wrote: > >>+ if (callee->calls_comdat_local) > >>+to->calls_comdat_local = true; > >>+ else if (to->calls_comdat_local && symtab_comdat_local_p (callee)) > >>+{ > >>+ struct cgraph_edge *se = to->callees; > >>+ for (; se; se = se->next_calle

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-13 Thread Jason Merrill
On 12/13/2013 05:58 AM, Jan Hubicka wrote: + if (callee->calls_comdat_local) +to->calls_comdat_local = true; + else if (to->calls_comdat_local && symtab_comdat_local_p (callee)) +{ + struct cgraph_edge *se = to->callees; + for (; se; se = se->next_callee) + if (se->inlin

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-13 Thread Jan Hubicka
> On 12/12/2013 03:08 PM, Jan Hubicka wrote: > >So only reason why this is optimize_size only is the fact that we can't rely > >on inliner > >to fix up the wrappers? > > I was just being conservative. In fact, the inliner seems to handle > small [cd]tors well, inlining them into the wrappers and

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-12 Thread Jason Merrill
On 12/12/2013 03:08 PM, Jan Hubicka wrote: So only reason why this is optimize_size only is the fact that we can't rely on inliner to fix up the wrappers? I was just being conservative. In fact, the inliner seems to handle small [cd]tors well, inlining them into the wrappers and then into c

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-12 Thread Jan Hubicka
Hi, > diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c > index f368cab..3576f7d 100644 > --- a/gcc/c-family/c-opts.c > +++ b/gcc/c-family/c-opts.c > @@ -899,6 +899,10 @@ c_common_post_options (const char **pfilename) >if (warn_implicit_function_declaration == -1) > warn_implicit_

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-11 Thread Jason Merrill
On 12/11/2013 03:53 PM, Jan Hubicka wrote: Lets go with minimal version of patch that makes things working and I will take care of solving the inliner limitations. OK, this patch adjusts calls_comdat_local in compute_inline_parameters, as you suggested. What do you think of the change to inli

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-11 Thread Jan Hubicka
> On 12/11/2013 12:14 PM, Jan Hubicka wrote: > >>Is ipa_passes the right place to initialize calls_comdat_local? > > > >The flag is probably needed in both early inliner and IPA inliner. A > >conservative > >place to initialize it would be in inline_analyze_function. > >(early inliner analyze fun

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-11 Thread Jan Hubicka
> On 12/11/2013 12:45 PM, Jan Hubicka wrote: > >I wonder, if we won't end up with better code going the oposite way. > >We can declare those functions static first and then have post-inliner IPA > >pass that will > >travel the callgraph and mark all static functions/variables that are > >accessed

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-11 Thread Jason Merrill
On 12/11/2013 12:45 PM, Jan Hubicka wrote: I wonder, if we won't end up with better code going the oposite way. We can declare those functions static first and then have post-inliner IPA pass that will travel the callgraph and mark all static functions/variables that are accessed only from one

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-11 Thread Jason Merrill
On 12/11/2013 12:14 PM, Jan Hubicka wrote: Is ipa_passes the right place to initialize calls_comdat_local? The flag is probably needed in both early inliner and IPA inliner. A conservative place to initialize it would be in inline_analyze_function. (early inliner analyze function twice, first

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-11 Thread Jan Hubicka
Hi, while thinking of the details on how to handle comdat locals through ipa-cp/inliner/folding I wonder, if we won't end up with better code going the oposite way. We can declare those functions static first and then have post-inliner IPA pass that will travel the callgraph and mark all static f

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-11 Thread Jan Hubicka
> On 12/11/2013 10:11 AM, Jason Merrill wrote: > >So, it's probably possible to make it work to clone the comdat local > >function into another comdat local function, but it's not useful, and > >it's easier to just prevent it. > > Well, not that much easier actually. I'm attaching both a delta >

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-11 Thread Jan Hubicka
> On 12/11/2013 08:55 AM, Jan Hubicka wrote: > + /* Don't clone decls local to a comdat group. */ > + if (comdat_local_p (node)) > +return false; > >>> > >>>Why? It seems this should work if the clone becomes another comdat local? > >> > >>Right, I think the problem was that the

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-11 Thread Jason Merrill
On 12/11/2013 11:24 AM, Jason Merrill wrote: Well, not that much easier actually. I'm attaching both a delta from my last patch and a complete patch against trunk. Oops, forgot to remove the gimple-fold change, doing that now. Jason

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-11 Thread Jason Merrill
On 12/11/2013 10:11 AM, Jason Merrill wrote: So, it's probably possible to make it work to clone the comdat local function into another comdat local function, but it's not useful, and it's easier to just prevent it. Well, not that much easier actually. I'm attaching both a delta from my last

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-11 Thread Jason Merrill
On 12/11/2013 08:55 AM, Jan Hubicka wrote: + /* Don't clone decls local to a comdat group. */ + if (comdat_local_p (node)) +return false; Why? It seems this should work if the clone becomes another comdat local? Right, I think the problem was that the clone wasn't becoming comdat local

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-11 Thread Jan Hubicka
> On 12/10/2013 04:48 AM, Jan Hubicka wrote: > >The case where I played with local comdats was the following. > >I made cgraph_body_availability to get context argument (i.e. instead of > >saying > >if something is available in current unit, it was saying if it is available > >in current function/

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-10 Thread Jason Merrill
On 12/10/2013 04:48 AM, Jan Hubicka wrote: The case where I played with local comdats was the following. I made cgraph_body_availability to get context argument (i.e. instead of saying if something is available in current unit, it was saying if it is available in current function/variable). If t

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-10 Thread Jan Hubicka
> > diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c > > index fbb63da..aa49bfe 100644 > > --- a/gcc/ipa-inline.c > > +++ b/gcc/ipa-inline.c > > @@ -230,6 +230,25 @@ report_inline_failed_reason (struct cgraph_edge *e) > > } > > } > > > > +/* True iff NODE calls another function which is loc

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-10 Thread Jan Hubicka
> > * gimple-fold.c (can_refer_decl_in_current_unit_p): Check > > references to comdat-local symbols. Also i think this change needs more work. FROM_DECL is not the function you are going to get the refernece, it is variable whose constructor the value was take from. I thi

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-10 Thread Jan Hubicka
> This all started with Stuart Hastings' original decloning patch way > back in 2002: > http://gcc.gnu.org/ml/gcc-patches/2002-08/msg00354.html > Bill Maddox tried to revive it in 2007: > http://gcc.gnu.org/ml/gcc-patches/2007-11/msg01147.html > > I'm embarrassed that it has taken so long to g

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-09 Thread Jason Merrill
On 11/21/2013 12:41 PM, Jason Merrill wrote: I had to change various things in cgraph/ipa in order to support the notion of a comdat-local symbol which can only be referenced from within that comdat, which is what I'm looking for feedback/approval for. The change to can_refer_decl_in_current_uni

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-12-09 Thread Jason Merrill
On 11/21/2013 12:41 PM, Jason Merrill wrote: I had to change various things in cgraph/ipa in order to support the notion of a comdat-local symbol which can only be referenced from within that comdat, which is what I'm looking for feedback/approval for. The change to can_refer_decl_in_current_uni

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-11-21 Thread Jason Merrill
On 11/21/2013 01:24 PM, Joseph S. Myers wrote: The new option should be able to use Var() in the .opt file rather than having a variable defined explicitly or any explicit handling code in c_common_handle_option, and shouldn't need to use UInteger (given the option has no arguments). Good point

Re: RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-11-21 Thread Joseph S. Myers
The new option should be able to use Var() in the .opt file rather than having a variable defined explicitly or any explicit handling code in c_common_handle_option, and shouldn't need to use UInteger (given the option has no arguments). -- Joseph S. Myers jos...@codesourcery.com

RFA (cgraph): C++ 'structor decloning patch, Mark III

2013-11-21 Thread Jason Merrill
This all started with Stuart Hastings' original decloning patch way back in 2002: http://gcc.gnu.org/ml/gcc-patches/2002-08/msg00354.html Bill Maddox tried to revive it in 2007: http://gcc.gnu.org/ml/gcc-patches/2007-11/msg01147.html I'm embarrassed that it has taken so long to get this in.