On Sat, May 25, 2013 at 9:33 PM, Jan Hubicka <hubi...@ucw.cz> wrote:
>> Here's an idea that could make it easier to remove the "cfun" global.
>>
>> "cfun" is a major piece of global state within gcc: it's the 5th most
>> accessed variable in the build (accessed in ~4600 places within one stage
>> of a build, based on [1]). This is an obstacle to making gcc's code be
>> usable as a library.
>
> Yep, note that cfun is not the only beast of this. For 99% of backend it
> needs
> to be kept in sync with current_function_decl pointing to the declaration such
> that DECL_STRUCT_FUNCTION (current_function_decl) = cfun.
> There is also implicit notion of "current cgraph node" that is accessed by
> cgraph_get_node (current_function_decl) many times that can get expensive
> since it leads to hashtable lookup.
> Finally there is "crtl" that is macro hidding a global variable x_rtl
> data for RTL data of the currently compiled function.
>
> In many ways these four are all closely related and data in between them
> are not distributed in very engineered way.
Note that ideally there exists a single global variable per compilation phase.
Frontends would use current_function_decl (as struct function is not available
very early) while the middle-end and target would use cfun.
Through cfun you can reach its decl and you should ideally be able to reach
the cgraph node, too.
>> I can think of three approaches to "cfun":
>> (a) status quo: a global variable, with macros to prevent direct
>> assignment, and an API for changing cfun.
>
> We have push_cfun/pop_cfun for that. If you did not look into what they
> really do, you may be disappointed. They are huge&expensive beasts switching
> a lot of global state in compiler for optimize attribute. So it means that
> on the track removing them we will need to cleanup this area, too.
Yeah ...
>> (b) have a global "context" or "universe" object, and put cfun in
>> there (perhaps with tricks to be able to make this a singleton in a
>> non-library build, optimizing away the context lookups somehow
>> - see [2] for discussion on this)
>> (c) go through all of the places where cfun is used, and somehow ensure
>> that they're passed in the data they need. Often it's not the
>> function that's used, but its cfg.
>>
>> I think (c) is the ideal from a modularity perspective (it unlocks the
>> ability to have optimization passes be working on different functions in
>> different threads), but the most difficult.
>
> (c) is direction I was trying to push IPA code to in longer run. We need
> a single object holding "function" and that should be passed to most
> of the macros.
>
> I am not entirely happy with contents of the "struct function" at all
> (it is kind of kitchen sink for all the data we randomly need to associate
> with function), but it is closest to it. Most of IPA node considers the
> ultimate function pointer to be the cgraph node however.
>
> We do not want to melt those all thogether, since the data in there have
> different
> lifetime. In particular struct_function is not in memory during WPA.
I agree to all of the above.
It would be interesting to have a static analyzer compute what are the most
executed functions that ultimately end up referencing cfun ...
>>
>> One part of the puzzle is that various header files in the build define
>> macros that reference the "cfun" global, e.g.:
>>
>> #define n_basic_blocks (cfun->cfg->x_n_basic_blocks)
>>
>> This one isn't in block caps, which might mislead a new contributor into
>> thinking it's a variable, rather than a macro, so there may be virtue in
>
> Yep yo umany notice there are already _FN variants for that. The macors
> are lower case since historically they were variables. I agree they should
> be replaced.
Yes, explicit "cfun" usage is way better than implicit one - especially because
we still have implicit uses in functions that are already being passed the
function context as parameter ...
>> removing these macros for that reason alone. (I know that these confused
>> me for a while when I first started writing my plugin) [3]
>>
>> So I had a go at removing these macros, to make usage of "cfun" be
>> explicit.
>>
>> I wrote a script to do most of the gruntwork automatically: [4]
>>
>> The following patches introduce accessor methods to control_flow_graph,
>> then remove all of the macros that reference cfun from basic-block.h,
>> replacing all of the places that use them with explicit uses of
>> "cfun->cfg->get_foo ()" as appropriate. There are various other headers
>> that define macros that use cfun, but I thought I'd post this to get a
>> sense of how maintainers feel about this approach.
>>
>> I notice that we're constantly accessing:
>>
>> some loop
>> {
>> ...
>> use cfun->cfg->x_some_field;
>> ...
>> }
>>
>> Would it potentially be faster to replace some of these with:
>>
>> control_flow_graph &cfg = *cfun->cfg;
>> some loop
>> {
>> ...
>> use cfg.get_some_field () // assuming inlining of accessor
>> ...
>> }
>>
>> to avoid constantly derefing cfun->cfg?
I think that was one reason crtl was introduced ... to avoid an indirection
through cfun.
>> (That said, would
>> -fstrict-aliasing
>> be able to note that cfun->cfg doesn't change, or in a non-LTO build I'm
>> guessing it can't make that assumption if the loop calls into functions it
>> can't see inside?).
>
> I like the general direction (i.e. it is what I was hoping for to happen for
> years, but was too lazy to enforce it more curefully).
> We should have single "function" object that is passed around and it should
> have substructures or pointers to all the various stuff we hold about function
> globally (gimple body, cfg, loop tree, profile, IPA data).
>
> Just some random toughts. I epxect there will be more discussion ;)
I indeed welcome the effort to make all 'cfun' uses explicit. Even more welcome
is reducing the number of 'cfun' references ;) Like giving all gimple/rtl pass
execute () / gate () functions a struct function argument (or even
better abstract
that so we can add more context transparently later, like dump_file /
dump_flags).
Richard.
> Honza
