diff mbox

PATCH trunk: gengtype honoring mark_hook-s inside struct inside union-s

Message ID 20121003110244.GA13256@hector.lesours
State New
Headers show

Commit Message

Basile Starynkevitch Oct. 3, 2012, 11:02 a.m. UTC 
On Wed, Oct 03, 2012 at 12:21:02PM +0300, Laurynas Biveinis wrote:
> Hello Basile -
> 
> > 2012-10-02  Basile Starynkevitch  <basile@starynkevitch.net>
> >
> >         * gengtype.c (walk_type): Emit mark_hook when inside a
> >           struct of a union member.
> 
> Can you send me off-list the gengtype output before and after the fix?

I messed something, the example I did send was wrong. Let's start all over again.

Consider the following file 
(I named this example on purpose differently than in the previous message)
####################
/* file basilemarkh.h */

   struct  GTY ((mark_hook("mymarker"))) mytest_st {
     int myflag;
     tree mytree;
     gimple mygimple;
   };

   #define MYUTAG 1
   union GTY ((desc ("%0.u_int"))) myutest_un
   {
     int GTY ((skip)) u_int;
     struct mytest_st GTY ((tag ("MYUTAG"))) u_mytest;
   };

   static GTY (()) union myutest_un *myutestptr;

   static inline void mymarker (struct mytest_st *s)
   {
     s->myflag = 1;
   }

   /* eof basilemarkh.h */
###################

With the gengtype from 4.7 in Debian/Sid/AMD64 (a 4.7.2 I believe) 
/usr/lib/gcc/x86_64-linux-gnu/4.7/gengtype  -D -v \
   -r /usr/lib/gcc/x86_64-linux-gnu/4.7/gtype.state \
   -P _g-4.7-basilemarkh.h basilemarkh.h

#### from generated  _g-4.7-basilemarkh.h 
/* functions code */

void
gt_ggc_mx_myutest_un (void *x_p)
{
  union myutest_un * const x = (union myutest_un *)x_p;
  if (ggc_test_and_set_mark (x))
    {
      switch ((*x).u_int)
        {
        case MYUTAG: //// ###### no mark hook here, should have a call to mymarker
          gt_ggc_m_9tree_node ((*x).u_mytest.mytree);
          gt_ggc_m_18gimple_statement_d ((*x).u_mytest.mygimple);
          break;
        default:
          break;
        }
    }
}
### end of excerpt from generated  _g-4.7-basilemarkh.h 

With the gengtype from unpatched trunk svn rev 192031 the emitted gt_ggc_mx_myutest_un is exactly the same.
Of course the //// ### comment above has been added manually by me Basile.

So I applied and I am proposing the following patch to gcc trunk 192031
(Laurynas, I did take your remarks into account)
##### patch to trunk

##### end of patch

with the following ChangeLog entry
##### gcc/ChangeLog entry
2012-10-03  Basile Starynkevitch  <basile@starynkevitch.net>

	* gengtype.c (walk_type): Emit mark_hook when inside a
          struct of a union member.

##### end of gcc/ChangeLog entry

If you apply my proposed patch the emitted function is
### from generated _gtrunk-basilemarkh.h with my patch to gengtype.c
/* functions code */

void
gt_ggc_mx_myutest_un (void *x_p)
{
  union myutest_un * const x = (union myutest_un *)x_p;
  if (ggc_test_and_set_mark (x))
    {
      switch ((*x).u_int)
        {
        case MYUTAG:
          mymarker (&(*x).u_mytest));
          gt_ggc_m_9tree_node ((*x).u_mytest.mytree);
          gt_ggc_m_18gimple_statement_d ((*x).u_mytest.mygimple);
          break;
        default:
          break;
        }
    }
}
#### end of excerpt from generated  _gtrunk-basilemarkh.h


If you just use the gengtype from trunk 192031 without my patch
you get the same code as from gcc-4.7's gengtype:
#### from generated _g192031-basilemarkh.h 
void
gt_ggc_mx_myutest_un (void *x_p)
{
  union myutest_un * const x = (union myutest_un *)x_p;
  if (ggc_test_and_set_mark (x))
    {
      switch ((*x).u_int)
        {
        case MYUTAG:
          gt_ggc_m_9tree_node ((*x).u_mytest.mytree);
          gt_ggc_m_18gimple_statement_d ((*x).u_mytest.mygimple);
          break;
        default:
          break;
        }
    }
}
### end of excerpt from generated _g192031-basilemarkh.h


Is my patch here ok for trunk?

Regards.

Comments

