> 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.
> 
> 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.

> (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.
> 
> 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.

> 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?  (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 ;)
Honza

Reply via email to