Re: [plugins] RFA: Merge plugins into mainline [3/5 - ME]

[Ian Lance Taylor <iant at google dot com>]

Diego Novillo <dnovillo@google.com> writes:

> 2009-04-02  Le-Chun Wu  <lcwu@google.com>
>
> 	* tree-pass.h (register_one_dump_file): Add a prototype for
> 	register_one_dump_file.
> 	* toplev.c (compile_file): Call initialize_plugins.
> 	(do_compile): Call invoke_plugin_callbacks.
> 	(toplev_main): Call invoke_plugin_callbacks.
> 	* opts.c (common_handle_option): Handle OPT_fplugin_ and
> 	OPT_fplugin_arg_.
> 	* common.opt: Add -fplugin= and -fplugin-arg-.
> 	* gcc-plugin.h: New public header file for plugins to include.
> 	* plugin.c: New source file.
> 	* plugin.h: New internal header file.
> 	* passes.c (register_one_dump_file): Make it external.


> +#include "errors.h"
> +#include "toplev.h"

It doesn't make sense to include both "errors.h" and "toplev.h".
"errors.h" declares error() and friends for use in programs which are
not linked with the diagnostic machinery--e.g., genattrtab.  "toplev.h"
is for programs which are linked with the gcc middle-end.


> +  if (*slot)
> +    {
> +      plugin = (struct plugin_name_args *) *slot;
> +      if (strcmp (plugin->full_name, plugin_name))
> +        error (G_("Plugin %s was specified with different paths:\n%s\n%s"),
> +               plugin->base_name, plugin->full_name, plugin_name);
> +      return;
> +    }

I don't think you need G_ here.  You only need that for strings which
are passed indirectly to error.

> +          if (key_parsed)
> +            {
> +              error (G_("Malformed option -fplugin-arg-%s"
> +                        " (multiple '=' signs)"), arg);
> +              return;
> +            }

Here too.

> +  if (!key_start)
> +    {
> +      error (G_("Malformed option -fplugin-arg-%s (missing -<key>[=<value>])"),
> +             arg);
> +      return;
> +    }

And here.  And elsewhere, which I won't mention.


> +/* Insert the plugin pass at the proper position. Return true if the pass 
> +   is successfully added.
> +
> +   PLUGIN_PASS_INFO - new pass to be inserted
> +   PASS_LIST        - root of the pass list to insert the new pass to  */
> +

You don't have to do it now, but I would like to permit backends to
register their own passes.  That suggests that most of this code should
move to tree-pass.c.


> +      case PLUGIN_FINISH_STRUCT:
> +      case PLUGIN_FINISH_UNIT:
> +      case PLUGIN_CXX_CP_PRE_GENERICIZE:
> +      case PLUGIN_FINISH:
> +        {
> +          struct callback_info *new_callback;
> +          if (!callback)
> +            {
> +              error (G_("Plugin %s registered a null callback function"),
> +                     plugin_name);
> +              return;
> +            }

Have the error message say which callback was not registered, at least
as an index.


+void
+invoke_plugin_callbacks (enum plugin_event event, void *gcc_data)

How about a new timevar for all plugin activity?  We would want to use
it here and in initialize_plugins.  Plugins which want more
sophisticated control can do their own timevar manipulation.


Ian