On 05/28/2013 11:00 AM, David Malcolm wrote:
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?
Probably. However, I would advise against trying to pack too much into
a single patch.
So one possibility to move forward would be a single patch which removes
the n_basic_blocks macro and fixes all the uses along with their
overly-long lines. It's simple, obvious and very self-contained.
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.
I'd do that as a separate follow-up patch.
jeff
