diff mbox

Patch: New GTY ((atomic)) option

Message ID 1305828635.647320080@www2.webmail.us
State New
Headers show

Commit Message

Nicola Pero May 19, 2011, 6:10 p.m. UTC
>>        * gengtype.c (walk_type): Implemented "atomic" GTY option.
>>        * doc/gty.texi (GTY Options): Document "atomic" GTY option.
>
> The patch is OK, with difference between "skip" and "atomic" options
> documented. (Can be done as a follow-up patch).

Thanks for the quick review.  Here's an updated patch with revised
documentation.

Ok to go ?

Thanks

Comments

Basile Starynkevitch May 19, 2011, 7:45 p.m. UTC | #1
On Thu, 19 May 2011 20:10:35 +0200 (CEST)
"Nicola Pero" <nicola.pero@meta-innovation.com> wrote:

> >>        * gengtype.c (walk_type): Implemented "atomic" GTY option.
> >>        * doc/gty.texi (GTY Options): Document "atomic" GTY option.
> >
> > The patch is OK, with difference between "skip" and "atomic" options
> > documented. (Can be done as a follow-up patch).
> 
> Thanks for the quick review.  Here's an updated patch with revised
> documentation.
> 
> Ok to go ?

I have no power to approve that, but I find the patch quite nice.

However, did you check that the atomic qualifier is correctly written &
re-read in the state (I believe you did, otherwise it probably won't
work). This is needed for plugins using it, or using atomic qualified
fields of existing (or future) structures.

Regards
Basile Starynkevitch May 19, 2011, 8:07 p.m. UTC | #2
On Thu, 19 May 2011 21:45:49 +0200
Basile Starynkevitch <basile@starynkevitch.net> wrote:
> However, did you check that the atomic qualifier is correctly written &
> re-read in the state (I believe you did, otherwise it probably won't
> work). This is needed for plugins using it, or using atomic qualified
> fields of existing (or future) structures.

This also brings a wish. Now that gengtype is plugin friendly, I would
find great if some testcases appeared for gengtype on plugins. That
would be the good way to test new GTY or gengtype features.
Unfortunately, this probably requires fluency with dejagnu & expect,
which I don't have. The dream I have is to be able to test with dejagnu
that gengtype -P is able to generate some gt*.h file with such and such
regexp patterns.

To say it otherwise, testing new GTY features is best done on
plugin-like code, because these new featurres cannot be used before
gengtype has been properly extended. That "plugin" code don't need to
do something useful, or even to work as a GCC plugin; it simply have to
be passed to gengtype with -P.

Regards.
Laurynas Biveinis May 20, 2011, 2:06 a.m. UTC | #3
2011/5/19 Nicola Pero <nicola.pero@meta-innovation.com>:
> Ok to go ?

> +2011-05-20  Nicola Pero  <nicola.pero@meta-innovation.com>
> +
> +       * gengtype.c (walk_type): Implemented "atomic" GTY option.
> +       * doc/gty.texi (GTY Options): Document "atomic" GTY option.

I assume it received the usual testing? If yes, then OK. Thanks!
diff mbox

Patch

Index: gcc/doc/gty.texi
===================================================================
--- gcc/doc/gty.texi    (revision 173917)
+++ gcc/doc/gty.texi    (working copy)
@@ -383,6 +383,51 @@ 
   size_t size = sizeof (struct sorted_fields_type) + n * sizeof (tree);
 @end smallexample
 
