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

[Joern Rennecke <amylaar@spamcop.net>]

Quoting Joern Rennecke <amylaar@spamcop.net>:
> 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;
}
...


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?