David Malcolm <dmalc...@redhat.com> wrote:
>On Tue, 2013-11-19 at 09:49 +0100, Richard Biener wrote:
>> On Mon, 18 Nov 2013, David Malcolm wrote:
>>
>> > On Fri, 2013-11-15 at 20:38 -0500, David Malcolm wrote:
>> > > 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.
>> >
>> > Committed to trunk as r204995; I plan to commit followup patches to
>> > remove the other such macros.
>>
>> Thanks!
>
>The following removed the "n_edges" macro. I committed it to trunk as
>r205044 having successfully bootstrapped on x86_64-unknown-linux-gnu.
>
>(should I continue to post these patches as I commit them?)
Yes.
Thanks,
Richard.
