Le-Chun Wu wrote:
Hi Sean,
It's great that you updated the wiki page with the latest and more
detailed API design.
We (at Google) also started to look at the GCC plugin support a couple
of weeks ago. We had a quick prototype implemented based on the
original APIs that Taras put together in the old wiki. (I can send out
the patch later for people's reference.) The updated APIs in general
look good to me. I do have some comments based on our experience with
the initial prototyping:
Neat! I'd love to see that.
void register_callbacks(int nregistrations, struct plugin_registration
*registrations);
Instead of passing in an array of plugin_registration objects with a
single register_callbacks call, it's probably better to have the
plugin call a sequence of register_callback with the necessary
information, as shown in the following example:
void plugin_register_callback (const char *plugin_name, enum
plugin_event event, plugin_callback_func callback, void *user_data);
/* In plugin code */
void
plugin_init()
{
...
register_callback (plugin_name, PLUGIN_FINISH_STRUCT, handle_struct, NULL);
register_callback (plugin_name, PLUGIN_FINISH_UNIT,
handle_end_of_compilation_unit, NULL);
...
}
In the function body of register_callback, GCC can then create the
plugin_registration objects and chain them together based on the event
type. Because GCC will need to maintain a list of plugin_registration
objects for each event anyway, we might as well let it create (and
destroy if necessary) the objects instead of leaving the task to the
plugins.
Note that in my example an additional parameter, plugin_name, is added
in register_callback. We found it useful to have the name around when
emitting error messages if something goes wrong during the callback
registration process.
Sean,
I agree with Le-Chun that separate register_callback calls would be
better. Would you change that(in the interest of not having too many
people modify the api), or should I? By the way, reformatting made the
page much more readable, thanks.
I think having a plugin-name parameter is something we can decide on
later ( just like the version stuff). I can see it being useful for
debugging a situation with multiple loaded plugins, but I'm not
convinced that it will be a common problem.
-fplugin=/path/to/plugin.so;arg1=value;arg2=value;...
I am not sure if it is GCC's responsibility to understand key-value
(or any other types of) arguments to plugins. I think GCC should
simply take a string (which, of course, can be something like
"arg1=value arg2=value") and pass it (unparsed) to the plugin. It is
plugin's job to parse/process the given arguments (whatever way it
likes). So the prototype of the plugin_init would look like this:
void plugin_init (const char *plugin_name, const char *plugin_arg);
In our current prototype, we implemented the originally proposed flag
"-fplugin-arg=", which is associated with the plugin specified in the
most recently parsed "-fplugin" flag. If a "-fplugin-arg" flag is used
in the command-line without any preceding "-fplugin", an error message
is emitted. Having the plugin name and its arguments concatenated as
currently proposed is also fine, but I would prefer a simple string
(like below) to a series of key-value pairs.
-fplugin=/path/to/plugin.so;"arguments"
(Note that the double quotes may not needed if the "arguments" doesn't
contain characters such as spaces.)
I agree with Daniel Jacobowitz's comment that letting every plugin to
parse command-lines will lead to insanity. I'd be nice to let GCC take
care of as much as possible there.
From my brief encounter with the complexity of parameter handling code
in GCC I would be tempted to start with the simplest possible proposal:
either use -fplugin-arg as I described or stuffing everything into
-fplugin and using a semi-colon separator.
Infact, I would prefer not using -fplugin-arg as that leaves more room
for future implementations to use -fplugin-<name>-<whatever> to do
plugin arguments.
Pass Manager
We think it would be quite daunting (and probably open up a can of
worms) to allow plugins to re-order passes. So to get things moving
quickly, in our initial prototype we only support insertion of a new
pass and replacing an existing pass. When registering a new pass with
GCC, the plugin uses the normal register_callback call with the
PLUGIN_PASS_MANAGER_SETUP event. However, instead of registering a
callback function, it passes in a plugin_pass object (see below) that
specifies the opt_pass object of the new pass, the name of the
reference pass, the static instance number of the reference pass, and
whether to insert before/after or replacing the reference pass.
enum pass_positioning_ops
{
PASS_POS_INSERT_AFTER,
PASS_POS_INSERT_BEFORE,
PASS_POS_REPLACE
};
struct plugin_pass
{
struct opt_pass *pass;
const char *reference_pass_name;
/* Insert the pass at the specified instance of the reference pass.
If it's 0, do that for every instance. */
int ref_pass_instance_number;
enum pass_positioning_ops pos_op;
};
/* In plugin code */
void
plugin_init()
{
...
register_callback (plugin_name, PLUGIN_PASS_MANAGER_SETUP, NULL, &pass_info);
...
}
When registering and positioning a new pass, GCC will search the
passes chained up in all_lowering_passes, all_ipa_passes, and
all_passes (similar to Mozilla dehydra's implementation) to find the
reference pass(es) with the matching name and instance number, and
then either insert the new pass or replace the reference pass.
One caveat with our current implementation is that because the
registration of new plugin passes happens after the command-line
options are parsed, we cannot specify single pass dumping for plugin
passes (e.g. -fdump-tree-newpass). IR dumping of plugin passes is
enabled only when the dump-all flags (e.g. -fdump-tree-all) are
specified.
What do people think about this pass registration/positioning
interface that I described? (Again, I will send out our patch later so
that people can get a better idea if my description is not clear
enough.)
I think a better way to go about it would be to cleanup the existing
pass manager API to allow plugins to introspect/modify it better. I'd
rather focus on the bare plugin API at this moment.
Cheers,
Taras
