Quoting Grigori Fursin <gfur...@gmail.com>:
 > I am a bit confused about your above example - you suggest to add
this functionality on top of current ICI or substitute it?

The idea was to replace it.  The current event implementation has two
issues:
- It is very different from the existing GCC 4.5 events which makes it
  confusing to have it side-by-side.  I think if we can make the ICI code
  more of a natural extension of the current event code, it would be more
  likely to be acceptable.
- The parameter registering / unregistering and the event call itself
  introduce a number of hashing operations which are performed irrespective
  of the presence of a plugin.  This makes these events intrusive if placed
  in places with a high execution frequency.

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 ...

Were these discussions done face-to-face or on a news group/mailing list?
If the latter, could you point me where I can read up on the discussion
so that I better understand the issues involved.
Would these people who are not programming professionals both insert
event raising points as well as write event handling code?

If we would use C++, some things would get easier, i.e. we could have an
event class with subclasses for the separate event types, and then have
the parameter accessors be member functions.  This would remove the need
to repeatedly type the event name when accessing the parameters.
However, it would require to build GCC with C++, so I'd say this
significantly reduces the chances of having ICI integrated into the
gcc mainline and having release GCC binaries shipped with the
functionality enabled.

If plugin-side run time is not important, we can have register_plugin_event
as a wrapper for register_callback and use user_data to squirrel away the
event name and the actual callback function; then we can have a wrapper
callback function which sets a thread-local variable (can be non-exported
global inside the plugin as long as no threads are used) to the plugin name,
and make get_event_parameter use a dynamic function name lookup by stringing
together the event name with the parameter name.
This way, we could look the ICI interface on the plugin side pretty much
what it looks now, except that we should probably restrict the event and
parameter names to identifier characters, lest we need name mangling to translate them to function names.

I had a look at the adapt branch to gauge if the there is really a point for
having the varargs stuff, i.e. events with parameters that are not readily
available in a single struct.
The unroll_parameter_handler / graphite_parameter_handler events are somewhat
odd because they have a varying set of parameters.  So they really have a list
of parameters.  We could do this with somehing like:
         invoke_plugin_va_callbacks (PLUGIN_UNROLL_PARAMETER_HANDLER,
                                     "_loop.id", EP_INT, loop->num,
                                     "_loop.can_duplicate_loop_p",
                                     EP_UNSIGNED_CHAR,
                                     ici_can_duplicate_loop_p);
And then have the plugin pick through the va_list.  Or, if preferred,
have a helper function - possibly invoked implicitly by the ICI
infrastructure - go through the va_list and build the hash table of arguments
from it so that the current parameter query interface can be used.
In fact, we could use such a mechanism in general, so if we go back to passing pointers to parameters instead of parameters, you'd have backward
compatibility on the plugin side.
OTOH, does using pointers to parameters really make this interface easy to
use - it seems you had to use this to avoid type compatibility issues, which
are sidestepped by using a va_list.  If we pass parameter values instead, you
could have a query function that tells you if a parameter is set.  Possibly
also with a pointer argument to assign the value if the parameter is set.
tests if a parameter exists - if you like

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.

Having a single call to invoke an event by number (even if it is a
varargs call) is certainly much faster than having all these
register_parameter / unregister_parameter calls, particularly in the
case that no plugin has registered a callback for the event.

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 ;) !..

I thought you were hoping to push some ICI infrastructure into mainline
in time for 4.5 .  In order to do that
- Time is of the essence.
- We should reduce the impact that ICI has on the rest of GCC.

Reply via email to