This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PLUGIN] Support for registering cache tables from plugins
Hi Ian, thanks for reviewing this patch. Rafael and Basile already
pointed out some of these issues. I posted a revised patch here:
http://gcc.gnu.org/ml/gcc-patches/2009-09/msg00389.html
Since my original patch was a copy-and-paste of the existing code
for normal roots, I took the liberty of improving that code too.
I've attached an updated patch that takes your comments into account.
I think this needs a sentence about what a GGC cache table is.
I did this already in the above patch, by referring to "if_marked".
If you look up "if_marked" in the docs you find a reasonable amount
of info explaining what this is all about.
Also,
it needs to say the type of user_data (struct ggc_cache_tab *).
Done.
Really this needs a bunch more documentation some day--I can't imagine
many people being able to use it successfully.
The main difficulty I found was in getting gengtype to work. Without
Basile's patches I couldn't get it to produce anything useful.
Index: mainline/gcc/ggc-common.c
===================================================================
--- mainline.orig/gcc/ggc-common.c 2009-09-04 15:05:45.465074515 +0200
+++ mainline/gcc/ggc-common.c 2009-09-04 15:10:10.909100327 +0200
@@ -116,6 +116,33 @@
}
+/* This extra vector of dynamically registered cache_tab-s is used by
+ ggc_mark_roots and gives the ability to dynamically add new GGC cache
+ tables, for instance from some plugins; this vector is a heap one
+ [since it is used by GGC internally!] */
+typedef const struct ggc_cache_tab* const_ggc_cache_tab_t;
s/* / */
I would drop the square brackets and just make it a subclause. "this
vector is on the heap since it is used by GGC internally."
Done.
+DEF_VEC_P(const_ggc_cache_tab_t);
+DEF_VEC_ALLOC_P(const_ggc_cache_tab_t, heap);
+static VEC(const_ggc_cache_tab_t, heap) *extra_cache_vec;
+
+
Just one blank line here and above the comment.
Done.
Skip the VEC_alloc. The VEC_safe_push is all you need. It will do
the right thing when extra_cache_vec is NULL. It suffices to write
if (ct != NULL)
VEC_safe_push (const_ggc_cache_tab_t, heap, extra_cache_vec, ct);
Already done in the above patch.
@@ -165,6 +192,25 @@
ggc_set_mark ((*cti->base)->entries);
}
+ if (extra_cache_vec
+ && VEC_length(const_ggc_cache_tab_t,extra_cache_vec) > 0)
Just check VEC_length here.
Already done (a bit differently) in the above patch.
Space before left parenthesis.
Done.
Space
after comma.
Already done.
+ {
+ const_ggc_cache_tab_t ctp = NULL;
+ for (i=0;
Spaces around '='.
Already done.
+ VEC_iterate(const_ggc_cache_tab_t, extra_cache_vec, i, ctp);
Space before left parenthesis.
Done.
+ i++)
+ {
+ for (cti = ctp; cti->base != NULL; cti++)
+ if (*cti->base)
+ {
+ ggc_set_mark (*cti->base);
+ htab_traverse_noresize (*cti->base, ggc_htab_delete,
+ CONST_CAST (void *, (const void *)cti));
+ ggc_set_mark ((*cti->base)->entries);
+ }
+ }
+ }
Please pull the entire for loop into a separate small function, and
call that function from the loop on gt_ggc_cache_rtab as well.
Done.
+/* Register an additional cache table. This can be useful for some
+ plugins. Does nothing if the passed pointer is null. */
+extern void ggc_register_cache_tab (const struct ggc_cache_tab *);
s/null/NULL/
Done.
This patch is OK with those changes.
Thanks. If no-one objects to the new patch in the next day or so,
I will apply it.
Ciao,
Duncan.
Index: mainline/gcc/doc/plugins.texi
===================================================================
--- mainline.orig/gcc/doc/plugins.texi 2009-09-27 10:19:09.802652319 +0200
+++ mainline/gcc/doc/plugins.texi 2009-09-27 13:42:08.762649485 +0200
@@ -133,6 +133,7 @@
PLUGIN_GGC_MARKING, /* Extend the GGC marking. */
PLUGIN_GGC_END, /* Called at end of GGC. */
PLUGIN_REGISTER_GGC_ROOTS, /* Register an extra GGC root table. */
+ PLUGIN_REGISTER_GGC_CACHES, /* Register an extra GGC cache table. */
PLUGIN_ATTRIBUTES, /* Called during attribute registration */
PLUGIN_START_UNIT, /* Called before processing a translation unit. */
PLUGIN_EVENT_LAST /* Dummy event used for indexing callback
@@ -151,8 +152,8 @@
@item @code{void *user_data}: Pointer to plugin-specific data.
@end itemize
-For the PLUGIN_PASS_MANAGER_SETUP, PLUGIN_INFO, and
-PLUGIN_REGISTER_GGC_ROOTS pseudo-events the @code{callback} should be
+For the PLUGIN_PASS_MANAGER_SETUP, PLUGIN_INFO, PLUGIN_REGISTER_GGC_ROOTS
+and PLUGIN_REGISTER_GGC_CACHES pseudo-events the @code{callback} should be
null, and the @code{user_data} is specific.
@section Interacting with the pass manager
@@ -222,16 +223,19 @@
(and conversely, these routines should usually not be used in plugins
outside of the @code{PLUGIN_GGC_MARKING} event).
-Some plugins may need to add extra GGC root tables, e.g. to handle
-their own @code{GTY}-ed data. This can be done with the
-@code{PLUGIN_REGISTER_GGC_ROOTS} pseudo-event with a null callback and
-the extra root table as @code{user_data}. Running the @code{gengtype
--p @var{source-dir} @var{file-list} @var{plugin*.c} ...} utility
-generates this extra root table.
+Some plugins may need to add extra GGC root tables, e.g. to handle their own
+@code{GTY}-ed data. This can be done with the @code{PLUGIN_REGISTER_GGC_ROOTS}
+pseudo-event with a null callback and the extra root table (of type @code{struct
+ggc_root_tab*}) as @code{user_data}. Plugins that want to use the
+@code{if_marked} hash table option can add the extra GGC cache tables generated
+by @code{gengtype} using the @code{PLUGIN_REGISTER_GGC_CACHES} pseudo-event with
+a null callback and the extra cache table (of type @code{struct ggc_cache_tab*})
+as @code{user_data}. Running the @code{gengtype -p @var{source-dir}
+@var{file-list} @var{plugin*.c} ...} utility generates these extra root tables.
You should understand the details of memory management inside GCC
-before using @code{PLUGIN_GGC_MARKING} or
-@code{PLUGIN_REGISTER_GGC_ROOTS}.
+before using @code{PLUGIN_GGC_MARKING}, @code{PLUGIN_REGISTER_GGC_ROOTS}
+or @code{PLUGIN_REGISTER_GGC_CACHES}.
@section Giving information about a plugin
Index: mainline/gcc/gcc-plugin.h
===================================================================
--- mainline.orig/gcc/gcc-plugin.h 2009-09-27 10:19:09.794650747 +0200
+++ mainline/gcc/gcc-plugin.h 2009-09-27 10:19:17.966651204 +0200
@@ -40,6 +40,7 @@
PLUGIN_GGC_MARKING, /* Extend the GGC marking. */
PLUGIN_GGC_END, /* Called at end of GGC. */
PLUGIN_REGISTER_GGC_ROOTS, /* Register an extra GGC root table. */
+ PLUGIN_REGISTER_GGC_CACHES, /* Register an extra GGC cache table. */
PLUGIN_ATTRIBUTES, /* Called during attribute registration. */
PLUGIN_START_UNIT, /* Called before processing a translation unit. */
PLUGIN_EVENT_LAST /* Dummy event used for indexing callback
@@ -144,8 +145,8 @@
*/
/* This is also called without a callback routine for the
- PLUGIN_PASS_MANAGER_SETUP, PLUGIN_INFO, PLUGIN_REGISTER_GGC_ROOTS
- pseudo-events, with a specific user_data.
+ PLUGIN_PASS_MANAGER_SETUP, PLUGIN_INFO, PLUGIN_REGISTER_GGC_ROOTS and
+ PLUGIN_REGISTER_GGC_CACHES pseudo-events, with a specific user_data.
*/
extern void register_callback (const char *plugin_name,
Index: mainline/gcc/ggc-common.c
===================================================================
--- mainline.orig/gcc/ggc-common.c 2009-09-27 10:19:09.786652109 +0200
+++ mainline/gcc/ggc-common.c 2009-09-27 14:05:15.421652325 +0200
@@ -91,30 +91,59 @@
/* This extra vector of dynamically registered root_tab-s is used by
ggc_mark_roots and gives the ability to dynamically add new GGC root
- tables, for instance from some plugins; this vector is a heap one
- [since it is used by GGC internally!] */
-typedef const struct ggc_root_tab* const_ggc_root_tab_t;
+ tables, for instance from some plugins; this vector is on the heap
+ since it is used by GGC internally. */
+typedef const struct ggc_root_tab *const_ggc_root_tab_t;
DEF_VEC_P(const_ggc_root_tab_t);
DEF_VEC_ALLOC_P(const_ggc_root_tab_t, heap);
static VEC(const_ggc_root_tab_t, heap) *extra_root_vec;
-
/* Dynamically register a new GGC root table RT. This is useful for
plugins. */
void
ggc_register_root_tab (const struct ggc_root_tab* rt)
{
- if (!rt)
- return;
- if (!extra_root_vec)
- {
- int vlen = 32;
- extra_root_vec = VEC_alloc (const_ggc_root_tab_t, heap, vlen);
- }
- VEC_safe_push (const_ggc_root_tab_t, heap, extra_root_vec, rt);
+ if (rt)
+ VEC_safe_push (const_ggc_root_tab_t, heap, extra_root_vec, rt);
+}
+
+/* This extra vector of dynamically registered cache_tab-s is used by
+ ggc_mark_roots and gives the ability to dynamically add new GGC cache
+ tables, for instance from some plugins; this vector is on the heap
+ since it is used by GGC internally. */
+typedef const struct ggc_cache_tab *const_ggc_cache_tab_t;
+DEF_VEC_P(const_ggc_cache_tab_t);
+DEF_VEC_ALLOC_P(const_ggc_cache_tab_t, heap);
+static VEC(const_ggc_cache_tab_t, heap) *extra_cache_vec;
+
+/* Dynamically register a new GGC cache table CT. This is useful for
+ plugins. */
+
+void
+ggc_register_cache_tab (const struct ggc_cache_tab* ct)
+{
+ if (ct)
+ VEC_safe_push (const_ggc_cache_tab_t, heap, extra_cache_vec, ct);
}
+/* Scan a hash table that has objects which are to be deleted if they are not
+ already marked. */
+
+static void
+ggc_scan_cache_tab (const_ggc_cache_tab_t ctp)
+{
+ const struct ggc_cache_tab *cti;
+
+ for (cti = ctp; cti->base != NULL; cti++)
+ if (*cti->base)
+ {
+ ggc_set_mark (*cti->base);
+ htab_traverse_noresize (*cti->base, ggc_htab_delete,
+ CONST_CAST (void *, (const void *)cti));
+ ggc_set_mark ((*cti->base)->entries);
+ }
+}
/* Iterate through all registered roots and mark each element. */
@@ -123,8 +152,9 @@
{
const struct ggc_root_tab *const *rt;
const struct ggc_root_tab *rti;
+ const_ggc_root_tab_t rtp;
const struct ggc_cache_tab *const *ct;
- const struct ggc_cache_tab *cti;
+ const_ggc_cache_tab_t ctp;
size_t i;
for (rt = gt_ggc_deletable_rtab; *rt; rt++)
@@ -136,18 +166,11 @@
for (i = 0; i < rti->nelt; i++)
(*rti->cb) (*(void **)((char *)rti->base + rti->stride * i));
- if (extra_root_vec
- && VEC_length(const_ggc_root_tab_t,extra_root_vec) > 0)
+ for (i = 0; VEC_iterate (const_ggc_root_tab_t, extra_root_vec, i, rtp); i++)
{
- const_ggc_root_tab_t rtp = NULL;
- for (i=0;
- VEC_iterate(const_ggc_root_tab_t, extra_root_vec, i, rtp);
- i++)
- {
- for (rti = rtp; rti->base != NULL; rti++)
- for (i = 0; i < rti->nelt; i++)
- (*rti->cb) (*(void **) ((char *)rti->base + rti->stride * i));
- }
+ for (rti = rtp; rti->base != NULL; rti++)
+ for (i = 0; i < rti->nelt; i++)
+ (*rti->cb) (*(void **) ((char *)rti->base + rti->stride * i));
}
if (ggc_protect_identifiers)
@@ -156,14 +179,10 @@
/* Now scan all hash tables that have objects which are to be deleted if
they are not already marked. */
for (ct = gt_ggc_cache_rtab; *ct; ct++)
- for (cti = *ct; cti->base != NULL; cti++)
- if (*cti->base)
- {
- ggc_set_mark (*cti->base);
- htab_traverse_noresize (*cti->base, ggc_htab_delete,
- CONST_CAST (void *, (const void *)cti));
- ggc_set_mark ((*cti->base)->entries);
- }
+ ggc_scan_cache_tab (*ct);
+
+ for (i = 0; VEC_iterate (const_ggc_cache_tab_t, extra_cache_vec, i, ctp); i++)
+ ggc_scan_cache_tab (ctp);
if (! ggc_protect_identifiers)
ggc_purge_stringpool ();
Index: mainline/gcc/ggc.h
===================================================================
--- mainline.orig/gcc/ggc.h 2009-09-27 10:19:09.790653384 +0200
+++ mainline/gcc/ggc.h 2009-09-27 13:45:56.253649811 +0200
@@ -272,9 +272,13 @@
extern void ggc_collect (void);
/* Register an additional root table. This can be useful for some
- plugins. Does nothing if the passed pointer is null. */
+ plugins. Does nothing if the passed pointer is NULL. */
extern void ggc_register_root_tab (const struct ggc_root_tab *);
+/* Register an additional cache table. This can be useful for some
+ plugins. Does nothing if the passed pointer is NULL. */
+extern void ggc_register_cache_tab (const struct ggc_cache_tab *);
+
/* Return the number of bytes allocated at the indicated address. */
extern size_t ggc_get_size (const void *);
Index: mainline/gcc/plugin.c
===================================================================
--- mainline.orig/gcc/plugin.c 2009-09-27 10:19:09.806656038 +0200
+++ mainline/gcc/plugin.c 2009-09-27 10:19:17.998651548 +0200
@@ -57,6 +57,7 @@
"PLUGIN_GGC_MARKING",
"PLUGIN_GGC_END",
"PLUGIN_REGISTER_GGC_ROOTS",
+ "PLUGIN_REGISTER_GGC_CACHES",
"PLUGIN_START_UNIT",
"PLUGIN_EVENT_LAST"
};
@@ -499,6 +500,10 @@
gcc_assert (!callback);
ggc_register_root_tab ((const struct ggc_root_tab*) user_data);
break;
+ case PLUGIN_REGISTER_GGC_CACHES:
+ gcc_assert (!callback);
+ ggc_register_cache_tab ((const struct ggc_cache_tab*) user_data);
+ break;
case PLUGIN_FINISH_TYPE:
case PLUGIN_START_UNIT:
case PLUGIN_FINISH_UNIT:
@@ -566,6 +571,7 @@
case PLUGIN_PASS_MANAGER_SETUP:
case PLUGIN_EVENT_LAST:
case PLUGIN_REGISTER_GGC_ROOTS:
+ case PLUGIN_REGISTER_GGC_CACHES:
default:
gcc_assert (false);
}