+@findex atomic
+@item atomic
+
+The @code{atomic} option can only be used with pointers.  It informs
+the GC machinery that the memory that the pointer points to does not
+contain any pointers, and hence it should be treated by the GC and PCH
+machinery as an ``atomic'' block of memory that does not need to be
+examined when scanning memory for pointers.  In particular, the
+machinery will not scan that memory for pointers to mark them as
+reachable (when marking pointers for GC) or to relocate them (when
+writing a PCH file).
+
+The @code{atomic} option differs from the @code{skip} option.
+@code{atomic} keeps the memory under Garbage Collection, but makes the
+GC ignore the contents of the memory.  @code{skip} is more drastic in
+that it causes the pointer and the memory to be completely ignored by
+the Garbage Collector.  So, memory marked as @code{atomic} is
+automatically freed when no longer reachable, while memory marked as
+@code{skip} is not.
+
+The @code{atomic} option must be used with great care, because all
+sorts of problem can occur if used incorrectly, that is, if the memory
+the pointer points to does actually contain a pointer.
+
+Here is an example of how to use it:
+@smallexample
+struct GTY(()) my_struct @{
+  int number_of_elements;
+  unsigned int GTY ((atomic)) * elements;
+@};
+@end smallexample
+In this case, @code{elements} is a pointer under GC, and the memory it
+points to needs to be allocated using the Garbage Collector, and will
+be freed automatically by the Garbage Collector when it is no longer
+referenced.  But the memory that the pointer points to is an array of
+@code{unsigned int} elements, and the GC must not try to scan it to
+find pointers to mark or relocate, which is why it is marked with the
+@code{atomic} option.
+
+Note that, currently, global variables can not be marked with
+@code{atomic}; only fields of a struct can.  This is a known
+limitation.  It would be useful to be able to mark global pointers
+with @code{atomic} to make the PCH machinery aware of them so that
+they are saved and restored correctly to PCH files.
+
 @findex special
 @item special ("@var{name}")
 
Index: gcc/gengtype.c
===================================================================
--- gcc/gengtype.c      (revision 173917)
+++ gcc/gengtype.c      (working copy)
@@ -2386,6 +2386,7 @@ 
   int maybe_undef_p = 0;
   int use_param_num = -1;
   int use_params_p = 0;
+  int atomic_p = 0;
   options_p oo;
   const struct nested_ptr_data *nested_ptr_d = NULL;
 
@@ -2415,6 +2416,8 @@ 
       ;
     else if (strcmp (oo->name, "skip") == 0)
       ;
+    else if (strcmp (oo->name, "atomic") == 0)
+      atomic_p = 1;
     else if (strcmp (oo->name, "default") == 0)
       ;
     else if (strcmp (oo->name, "param_is") == 0)
@@ -2480,6 +2483,12 @@ 
       return;
     }
 
+  if (atomic_p && (t->kind != TYPE_POINTER))
+    {
+      error_at_line (d->line, "field `%s' has invalid option `atomic'\n", d->val);
+      return;
+    }
+
   switch (t->kind)
     {
     case TYPE_SCALAR:
@@ -2495,6 +2504,25 @@ 
            break;
          }
 
+       /* If a pointer type is marked as "atomic", we process the
+          field itself, but we don't walk the data that they point to.
+          
+          There are two main cases where we walk types: to mark
+          pointers that are reachable, and to relocate pointers when
+          writing a PCH file.  In both cases, an atomic pointer is
+          itself marked or relocated, but the memory that it points
+          to is left untouched.  In the case of PCH, that memory will
+          be read/written unchanged to the PCH file.  */
+       if (atomic_p)
+         {
+           oprintf (d->of, "%*sif (%s != NULL) {\n", d->indent, "", d->val);
+           d->indent += 2;
+           d->process_field (t, d);
+           d->indent -= 2;
+           oprintf (d->of, "%*s}\n", d->indent, "");
+           break;
+         }
+
        if (!length)
          {
            if (!UNION_OR_STRUCT_P (t->u.p)
Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog       (revision 173917)
+++ gcc/ChangeLog       (working copy)
@@ -1,3 +1,8 @@ 
+2011-05-20  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * gengtype.c (walk_type): Implemented "atomic" GTY option.
+       * doc/gty.texi (GTY Options): Document "atomic" GTY option.
+
 2011-05-19  Joseph Myers  <joseph@codesourcery.com>
 
        * config/arm/arm-fpus.def: New.