PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode.

PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode.

[Basile Starynkevitch]

[-- Attachment #1: Type: text/plain, Size: 890 bytes --]

Hello All,

When gengtype is used in plugin mode, it should also generate various 
#define ggc_alloc_* macro definitions. It seems that the 4.6.0 gengtype 
don't do that (and that is a bug).

The attached patch, to trunk rev171950, fixes this issue.

############# gcc/ChangeLog entry
2011-04-04  Basile Starynkevitch  <basile@starynkevitch.net>
	* gengtype.c (write_typed_alloc_def): Gets extra outf_p argument 
	and use it.
	(write_typed_struct_alloc_def, write_typed_typedef_alloc_def)
	(write_typed_alloc_defns): Likewise.
	(main): Calls write_typed_alloc_defns with output_header.
#############

Ok for trunk? Comments are welcome!

Cheers.
-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

[-- Attachment #2: gengtype-alloc-r171950.diff --]
[-- Type: text/x-diff, Size: 6440 bytes --]

Index: gcc/gengtype.c
===================================================================
--- gcc/gengtype.c	(revision 171950)
+++ gcc/gengtype.c	(working copy)
@@ -4197,52 +4197,56 @@ enum alloc_zone
    the allocator will be zone-specific.  */
 
 static void
-write_typed_alloc_def (bool variable_size, const char *type_specifier,
+write_typed_alloc_def (outf_p f, 
+		       bool variable_size, const char *type_specifier,
 		       const char *type_name, const char *allocator_type,
 		       enum alloc_quantity quantity, enum alloc_zone zone)
 {
   bool two_args = variable_size && (quantity == vector);
   bool third_arg = ((zone == specific_zone)
 		    && (variable_size || (quantity == vector)));
-
-  oprintf (header_file, "#define ggc_alloc_%s%s", allocator_type, type_name);
-  oprintf (header_file, "(%s%s%s%s%s) ",
+  if (!f)
+    return;
+  oprintf (f, "#define ggc_alloc_%s%s", allocator_type, type_name);
+  oprintf (f, "(%s%s%s%s%s) ",
 	   (variable_size ? "SIZE" : ""),
 	   (two_args ? ", " : ""),
 	   (quantity == vector) ? "n" : "",
 	   (third_arg ? ", " : ""), (zone == specific_zone) ? "z" : "");
-  oprintf (header_file, "((%s%s *)", type_specifier, type_name);
-  oprintf (header_file, "(ggc_internal_%salloc_stat (", allocator_type);
+  oprintf (f, "((%s%s *)", type_specifier, type_name);
+  oprintf (f, "(ggc_internal_%salloc_stat (", allocator_type);
   if (zone == specific_zone)
-    oprintf (header_file, "z, ");
+    oprintf (f, "z, ");
   if (variable_size)
-    oprintf (header_file, "SIZE");
+    oprintf (f, "SIZE");
   else
-    oprintf (header_file, "sizeof (%s%s)", type_specifier, type_name);
+    oprintf (f, "sizeof (%s%s)", type_specifier, type_name);
   if (quantity == vector)
-    oprintf (header_file, ", n");
-  oprintf (header_file, " MEM_STAT_INFO)))\n");
+    oprintf (f, ", n");
+  oprintf (f, " MEM_STAT_INFO)))\n");
 }
 
 /* Writes a typed allocator definition for a struct or union S.  */
 
 static void
-write_typed_struct_alloc_def (const type_p s, const char *allocator_type,
+write_typed_struct_alloc_def (outf_p f,
+			      const type_p s, const char *allocator_type,
 			      enum alloc_quantity quantity,
 			      enum alloc_zone zone)
 {
-  write_typed_alloc_def (variable_size_p (s), get_type_specifier (s),
+  write_typed_alloc_def (f, variable_size_p (s), get_type_specifier (s),
 			 s->u.s.tag, allocator_type, quantity, zone);
 }
 
 /* Writes a typed allocator definition for a typedef P.  */
 
 static void
-write_typed_typedef_alloc_def (const pair_p p, const char *allocator_type,
+write_typed_typedef_alloc_def (outf_p f,
+			       const pair_p p, const char *allocator_type,
 			       enum alloc_quantity quantity,
 			       enum alloc_zone zone)
 {
-  write_typed_alloc_def (variable_size_p (p->type), "", p->name,
+  write_typed_alloc_def (f, variable_size_p (p->type), "", p->name,
 			 allocator_type, quantity, zone);
 }
 
@@ -4250,43 +4254,46 @@ static void
    TYPEDEFS that are used by GC.  */
 
 static void
-write_typed_alloc_defns (const type_p structures, const pair_p typedefs)
+write_typed_alloc_defns (outf_p f,
+			 const type_p structures, const pair_p typedefs)
 {
   type_p s;
   pair_p p;
 
-  oprintf (header_file,
+  if (!f)
+    return;
+  oprintf (f,
 	   "\n/* Allocators for known structs and unions.  */\n\n");
   for (s = structures; s; s = s->next)
     {
       if (!USED_BY_TYPED_GC_P (s))
 	continue;
-      write_typed_struct_alloc_def (s, "", single, any_zone);
-      write_typed_struct_alloc_def (s, "cleared_", single, any_zone);
-      write_typed_struct_alloc_def (s, "vec_", vector, any_zone);
-      write_typed_struct_alloc_def (s, "cleared_vec_", vector, any_zone);
-      write_typed_struct_alloc_def (s, "zone_", single, specific_zone);
-      write_typed_struct_alloc_def (s, "zone_cleared_", single,
+      write_typed_struct_alloc_def (f, s, "", single, any_zone);
+      write_typed_struct_alloc_def (f, s, "cleared_", single, any_zone);
+      write_typed_struct_alloc_def (f, s, "vec_", vector, any_zone);
+      write_typed_struct_alloc_def (f, s, "cleared_vec_", vector, any_zone);
+      write_typed_struct_alloc_def (f, s, "zone_", single, specific_zone);
+      write_typed_struct_alloc_def (f, s, "zone_cleared_", single,
 				    specific_zone);
-      write_typed_struct_alloc_def (s, "zone_vec_", vector, specific_zone);
-      write_typed_struct_alloc_def (s, "zone_cleared_vec_", vector,
+      write_typed_struct_alloc_def (f, s, "zone_vec_", vector, specific_zone);
+      write_typed_struct_alloc_def (f, s, "zone_cleared_vec_", vector,
 				    specific_zone);
     }
 
-  oprintf (header_file, "\n/* Allocators for known typedefs.  */\n");
+  oprintf (f, "\n/* Allocators for known typedefs.  */\n");
   for (p = typedefs; p; p = p->next)
     {
       s = p->type;
       if (!USED_BY_TYPED_GC_P (s) || (strcmp (p->name, s->u.s.tag) == 0))
 	continue;
-      write_typed_typedef_alloc_def (p, "", single, any_zone);
-      write_typed_typedef_alloc_def (p, "cleared_", single, any_zone);
-      write_typed_typedef_alloc_def (p, "vec_", vector, any_zone);
-      write_typed_typedef_alloc_def (p, "cleared_vec_", vector, any_zone);
-      write_typed_typedef_alloc_def (p, "zone_", single, specific_zone);
-      write_typed_typedef_alloc_def (p, "zone_cleared_", single,
+      write_typed_typedef_alloc_def (f, p, "", single, any_zone);
+      write_typed_typedef_alloc_def (f, p, "cleared_", single, any_zone);
+      write_typed_typedef_alloc_def (f, p, "vec_", vector, any_zone);
+      write_typed_typedef_alloc_def (f, p, "cleared_vec_", vector, any_zone);
+      write_typed_typedef_alloc_def (f, p, "zone_", single, specific_zone);
+      write_typed_typedef_alloc_def (f, p, "zone_cleared_", single,
 				     specific_zone);
-      write_typed_typedef_alloc_def (p, "zone_cleared_vec_", vector,
+      write_typed_typedef_alloc_def (f, p, "zone_cleared_vec_", vector,
 				     specific_zone);
     }
 }
@@ -4990,8 +4997,8 @@ main (int argc, char **argv)
   open_base_files ();
 
   write_enum_defn (structures, param_structs);
-  write_typed_alloc_defns (structures, typedefs);
   output_header = plugin_output ? plugin_output : header_file;
+  write_typed_alloc_defns (output_header, structures, typedefs);
   DBGPRINT_COUNT_TYPE ("structures before write_types outputheader",
 		       structures);
   DBGPRINT_COUNT_TYPE ("param_structs before write_types outputheader",

Re: PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode.

[Laurynas Biveinis]

Hello Basile -

The patch is correct in general. But a few nitpicks:

> 2011-04-04  Basile Starynkevitch  <basile@starynkevitch.net>
>        * gengtype.c (write_typed_alloc_def): Gets extra outf_p argument
>        and use it.

...: New argument f.  Use it instead of header_file.

>        (write_typed_struct_alloc_def, write_typed_typedef_alloc_def)
>        (write_typed_alloc_defns): Likewise.
>        (main): Calls write_typed_alloc_defns with output_header.

> Index: gcc/gengtype.c
> ===================================================================
> --- gcc/gengtype.c	(revision 171950)
> +++ gcc/gengtype.c	(working copy)
> @@ -4197,52 +4197,56 @@ enum alloc_zone
>     the allocator will be zone-specific.  */
>
>  static void
> -write_typed_alloc_def (bool variable_size, const char *type_specifier,
> +write_typed_alloc_def (outf_p f,
> +		       bool variable_size, const char *type_specifier,

Here and for the rest of changed functions:
1) please describe f in the function header comment;
2) please align the second line of argument list.

> +  if (!f)
> +    return;

If f is NULL, it must be a bug, please use gcc_assert (f) instead.
Likewise in write_typed_alloc_defns.

> +  oprintf (f, "#define ggc_alloc_%s%s", allocator_type, type_name);
> +  oprintf (f, "(%s%s%s%s%s) ",
>  	   (variable_size ? "SIZE" : ""),

Here and in similar places please also align the argument list.

Thanks,
--
Laurynas

Re: PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode.

[Basile Starynkevitch]

[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]

On Thu, 7 Apr 2011 05:45:42 +0300
Laurynas Biveinis <laurynas.biveinis@gmail.com> wrote:
> The patch is correct in general. 

You didn't say "Ok with changes", so attached here is the patch to trunk 172161

############ gcc/ChangeLog entry ##########
2011-04-08  Basile Starynkevitch  <basile@starynkevitch.net>
	* gengtype.c (write_typed_alloc_def): New argument f. Use it instead
	  of header_file.
          (write_typed_struct_alloc_def, write_typed_typedef_alloc_def)
          (write_typed_alloc_defns): Likewise.
          (main): Calls write_typed_alloc_defns with output_header.
########### end of gcc/ChangeLog entry ##########

Ok for trunk?

By the way, this patch is enough for the MELT plugin (which probably 
is the only plugin needing GTY). But if an hypothetical plugin is made 
of several source files plugin1.c plugin2.c and a common header plugin.h
and if GTY-ed allocation appears in several files plugin1.c plugin2.c,
gengtype should perhaps better generate a header file (containing the 
generated ggc_alloc_* macros) and a body file (contains the marking 
routines).  This could be future work.

Cheers
-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

[-- Attachment #2: gengtype-alloc-r172161.diff --]
[-- Type: text/x-diff, Size: 7782 bytes --]

Index: gcc/gengtype.c
===================================================================
--- gcc/gengtype.c	(revision 172161)
+++ gcc/gengtype.c	(working copy)
@@ -4189,104 +4189,112 @@ enum alloc_quantity
 enum alloc_zone
 { any_zone, specific_zone };
 
-/* Writes one typed allocator definition for type identifier TYPE_NAME with
-   optional type specifier TYPE_SPECIFIER.  The allocator name will contain
-   ALLOCATOR_TYPE.  If VARIABLE_SIZE is true, the allocator will have an extra
-   parameter specifying number of bytes to allocate.  If QUANTITY is set to
-   VECTOR, a vector allocator will be output, if ZONE is set to SPECIFIC_ZONE,
+/* Writes one typed allocator definition into output F for type
+   identifier TYPE_NAME with optional type specifier TYPE_SPECIFIER.
+   The allocator name will contain ALLOCATOR_TYPE.  If VARIABLE_SIZE
+   is true, the allocator will have an extra parameter specifying
+   number of bytes to allocate.  If QUANTITY is set to VECTOR, a
+   vector allocator will be output, if ZONE is set to SPECIFIC_ZONE,
    the allocator will be zone-specific.  */
 
 static void
-write_typed_alloc_def (bool variable_size, const char *type_specifier,
+write_typed_alloc_def (outf_p f, 
+                       bool variable_size, const char *type_specifier,
 		       const char *type_name, const char *allocator_type,
 		       enum alloc_quantity quantity, enum alloc_zone zone)
 {
   bool two_args = variable_size && (quantity == vector);
   bool third_arg = ((zone == specific_zone)
 		    && (variable_size || (quantity == vector)));
-
-  oprintf (header_file, "#define ggc_alloc_%s%s", allocator_type, type_name);
-  oprintf (header_file, "(%s%s%s%s%s) ",
+  gcc_assert (f != NULL);
+  oprintf (f, "#define ggc_alloc_%s%s", allocator_type, type_name);
+  oprintf (f, "(%s%s%s%s%s) ",
 	   (variable_size ? "SIZE" : ""),
 	   (two_args ? ", " : ""),
 	   (quantity == vector) ? "n" : "",
 	   (third_arg ? ", " : ""), (zone == specific_zone) ? "z" : "");
-  oprintf (header_file, "((%s%s *)", type_specifier, type_name);
-  oprintf (header_file, "(ggc_internal_%salloc_stat (", allocator_type);
+  oprintf (f, "((%s%s *)", type_specifier, type_name);
+  oprintf (f, "(ggc_internal_%salloc_stat (", allocator_type);
   if (zone == specific_zone)
-    oprintf (header_file, "z, ");
+    oprintf (f, "z, ");
   if (variable_size)
-    oprintf (header_file, "SIZE");
+    oprintf (f, "SIZE");
   else
-    oprintf (header_file, "sizeof (%s%s)", type_specifier, type_name);
+    oprintf (f, "sizeof (%s%s)", type_specifier, type_name);
   if (quantity == vector)
-    oprintf (header_file, ", n");
-  oprintf (header_file, " MEM_STAT_INFO)))\n");
+    oprintf (f, ", n");
+  oprintf (f, " MEM_STAT_INFO)))\n");
 }
 
-/* Writes a typed allocator definition for a struct or union S.  */
+/* Writes a typed allocator definition into output F for a struct or
+   union S, with a given ALLOCATOR_TYPE and QUANTITY for ZONE.  */
 
 static void
-write_typed_struct_alloc_def (const type_p s, const char *allocator_type,
+write_typed_struct_alloc_def (outf_p f,
+			      const type_p s, const char *allocator_type,
 			      enum alloc_quantity quantity,
 			      enum alloc_zone zone)
 {
-  write_typed_alloc_def (variable_size_p (s), get_type_specifier (s),
+  write_typed_alloc_def (f, variable_size_p (s), get_type_specifier (s),
 			 s->u.s.tag, allocator_type, quantity, zone);
 }
 
-/* Writes a typed allocator definition for a typedef P.  */
+/* Writes a typed allocator definition into output F for a typedef P,
+   with a given ALLOCATOR_TYPE and QUANTITY for ZONE.  */
 
 static void
-write_typed_typedef_alloc_def (const pair_p p, const char *allocator_type,
+write_typed_typedef_alloc_def (outf_p f,
+                               const pair_p p, const char *allocator_type,
 			       enum alloc_quantity quantity,
 			       enum alloc_zone zone)
 {
-  write_typed_alloc_def (variable_size_p (p->type), "", p->name,
+  write_typed_alloc_def (f, variable_size_p (p->type), "", p->name,
 			 allocator_type, quantity, zone);
 }
 
-/* Writes typed allocator definitions for the types in STRUCTURES and
-   TYPEDEFS that are used by GC.  */
+/* Writes typed allocator definitions into output F for the types in
+   STRUCTURES and TYPEDEFS that are used by GC.  */
 
 static void
-write_typed_alloc_defns (const type_p structures, const pair_p typedefs)
+write_typed_alloc_defns (outf_p f,
+                         const type_p structures, const pair_p typedefs)
 {
   type_p s;
   pair_p p;
 
-  oprintf (header_file,
+  gcc_assert (f != NULL);
+  oprintf (f,
 	   "\n/* Allocators for known structs and unions.  */\n\n");
   for (s = structures; s; s = s->next)
     {
       if (!USED_BY_TYPED_GC_P (s))
 	continue;
-      write_typed_struct_alloc_def (s, "", single, any_zone);
-      write_typed_struct_alloc_def (s, "cleared_", single, any_zone);
-      write_typed_struct_alloc_def (s, "vec_", vector, any_zone);
-      write_typed_struct_alloc_def (s, "cleared_vec_", vector, any_zone);
-      write_typed_struct_alloc_def (s, "zone_", single, specific_zone);
-      write_typed_struct_alloc_def (s, "zone_cleared_", single,
+      write_typed_struct_alloc_def (f, s, "", single, any_zone);
+      write_typed_struct_alloc_def (f, s, "cleared_", single, any_zone);
+      write_typed_struct_alloc_def (f, s, "vec_", vector, any_zone);
+      write_typed_struct_alloc_def (f, s, "cleared_vec_", vector, any_zone);
+      write_typed_struct_alloc_def (f, s, "zone_", single, specific_zone);
+      write_typed_struct_alloc_def (f, s, "zone_cleared_", single,
 				    specific_zone);
-      write_typed_struct_alloc_def (s, "zone_vec_", vector, specific_zone);
-      write_typed_struct_alloc_def (s, "zone_cleared_vec_", vector,
+      write_typed_struct_alloc_def (f, s, "zone_vec_", vector, specific_zone);
+      write_typed_struct_alloc_def (f, s, "zone_cleared_vec_", vector,
 				    specific_zone);
     }
 
-  oprintf (header_file, "\n/* Allocators for known typedefs.  */\n");
+  oprintf (f, "\n/* Allocators for known typedefs.  */\n");
   for (p = typedefs; p; p = p->next)
     {
       s = p->type;
       if (!USED_BY_TYPED_GC_P (s) || (strcmp (p->name, s->u.s.tag) == 0))
 	continue;
-      write_typed_typedef_alloc_def (p, "", single, any_zone);
-      write_typed_typedef_alloc_def (p, "cleared_", single, any_zone);
-      write_typed_typedef_alloc_def (p, "vec_", vector, any_zone);
-      write_typed_typedef_alloc_def (p, "cleared_vec_", vector, any_zone);
-      write_typed_typedef_alloc_def (p, "zone_", single, specific_zone);
-      write_typed_typedef_alloc_def (p, "zone_cleared_", single,
+      write_typed_typedef_alloc_def (f, p, "", single, any_zone);
+      write_typed_typedef_alloc_def (f, p, "cleared_", single, any_zone);
+      write_typed_typedef_alloc_def (f, p, "vec_", vector, any_zone);
+      write_typed_typedef_alloc_def (f, p, "cleared_vec_", vector, any_zone);
+      write_typed_typedef_alloc_def (f, p, "zone_", single, specific_zone);
+      write_typed_typedef_alloc_def (f, p, "zone_cleared_", single,
 				     specific_zone);
-      write_typed_typedef_alloc_def (p, "zone_cleared_vec_", vector,
+      write_typed_typedef_alloc_def (f, p, "zone_cleared_vec_", vector,
 				     specific_zone);
     }
 }
@@ -4990,8 +4998,8 @@ main (int argc, char **argv)
   open_base_files ();
 
   write_enum_defn (structures, param_structs);
-  write_typed_alloc_defns (structures, typedefs);
   output_header = plugin_output ? plugin_output : header_file;
+  write_typed_alloc_defns (output_header, structures, typedefs);
   DBGPRINT_COUNT_TYPE ("structures before write_types outputheader",
 		       structures);
   DBGPRINT_COUNT_TYPE ("param_structs before write_types outputheader",

Re: PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode.

[Duncan Sands]

Hi Basile,

> By the way, this patch is enough for the MELT plugin (which probably
> is the only plugin needing GTY).

the dragonegg plugin also uses GTY, but only in a very mild way.

Ciao, Duncan.

Re: PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode.

[Laurynas Biveinis]

> 2011-04-08  Basile Starynkevitch  <basile@starynkevitch.net>
>        * gengtype.c (write_typed_alloc_def): New argument f. Use it instead
>          of header_file.
>          (write_typed_struct_alloc_def, write_typed_typedef_alloc_def)
>          (write_typed_alloc_defns): Likewise.
>          (main): Calls write_typed_alloc_defns with output_header.

The patch is OK. Thanks,

-- 
Laurynas

Re: PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode.

[Basile Starynkevitch]

On Fri, 8 Apr 2011 15:29:48 +0300
Laurynas Biveinis <laurynas.biveinis@gmail.com> wrote:

> > 2011-04-08  Basile Starynkevitch  <basile@starynkevitch.net>
> >        * gengtype.c (write_typed_alloc_def): New argument f. Use it instead
> >          of header_file.
> >          (write_typed_struct_alloc_def, write_typed_typedef_alloc_def)
> >          (write_typed_alloc_defns): Likewise.
> >          (main): Calls write_typed_alloc_defns with output_header.
> 
> The patch is OK. Thanks,

Committed revision 172203 (on trunk).

Actually, the above committed patch is better than nothing, but not
perfect. It happens to generate ggc_alloc macros for things which are
not defined in the plugin (however, this is not a big deal in practice,
since it generates some macros in common with those inside
gtype-desc.h).

I have no clear idea on how to improve it. We could for instance write
the ggc_alloc macros only for struct & typedefs defined in the plugin
files. We then would need a way to know if a file is a plugin file or
not.  What do you think?

We might also want to generate the ggc_alloc macros in a header file
(for the plugin) different of the one containing the marking routines.

Cheers
-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

Re: PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode.

[Laurynas Biveinis]

2011/4/8 Basile Starynkevitch <basile@starynkevitch.net>:
> Actually, the above committed patch is better than nothing, but not
> perfect. It happens to generate ggc_alloc macros for things which are
> not defined in the plugin (however, this is not a big deal in practice,
> since it generates some macros in common with those inside
> gtype-desc.h).

I think it is rather hard to solve it properly, i.e. only output what
is actually used. But, as you said, it is benign, so I think we'll
have to keep ignoring it for now.

--
Laurynas

Re: PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode.

[Basile Starynkevitch]

On Sun, 10 Apr 2011 21:47:05 +0300
Laurynas Biveinis <laurynas.biveinis@gmail.com> wrote:

> 2011/4/8 Basile Starynkevitch <basile@starynkevitch.net>:
> > Actually, the above committed patch is better than nothing, but not
> > perfect. It happens to generate ggc_alloc macros for things which are
> > not defined in the plugin (however, this is not a big deal in practice,
> > since it generates some macros in common with those inside
> > gtype-desc.h).
> 
> I think it is rather hard to solve it properly, i.e. only output what
> is actually used. But, as you said, it is benign, so I think we'll
> have to keep ignoring it for now.

A possible idea (I did not test it, and did not even thought a lot
about it), could be to add a boolean in output_file-s which tell if
they are plugin files or not. If that is doable, generating ggc_alloc
only for plugin related files is just testing that boolean in the line
location of structure descriptors.

Cheers


-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode on for plugin files

[Basile Starynkevitch]

[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]

On Fri, 8 Apr 2011 19:55:51 +0200
Basile Starynkevitch <basile@starynkevitch.net> wrote:

> Committed revision 172203 (on trunk).
> 
> Actually, the above committed patch is better than nothing, but not
> perfect. It happens to generate ggc_alloc macros for things which are
> not defined in the plugin (however, this is not a big deal in practice,
> since it generates some macros in common with those inside
> gtype-desc.h).
> 
> I have no clear idea on how to improve it. We could for instance write
> the ggc_alloc macros only for struct & typedefs defined in the plugin
> files. We then would need a way to know if a file is a plugin file or
> not.  What do you think?

The attached file improves the situation, by flagging input files and
generating ggc_alloc macros only relevant to plugin files.
########## gcc/ChangeLog entry #####
2011-04-11  Basile Starynkevitch  <basile@starynkevitch.net>

        * gengtype.h (struct input_file_st): Add inpisplugin field.
	(type_fileloc): New function.
	* gengtype.c
	(write_typed_struct_alloc_def): Add gcc_assert.
	(write_typed_alloc_defns): Ditto. Don't output for plugin files.
	(write_typed_alloc_defns): Don't output for plugin files.
	(input_file_by_name): Clear inpisplugin field.
	(main): Set inpisplugin field for plugin files.
########

Comments are welcome. Ok for trunk with what changes?

Regards.
-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

[-- Attachment #2: gengtype-allocplugin-r172252.diff --]
[-- Type: text/x-diff, Size: 3585 bytes --]

Index: gcc/gengtype.c
===================================================================
--- gcc/gengtype.c	(revision 172252)
+++ gcc/gengtype.c	(working copy)
@@ -4235,6 +4235,7 @@ write_typed_struct_alloc_def (outf_p f,
 			      enum alloc_quantity quantity,
 			      enum alloc_zone zone)
 {
+  gcc_assert (UNION_OR_STRUCT_P(s));
   write_typed_alloc_def (f, variable_size_p (s), get_type_specifier (s),
                          s->u.s.tag, allocator_type, quantity, zone);
 }
@@ -4269,6 +4270,13 @@ write_typed_alloc_defns (outf_p f,
     {
       if (!USED_BY_TYPED_GC_P (s))
 	continue;
+      gcc_assert (s->kind == TYPE_STRUCT || s->kind == TYPE_UNION
+		  || s->kind == TYPE_LANG_STRUCT);
+      /* In plugin mode onput output ggc_alloc macro definitions
+	 relevant to plugin input files.  */
+      if (nb_plugin_files > 0 
+	  && ((s->u.s.line.file == NULL) || !s->u.s.line.file->inpisplugin))
+	continue;
       write_typed_struct_alloc_def (f, s, "", single, any_zone);
       write_typed_struct_alloc_def (f, s, "cleared_", single, any_zone);
       write_typed_struct_alloc_def (f, s, "vec_", vector, any_zone);
@@ -4287,6 +4295,14 @@ write_typed_alloc_defns (outf_p f,
       s = p->type;
       if (!USED_BY_TYPED_GC_P (s) || (strcmp (p->name, s->u.s.tag) == 0))
 	continue;
+      /* In plugin mode onput output ggc_alloc macro definitions
+	 relevant to plugin input files.  */
+      if (nb_plugin_files > 0) 
+	{
+	  struct fileloc* filoc = type_fileloc(s);
+	  if (!filoc || !filoc->file->inpisplugin)
+	    continue;
+	};
       write_typed_typedef_alloc_def (f, p, "", single, any_zone);
       write_typed_typedef_alloc_def (f, p, "cleared_", single, any_zone);
       write_typed_typedef_alloc_def (f, p, "vec_", vector, any_zone);
@@ -4814,6 +4830,7 @@ input_file_by_name (const char* name)
   f = XCNEWVAR (input_file, sizeof (input_file)+namlen+2);
   f->inpbitmap = 0;
   f->inpoutf = NULL;
+  f->inpisplugin = 0;
   strcpy (f->inpname, name);
   slot = htab_find_slot (input_file_htab, f, INSERT);
   gcc_assert (slot != NULL);
@@ -4945,8 +4962,11 @@ main (int argc, char **argv)
 
       /* Parse our plugin files and augment the state.  */
       for (ix = 0; ix < nb_plugin_files; ix++)
-	parse_file (get_input_file_name (plugin_files[ix]));
-
+	{
+	  input_file* pluginput = plugin_files [ix];
+	  pluginput->inpisplugin = true;
+	  parse_file (get_input_file_name (pluginput));
+	}
       if (hit_error)
 	return 1;
 
Index: gcc/gengtype.h
===================================================================
--- gcc/gengtype.h	(revision 172252)
+++ gcc/gengtype.h	(working copy)
@@ -33,6 +33,7 @@ struct input_file_st
   struct outf* inpoutf;  /* Cached corresponding output file, computed
                             in get_output_file_with_visibility.  */
   lang_bitmap inpbitmap; /* The set of languages using this file.  */
+  bool inpisplugin;      /* Flag set for plugin input files.  */
   char inpname[1];       /* A variable-length array, ended by a null
                             char.  */
 };
@@ -304,6 +305,7 @@ struct type {
   } u;
 };
 
+
 /* The one and only TYPE_STRING.  */
 extern struct type string_type;
 
@@ -328,6 +330,19 @@ extern struct type scalar_char;
 
 
 
+/* Give the file location of a type, if any. */
+static inline struct fileloc* 
+type_fileloc (type_p t)
+{
+  if (!t) 
+    return NULL;
+  if (UNION_OR_STRUCT_P(t))
+    return &t->u.s.line;
+  if  (t->kind == TYPE_PARAM_STRUCT)
+    return &t->u.param_struct.line;
+  return NULL;
+}
+
 /* Structure representing an output file.  */
 struct outf
 {

Re: PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode on for plugin files

[Laurynas Biveinis]

2011/4/11 Basile Starynkevitch <basile@starynkevitch.net>:
> On Fri, 8 Apr 2011 19:55:51 +0200
> Basile Starynkevitch <basile@starynkevitch.net> wrote:
>
>> Committed revision 172203 (on trunk).
>>
>> Actually, the above committed patch is better than nothing, but not
>> perfect. It happens to generate ggc_alloc macros for things which are
>> not defined in the plugin (however, this is not a big deal in practice,
>> since it generates some macros in common with those inside
>> gtype-desc.h).
>>
>> I have no clear idea on how to improve it. We could for instance write
>> the ggc_alloc macros only for struct & typedefs defined in the plugin
>> files. We then would need a way to know if a file is a plugin file or
>> not.  What do you think?
>
> The attached file improves the situation, by flagging input files and
> generating ggc_alloc macros only relevant to plugin files.

What about include files in plugins? Are these marked as input files?

-- 
Laurynas

Re: PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode on for plugin files

[Basile Starynkevitch]

On Fri, 15 Apr 2011 06:38:46 +0300
Laurynas Biveinis <laurynas.biveinis@gmail.com> wrote:

> 2011/4/11 Basile Starynkevitch <basile@starynkevitch.net>:
> >
> > The attached file improves the situation, by flagging input files and
> > generating ggc_alloc macros only relevant to plugin files.
> 
> What about include files in plugins? Are these marked as input files?

The plugin input files are those passed on the command line.

You can pass gengtype -P gt-plugin.h yourplugin.h plugin1.c plugin2.c

Regards


-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

Re: PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode on for plugin files

[Laurynas Biveinis]

On 04/11/2011 04:35 AM, Basile Starynkevitch wrote:
> 2011-04-11  Basile Starynkevitch  <basile@starynkevitch.net>
>
>         * gengtype.h (struct input_file_st): Add inpisplugin field.
> 	(type_fileloc): New function.
> 	* gengtype.c
> 	(write_typed_struct_alloc_def): Add gcc_assert.
> 	(write_typed_alloc_defns): Ditto. Don't output for plugin files.
> 	(write_typed_alloc_defns): Don't output for plugin files.
> 	(input_file_by_name): Clear inpisplugin field.
> 	(main): Set inpisplugin field for plugin files.

Did you test this patch that it bootstraps and that a GTY-using plugin
gets the right set of definitions in the output?

> @@ -4814,6 +4830,7 @@ input_file_by_name (const char* name)
>    f = XCNEWVAR (input_file, sizeof (input_file)+namlen+2);
>    f->inpbitmap = 0;
>    f->inpoutf = NULL;
> +  f->inpisplugin = 0;

= false;

> @@ -304,6 +305,7 @@ struct type {
>    } u;
>  };
>
> +
>  /* The one and only TYPE_STRING.  */
>  extern struct type string_type;
>

Please drop this.

OK with these changes and confirmation that the patch was tested.

Thanks,
Laurynas

Re: PATCH [trunk] gengtype should generate ggc_alloc macros in plugin mode on for plugin files

[Basile Starynkevitch]

On Tue, 19 Apr 2011 06:42:58 +0300
Laurynas Biveinis <laurynas.biveinis@gmail.com> wrote:
> 
> Did you test this patch that it bootstraps and that a GTY-using plugin
> gets the right set of definitions in the output?

I checked both (using MELT as the plugin to test it). 
I very slightly & trivially improved the patch by updating the copyright year of gengtype.h and by 
replacing in the patch of write_typed_struct_alloc_def
+      gcc_assert (s->kind == TYPE_STRUCT || s->kind == TYPE_UNION
+		  || s->kind == TYPE_LANG_STRUCT);
with the more readable & shorter equivalent
       gcc_assert (UNION_OR_STRUCT_P (s));



> OK with these changes and confirmation that the patch was tested.

Committed revision 172705.

Thanks
-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***