On Sat, Aug 15, 2015 at 8:05 AM, Mikhail Maltsev <malts...@gmail.com> wrote:
> On 08/14/2015 11:02 AM, Richard Biener wrote:
>> 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);
>>
> I attached some sketch of what this "clever C++" could look like. It still
> requires some changes in classes which would use this new base class, but they
> are not very significant compared to converting pass functions into members of
> these classes.
>
>> all of the above would of course require that passes make all their
>> implementation
>> be methods of their pass class. So even more refactoring.
> The good thing is that this can be done incrementally, i.e. changing one pass
> at
> a time.
True. I think we want a cfun.h header so that if cfun.h is not
included the overloads
with the default parameter don't work (uh, hopefully they'll still parse...).
Note that one of the unfortunate thing is this default arg stuff only
works on trailing
arguments and thus is inconsistend with other API having a function * argument.
It also makes it hard to apply to APIs that use default args themselves.
Not using default args but forwarder overloads like
dominated_by_p (cdi_direction, basic_block, basic_block)
{
return dominated_by_p (cfun, dir, bb1, bb2);
}
would avoid this. Of course then choosing one or the other becomes a pure
name-lookup issue, but I supose that's ok.
And of course with those available there isn't a good reason (apart from JIT
avoiding globals) to switch a pass over to the new scheme.
>> Refactoring APIs that are used from IPA and make push/pop_cfun necessary
>> for them are another thing. (there are still plenty left I think)
> What I could find is:
> 1. ipa_analyze_node
> 2. some transactional memory-related stuff: ipa_tm_scan_irr_function,
> ipa_tm_transform_transaction, ipa_tm_transform_clone, ipa_tm_execute.
> 3. tree_profiling
>
> Presumably there are no more calculate_dominance_info/free_dominance_info call
> sites with push_cfun/pop_cfun nearby (except for passes.c, but they are not
> related to IPA). So now I need to figure out, what other global state (which
> is
> set up by push_cfun) can these functions use? Could you give some advice about
> that, in sense, what should I look for (e.g., push_cfun calls some target
> hooks,
> but I did not try to look in detail, what kind of actions are performed by
> it)?
Actions can include computing register preferences and other expensive stuff
if a function has a target or optimize attribute different from the previously
active one. It's the push/pop_cfun calls from ipa-* that are expensive, but
more so that from tree-sra.c (that's in theory quadratic). Basically whenever
only very little is done inside a push/pop pair the it's best to avoid it.
Calling push/pop from non-IPA context is to be avoided even more so.
> As for the API. I propose to, at least, remove uses of cfun from dominance.c
> itself, but provide helper functions which will allow to leave the callers
> unchanged, but make it possible to use all dominance-related API on functions
> other than cfun.
> Does such change sound reasonable to you?
Like providing the overload I suggested above? Yes, that makes sense to me.
Thanks,
Richard.
>
> On 08/14/2015 11:11 PM, David Malcolm wrote:
>> The JIT guards all access to GCC's state in a big mutex ("jit_mutex", in
>> gcc/jit/jit-playback.c).
> Yeah, I know, I looked through JIT documentation briefly.
>>
>> 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.
> And GGC is a "stop-the-world" collector, which is just not designed for
> multithreaded environment. (sigh)
>
> As for the patch, I think reviewing this variant does not make much sense,
> because of Richard suggestion on how to avoid changing the callers. But still
> thanks for looking.
>
>>
>> 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.
>>
> By the way, speaking of timevars. I wrote about my concerns w.r.t. them, but
> did
> not get any feedback:
> https://gcc.gnu.org/ml/gcc/2015-07/msg00165.html
> Briefly speaking, I noticed that our timers introduce rather significant
> measurement errors and proposed some ideas about reducing them. What do you
> think?
>
> --
> Regards,
> Mikhail Maltsev
