Re: adding GGC events for plugins.

[Diego Novillo <dnovillo at google dot com>]

Some comments in addition to what's been suggested earlier in the
thread:


On Tue, May 12, 2009 at 07:34, Basile STARYNKEVITCH
<basile@starynkevitch.net> wrote:
> 2009-05-12 ÂBasile Starynkevitch Â<basile@starynkevitch.net>
> Â Â Â Â* gcc/doc/plugins.texi: Mentioned PLUGIN_GGC_START,

Blank line after the date.  The file name should be relative to
the same directory where the ChangeLog file is.  See other
entries in gcc/ChangeLog for reference.

> Â Â Â ÂPLUGIN_GGC_MARKING, PLUGIN_GGC_END.
> Â Â Â Â* gcc/ggc.h: Declared ggc_register_root_tab.
> Â Â Â Â* gcc/gcc-plugin.h (enum plugin_event): Added PLUGIN_GGC_START,
> Â Â Â ÂPLUGIN_GGC_MARKING, PLUGIN_GGC_END, PLUGIN_REGISTER_GGC_ROOTS.
> Â Â Â Â* gcc/ggc-common.c: including "plugin.h".
> Â Â Â Â(extra_root_tab, extra_root_count, extra_root_length): new static
> Â Â Â Âvariables.
> Â Â Â Â(ggc_register_root_tab): added new function.
> Â Â Â Â(ggc_mark_roots): scan the extra_root_tab if needed, and do the
> Â Â Â ÂPLUGIN_GGC_MARKING callback.
> Â Â Â Â* gcc/ggc-zone.c (ggc_collect): call PLUGIN_GGC_START &
> Â Â Â ÂPLUGIN_GGC_END callbacks.
> Â Â Â Â* gcc/ggc-page.c (ggc_collect): likewise.
> Â Â Â Â* gcc/plugin.c (plugin_event_name, register_callback)
> Â Â Â Â(invoke_plugin_callbacks): handle PLUGIN_GGC_START,
> Â Â Â ÂPLUGIN_GGC_MARKING, PLUGIN_GGC_END, PLUGIN_REGISTER_GGC_ROOTS.
> Â Â Â Â* gcc/Makefile.in: plugin.o also depends upon $(GGC_H).
>
>

> Index: gcc/doc/plugins.texi
> ===================================================================
> --- gcc/doc/plugins.texi	(revision 147426)
> +++ gcc/doc/plugins.texi	(working copy)
> @@ -9,7 +9,7 @@
>
>  @section Loading Plugins
>
> -Plugins are supported on platforms that support @option{-ld
> +Plugins are supported on platforms that support @option{-ldl
>  -rdynamic}.  They are loaded by the compiler using @code{dlopen}
>  and invoked at pre-determined locations in the compilation
>  process.
> @@ -71,6 +71,9 @@
>    PLUGIN_FINISH_UNIT,           /* Useful for summary processing.  */
>    PLUGIN_CXX_CP_PRE_GENERICIZE, /* Allows to see low level AST in C++ FE.  */
>    PLUGIN_FINISH,                /* Called before GCC exits.  */
> +  PLUGIN_GGC_START,		/* Called at start of Gcc Garbage Collection */

s/Gcc/GCC/

> +
> +/* extra table of dynamically registers root tab, its count and allocated length */

Comments should be complete sentences and end in '.  */'

> +static const struct ggc_root_tab**extra_root_tab;
> +static size_t extra_root_count;
> +static size_t extra_root_length;

Better use a VEC() here.

> +
> +void
> +ggc_register_root_tab (const struct ggc_root_tab* rt)
> +{

Needs comment.

> +
> +  /* some plugins may need to register their routine mark additional
> +     data */

Fix comment formatting.

> +  /* Some plugins may need to know when GGC is really starting */
> +  invoke_plugin_callbacks (PLUGIN_GGC_START, NULL);

No need to add a comment here.  The call is self-explanatory.

> +  /* Some plugins may need to know when GGC ended */
> +  invoke_plugin_callbacks (PLUGIN_GGC_END, NULL);

Likewise.

> +  /* Some plugins may need to know when GGC is really starting */
> +  invoke_plugin_callbacks (PLUGIN_GGC_START, NULL);

Likewise.

> +
>    clear_marks ();
>    ggc_mark_roots ();
>  #ifdef GATHER_STATISTICS
> @@ -1948,6 +1952,9 @@
>
>    G.allocated_last_gc = G.allocated;
>
> +  /* Some plugins may need to know when GGC ended */
> +  invoke_plugin_callbacks (PLUGIN_GGC_END, NULL);

Likewise.

This also needs a testcase and documentation explaining how to
use it.


Diego.