On Tue, 2013-05-28 at 06:39 -0600, Jeff Law wrote:
> On 05/25/2013 07:02 AM, David Malcolm wrote:
> > 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.
> > (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'd think B or C is going to be the way to go here. B may also be an
> intermediate step towards C.
>
> >
> > 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
> > removing these macros for that reason alone. (I know that these confused
> > me for a while when I first started writing my plugin) [3]
> There's a few of these that have crept in over the years.
> n_basic_blocks used to be a global variable. At some point it was
> stuffed into cfun, but it was decided not to go back and fix all the
> references -- possibly due to not wanting to fix the overly long lines
> after the mechanical change.
If a mechanical change could fix the overly-long lines as it went along,
would such a change be acceptable now?
> > 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?).
> Depends on the context. If there's a function call in the loop, then
> we're pretty much screwed. If we're using cfun->cfg regularly in the
> loop, my tendency is to go ahead and manually pull it out of the loop.
I'm thinking about a revised version of the patch that detects when
cfun->cfg is used in the function and adds a:
control_flow_graph &cfg = *cfun->cfg;
local to the top of the function, and replaces the macros with accesses
to that "cfg", rather than "cfun->cfg". This gives us the
pull-it-out-the-loop, and makes the code simpler even where there isn't
a loop, IMHO, though tastes may vary.
Assuming I correctly handle the cases e.g. where cfun is modified, does
that sound a reasonable thing for me to be working on?
Thanks
Dave