Basile Starynkevitch Oct. 4, 2012, 8:06 a.m. UTC | #1 
On Wed, Oct 03, 2012 at 01:02:44PM +0200, Basile Starynkevitch wrote:
> On Wed, Oct 03, 2012 at 12:21:02PM +0300, Laurynas Biveinis wrote:
> > Hello Basile -
> > 
> > > 2012-10-02  Basile Starynkevitch  <basile@starynkevitch.net>
> > >
> > >         * gengtype.c (walk_type): Emit mark_hook when inside a
> > >           struct of a union member.
> > 
> > Can you send me off-list the gengtype output before and after the fix?
> 
> I messed something, the example I did send was wrong. Let's start all over again.
> 
> Consider the following file 
[...]

This is PR54809 on our bugzilla.

Thanks.
Laurynas Biveinis Oct. 4, 2012, 3:51 p.m. UTC | #2 
> 2012-10-03  Basile Starynkevitch  <basile@starynkevitch.net>
>
>         * gengtype.c (walk_type): Emit mark_hook when inside a
>           struct of a union member.

This is OK.

Thanks,
Basile Starynkevitch Oct. 4, 2012, 5:24 p.m. UTC | #3 
On Thu, Oct 04, 2012 at 06:51:35PM +0300, Laurynas Biveinis wrote:
> > 2012-10-03  Basile Starynkevitch  <basile@starynkevitch.net>
> >
> >         * gengtype.c (walk_type): Emit mark_hook when inside a
> >           struct of a union member.
> 
> This is OK.

thanks, Committed revision 192092 to trunk.


I believe this patch should be backported into GCC 4.7 and 4.6

Regards.
Richard Biener Oct. 4, 2012, 5:26 p.m. UTC | #4 
On Thu, Oct 4, 2012 at 7:24 PM, Basile Starynkevitch
<basile@starynkevitch.net> wrote:
> On Thu, Oct 04, 2012 at 06:51:35PM +0300, Laurynas Biveinis wrote:
>> > 2012-10-03  Basile Starynkevitch  <basile@starynkevitch.net>
>> >
>> >         * gengtype.c (walk_type): Emit mark_hook when inside a
>> >           struct of a union member.
>>
>> This is OK.
>
> thanks, Committed revision 192092 to trunk.
>
>
> I believe this patch should be backported into GCC 4.7 and 4.6

I see no reason for this unless it is a regression.

Richard.

> 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 mines, sont seulement les miennes} ***
Basile Starynkevitch Oct. 4, 2012, 5:33 p.m. UTC | #5 
On Thu, Oct 04, 2012 at 07:26:23PM +0200, Richard Guenther wrote:
> On Thu, Oct 4, 2012 at 7:24 PM, Basile Starynkevitch
> <basile@starynkevitch.net> wrote:
> > On Thu, Oct 04, 2012 at 06:51:35PM +0300, Laurynas Biveinis wrote:
> >> > 2012-10-03  Basile Starynkevitch  <basile@starynkevitch.net>
> >> >
> >> >         * gengtype.c (walk_type): Emit mark_hook when inside a
> >> >           struct of a union member.
> >>
> >> This is OK.
> >
> > thanks, Committed revision 192092 to trunk.
> >
> >
> > I believe this patch should be backported into GCC 4.7 and 4.6
> 
> I see no reason for this unless it is a regression.

If GCC 4.7 will have future micro releases, (like an hypothetical 4.7.3) they will have the same bug.
What is the procedure to get this bug fixed in 4.7.3?

(and there are plugins for 4.7 affected by this bug, http://gcc-melt.org/ for example)

Regards.
diff mbox

Patch

Index: gcc/gengtype.c
===================================================================
--- gcc/gengtype.c	(revision 192031)
+++ gcc/gengtype.c	(working copy)
@@ -2810,6 +2810,7 @@  walk_type (type_p t, struct walk_type_data *d)
 	const char *oldval = d->val;
 	const char *oldprevval1 = d->prev_val[1];
 	const char *oldprevval2 = d->prev_val[2];
+	const char *struct_mark_hook = NULL;
 	const int union_p = t->kind == TYPE_UNION;
 	int seen_default_p = 0;
 	options_p o;
@@ -2833,7 +2834,14 @@  walk_type (type_p t, struct walk_type_data *d)
 	  if (!desc && strcmp (o->name, "desc") == 0
 	      && o->kind == OPTION_STRING)
 	    desc = o->info.string;
+	  else if (!struct_mark_hook && strcmp (o->name, "mark_hook") == 0
+		   && o->kind == OPTION_STRING)
+	    struct_mark_hook = o->info.string;
 
+	if (struct_mark_hook) 
+	    oprintf (d->of, "%*s%s (&%s));\n",
+		     d->indent, "", struct_mark_hook, oldval);
+	  
 	d->prev_val[2] = oldval;
 	d->prev_val[1] = oldprevval2;
 	if (union_p)