Re: [PATCH 2/2] Get rid of global state accesses in dominance.c

[David Malcolm <dmalcolm@redhat.com>]

On Fri, 2015-08-14 at 12:25 -0600, Jeff Law wrote:
> On 08/14/2015 02:02 AM, Richard Biener wrote:
> > On Fri, Aug 14, 2015 at 3:04 AM, Mikhail Maltsev <maltsevm@gmail.com> wrote:
> >> The second part removes all global state accesses (i.e. accesses to cfun and
> >> it's members) from dominance.c. This requires to change lots of code, but I hope
> >> that this is a step in right direction (if my understanding of ongoing
> >> re-architecture w.r.t. to global state is correct).
> >>
> >> For now this second part lacks a changelog entry, but it's very "mechanical". I
> >> will, of course, write it if the patch gets approved.
> >
> > So the last time I did similar refactoring I wondered if we can somehow avoid
> > the "noise" in non-IPA passes.  Ideas I came up with are
> >
> >   a)  Inherit gimple/rtl pass classes from a class which is initialized with the
> >        function the pass operates on and provides methods like
> >
> >      bool dom_info_available_p (..) { return dom_info_available_p (fn, ...); }
> >
> >       thus wraps APIs working on a specific function.
> >
> >   b)  Do sth similar but make it work with overloads and clever (no idea what!)
> >      C++ that disables them if this_fn cannot be looked up
> >
> >      template <disable-me-if-this_fn-cannot_be_lookedup-at-instantiation-place>
> >      bool dom_info_available_p (..., struct function *fn = this_fn);
> >
> > all of the above would of course require that passes make all their
> > implementation
> > be methods of their pass class.  So even more refactoring.
> >
> > Note that we do not have any IPA pass which accesses dominators, so the
> > implicit 'cfun' use was ok.  The cases I refactored were those where we had
> > to push/pop_cfun () in IPA passes (which can be expensive) because it
> > used APIs with implicit cfun.
> >
> > Overall I'm not sure we want all APIs using 'cfun' to be refactored.
> > It is after
> > all useless noise to callers if all callers are effectively using 'cfun'.
> And since the main driver for eliminating global state is David's work 
> on the JIT, perhaps see if any of this helps David in a noticeable way 
> before giving it a yea/nea.

The JIT guards all access to GCC's state in a big mutex ("jit_mutex", in
gcc/jit/jit-playback.c).

For example, this includes everything to do with GC and GTY, since
there's implicitly a single set of GC roots and the GC code isn't thread
safe.

So in theory this patch might reduce the amount of things that need to
be guarded by the mutex, but sadly the mountain of work there is so
large that I'm not sure how helpful it is.  (sigh)

I've been tackling things on an as-needed basis - for example, the
recent timevar global-state removal was motivated by wanting to expose a
profiling API to jit client code.

So if there are other benefits beyond merely eliminating accesses to
globals, then maybe it's good.  Does the code become simpler, more easy
to understand?  Does it eliminate the need to change "cfun" (IIRC that
can be expensive).

For example, the jit has some rather ugly code to purge global state
after each in-process invocation.  If that can be simplified or made
less ugly, that's good.

Hope this is constructive.
Dave