[PATCH] Eliminate n_basic_blocks macro (was Re: [PATCH] Avoid some unnecessary set_cfun calls)

[David Malcolm]

On Wed, 2013-11-13 at 14:44 +0100, Richard Biener wrote:
> On Wed, 13 Nov 2013, David Malcolm wrote:
> 
> > On Wed, 2013-11-13 at 13:53 +0100, Richard Biener wrote:
> > > On Wed, 13 Nov 2013, Martin Jambor wrote:
> > > 
> > > > Hi,
> > > > 
> > > > On Wed, Nov 13, 2013 at 10:49:09AM +0100, Jakub Jelinek wrote:
> > > > > Hi!
> > > > > 
> > > > > void f1 (void) {}
> > > > > __attribute__((target ("avx"))) void f2 (void) {}
> > > > > __attribute__((target ("avx2"))) void f3 (void) {}
> > > > > __attribute__((target ("sse3"))) void f4 (void) {}
> > > > > __attribute__((target ("ssse3"))) void f5 (void) {}
> > > > > __attribute__((target ("sse4"))) void f6 (void) {}
> > > > > takes about 3 seconds to compile at -O2, because set_cfun is terribly
> > > > > expensive and there are hundreds of such calls.
> > > > > The following patch is just a quick change to avoid some of them:
> > > > > execute_function_todo starts with:
> > > > >   unsigned int flags = (size_t)data;
> > > > >   flags &= ~cfun->last_verified;
> > > > >   if (!flags)
> > > > >     return;
> > > > > and if flags is initially zero, it does nothing.
> > > > > Similarly, execute_function_dump has the whole body surrounded by
> > > > >   if (dump_file && current_function_decl)
> > > > > and thus if dump_file is NULL, there is nothing to do.
> > > > > So IMHO in neither case (which happens pretty frequently) we need to
> > > > > set_cfun to every function during IPA.
> > > > > 
> > > > > Also, I wonder if we couldn't defer the expensive ira_init, if the info
> > > > > computed by it is used only during RTL optimization passes (haven't verified
> > > > > it yet), then supposedly we could just remember using some target hook
> > > > > what the last state was when we did ira_init last time, and call ira_init
> > > > > again at the start of expansion or so if it is different from the
> > > > > last time.
> > > > 
> > > > I was wondering whether the expensive parts of set_cfun could only be
> > > > run in pass_all_optimizations (and the -Og equivalent) but not when
> > > > changing functions in early and IPA passes.
> > > 
> > > Sounds like a hack ;)
> > > 
> > > Better get things working without the cfun/current_function_decl globals.
> > > Wasn't there someone replacing all implicit uses with explicit ones
> > > for stuff like n_basic_blocks?
> > 
> > I was working on this:
> > http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00780.html
> > though I switched to other tasks I felt were higher priority; sorry.
> > 
> > Do you still want me to go ahead and commit the series of changes you
> > pre-approved there?
> > 
> > i.e. the "n_basic_blocks" macro goes away in favor of:
> >    n_basic_blocks_for_fn (cfun)
> > as a renaming of the existing n_basic_blocks_for_function macro,
> > followed up by analogous changes to the other macros.
> > 
> > Or should I repost before committing?
> 
> I'd say create the n_basic_blocks patch and post it, that gives
> people a chance to object.  If nobody chimes in I approve it
> and pre-approve the rest ;)
> 
> Using n_basic_blocks_for_fn (cfun) might feel backwards if
> eventually we'd want to C++-ify struct function and make
> n_basic_blocks a member function which would make it
> cfun->n_basic_blocks () instead.  Ok, I think that will get
> us into C++ bikeshedding again ;)

[I can't face another C vs C++ discussion right now :)]

Thanks.  Attached is such a patch, eliminating the:
  n_basic_blocks
macro in favor of
  n_basic_blocks_for_fn (cfun)

Successfully bootstrapped on x86_64-unknown-linux-gnu, and successfully
compiled stage1 on spu-unknown-elf and s390-linux-gnu (given that those
config files are affected).

Given the conditional pre-approval above, I'm posting here to give
people a change to object - otherwise I'll commit, and followup with the
other macros that implicitly use cfun as per the thread linked to above.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: eliminate-n_basic_blocks.patch
Type: text/x-patch
Size: 65887 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20131116/3ca71bb6/attachment.bin>