RE: [plugins-ici-cloning-instrumentation] new GCC plugin developements

["Grigori Fursin" <gfursin at gmail dot com>]

Hi Joern,

> > I think we could have the ICI event flexibility/stability with lower
> > overhead if the event sender requests an event identifier number (which
> > can be allocated after the numbers of the gcc 4.5 static event enum values)
> > for an event name at or before the first event with that name, and then
> > sends this identifier number with one or more pointers, which might point
> > to internal gcc data structures, and a pointer to a function to look up
> > the address of a named parameter.  The event sender site source code can
> > then provide information to build the lookup functions at build time,
> > e.g. using gperf.
> 
> I thought a bit more about this, and decided that using gperf-generated hash
> tables is probably overkill.
> 
> It is useful to have provisions for the event generator and the event
> callback being somewhat out of step, but do we really have to cater
> for multiple sources of the same event providing their parameters in
> a different way?
> If there is only one way to find a parameter with a particular name for
> a particular event (for a given combination of compiler binary and plugins),
> that this can be defined with an accessor function, which would
> generally be defined in the same module which raises the event.
> Actually, if we passed around the dso which raises the event, we could
> dynamically look up the accessor function to allow co-existence of different
> accessor functions for the same event::parameter tuple, but I don't think
> there is a need for that.
> 
> Here is an example of how I think we can reduce the overhead while keeping
> a good amount of flexibility; in loop-unroll.c we currently have:
>          /* Code for loop-unrolling ICI decision enabling.  */
>          register_event_parameter ("loop->num", &(loop->num));
>          register_event_parameter ("loop->ninsns", &(loop->ninsns));
>          register_event_parameter ("loop->av_ninsns", &(loop->av_ninsns));
> 
>          register_event_parameter ("loop->lpt_decision.times",
> &(loop->lpt_decision.times));
>          register_event_parameter ("loop->lpt_decision.decision",
> &(loop->lpt_decision.decision));
>          register_event_parameter ("loop->lpt_decision.unroll_runtime",
>                loop->lpt_decision.decision == LPT_UNROLL_RUNTIME ?
> (void *) 1 : (void *) 0);
>          register_event_parameter ("loop->lpt_decision.unroll_constant",
>                loop->lpt_decision.decision == LPT_UNROLL_CONSTANT ?
> (void *) 1 : (void *) 0);
> 
>          call_plugin_event("unroll_feature_change");
> 
>          unregister_event_parameter ("loop->num");
>          unregister_event_parameter ("loop->ninsns");
> 
>          unregister_event_parameter ("loop->av_ninsns");
>          unregister_event_parameter ("loop->lpt_decision.times");
>          unregister_event_parameter ("loop->lpt_decision.decision");
> 
> Instead we could have:
> 
>          invoke_plugin_va_callbacks (PLUGIN_UNROLL_FEATURE_CHANGE, loop);
> and then accessor functions:
> int
> plugin_unroll_feature_change_param_loop_num (va_list va)
> {
>    struct loop *loop = va_arg (va, struct loop *);
>    return loop->num;
> }
> 
> unsigned
> plugin_unroll_feature_change_param_loop_ninsns (va_list va)
> {
>    struct loop *loop = va_arg (va, struct loop *);
>    return loop->ninsns;
> }
> 
> unsigned
> plugin_unroll_feature_change_param_loop_av_ninsns (va_list va)
> {
>    struct loop *loop = va_arg (va, struct loop *);
>    return loop->av_ninsns;
> }
> ...
> bool
> plugin_unroll_feature_change_param_loop_lpt_decision_unroll_runtime
> (va_list va)
> {
>    struct loop *loop = va_arg (va, struct loop *);
>    return loop->lpt_decision.decision == LPT_UNROLL_RUNTIME;
> }
> ...

I am a bit confused about your above example - you suggest to add 
this functionality on top of current ICI or substitute it? 

If you want to substitute it, I personally disagree. We spent a very long time 
with many colleagues and real ICI users discussing how to simplify the usage of 
events for people who are not programming professionals, 
so I really prefer to keep the current structure as it is ... 

However, if it is the way to speedup slow prototype plugins and addition to ICI,
it may be fine but I need to think about it more. 

In both cases, I think it is not critical for now and should be the second step 
after current ICI is synced with the mainline. However, suggestions from others 
are very welcome ;) !..

> There is still another practical issue: as I change the ICI infrastructure
> to fit better with the existing gcc 4.5 plugin infrastructure,
> the ICI plugins must be adapted to keep working.
> As we are trying to have something working in a short time frame, I think
> I should pick one plugin and modify it in lock-step with the infrastructure
> to demonstrate how it is supposed to work.
> 
> Do you think the adapt.c plugin is suitable for that purpose?

Yes, adapt.c is the latest plugin that we use for our experiments ...

Cheers,
Grigori