Comments

I'm attempting to eliminate global state from the insides of gcc.
gcc/tracer.c has various global variables, which are only used during
the lifetime of the execute callback of that pass, and cleaned up at the
end of each invocation of the pass.
The attached patch introduces a class to hold the state of the pass
("tracer_state"), eliminating these globals. An instance of the state
is created on the stack, and all of the various "static" functions in
tracer.c that used the globals become member functions of the state.
Hence the state is passed around by the implicit "this" of the
tracer_state, avoiding the need to patch each individual use of a field
within the state, minimizing the diff.
Bootstrapped and tested on x86_64-unknown-linux-gnu against r199189, and
has the same test results as an unpatched bootstrap of that revision.
OK to commit to trunk?
2013-05-21 David Malcolm <dmalcolm@redhat.com>
* tracer.c (tracer_state): New class
(probability_cutoff): Convert global variable to be member data
within tracer_state
(branch_ratio_cutoff): ditto
(bb_seen): ditto
(mark_bb_seen): convert to...
(tracer_state::mark_bb_seen): ...member function of new
tracer_state class
(find_best_predecessor): likewise, convert to...
(tracer_state::find_best_predecessor): ...this
(find_trace): likewise, convert to...
(tracer_state::find_trace): ...this
(tail_duplicate): likewise, convert to...
(tracer_state::tail_duplicate): ...this
(tracer): introduce tracer_state instance to move state within
the pass from being global to being on the stack

On Wed, May 22, 2013 at 08:45:45PM -0400, David Malcolm wrote:
> I'm attempting to eliminate global state from the insides of gcc.> > gcc/tracer.c has various global variables, which are only used during> the lifetime of the execute callback of that pass, and cleaned up at the> end of each invocation of the pass.> > The attached patch introduces a class to hold the state of the pass> ("tracer_state"), eliminating these globals. An instance of the state> is created on the stack, and all of the various "static" functions in> tracer.c that used the globals become member functions of the state.> Hence the state is passed around by the implicit "this" of the> tracer_state, avoiding the need to patch each individual use of a field> within the state, minimizing the diff.
But do we want to handle the global state this way? This adds overhead
to (almost?) every single function (now method) in the file (because it gets
an extra argument). While that might be fine for rarely executed functions,
if it involves also hot functions called many times, where especially on
register starved hosts it could increase register pressure, plus the
overhead of passing the this argument everywhere, this could start to be
noticeable. Sure, if you plan to do that just in one pass (but, why then?),
it might be tiny slowdown, but after you convert the hundreds of passes in
gcc that contain global state it might become significant.
There are alternative approaches that should be considered.
E.g. global state of a pass can be moved into a per-pass structure,
and have some way how to aggregate those per pass structures together from
all the passes in the whole compiler (that can be either manual process,
say each pass providing its own *-passstate.h and one big header including
all that together), or automatic ones (say gengstate or a new tool could
create those for us from special markings in the source, say new option on
GTY or something) and have some magic macro how to access the global state
within the pass (thispass->fieldname ?). Then e.g. depending on how the
compiler would be configured and built, thispass could be just address of a
pass struct var (i.e. essentially keep the global state as is, for
performance reasons), or when trying to build compiler as a library (with
-fpic overhead we probably don't want for cc1/cc1plus - we can build all the
*.o files twice, like libtool does) thispass could expand to __thread
pointer var dereference plus a field inside of the global compiler state
structure it points to for the current pass. Thus, the library version
of the compiler would be somewhat slower (both -fpic overhead and TLS
overhead), and would need either a few of the entrypoints tweaked to adjust
the TLS pointer to the global state, or we could require users to just call
a special function to make the global state current in the current thread
before calling compiler internals.
Jakub