[1/2] Convert symtab, cgraph and varpool nodes into a real class hierarchy

This patch is the handwritten part of the conversion of these types
to C++; it requires the followup patch, which is autogenerated.

It converts:
  struct GTY(()) symtab_node_base
to:
  class GTY((user)) symtab_node_base

and converts:
  struct GTY(()) cgraph_node
to:
  struct GTY((user)) cgraph_node : public symtab_node_base

and:
  struct GTY(()) varpool_node
to:
  class GTY((user)) varpool_node : public symtab_node_base

dropping the symtab_node_def union.

Since gengtype is unable to cope with inheritance, we have to mark the
types with GTY((user)), and handcode the gty field-visiting functions.
Given the simple hierarchy, we don't need virtual functions for this.

Unfortunately doing so runs into various bugs in gengtype's handling
of GTY((user)), so the patch also includes workarounds for these bugs.

gengtype walks the graph of the *types* in the code, and produces
functions in gtype-desc.[ch] for all types that are reachable from a
GTY root.

However, it ignores the contents of GTY((user)) types when walking
this graph.

Hence if you have a subgraph of types that are only reachable
via fields in GTY((user)) types, gengtype won't generate helper code
for those types.

Ideally there would be some way to mark a GTY((user)) type to say
which types it references, to avoid having to mark these types as
GTY((user)).

For now, work around this issue by providing explicit roots of the
missing types, of dummy variables (see the bottom of cgraph.c)

gcc/

	* cgraph.c (gt_ggc_mx): New.
	(gt_pch_nx (symtab_node_base *)): New.
	(gt_pch_nx (symtab_node_base *, gt_pointer_operator, void *)): New.
	(dummy_call_site_hash): New.
	(dummy_ipa_ref_list): New.
	(dummy_cgraph_clone_info): New.
	(dummy_ipa_replace_map_pointer): New.
	(dummy_varpool_node_ptr): New.

	* cgraph.h (symtab_node_base): Convert to a class;
	add GTY((user)).
	(gt_ggc_mx): New.
	(gt_pch_nx (symtab_node_base *p)): New.
	(gt_pch_nx (symtab_node_base *p, gt_pointer_operator op,
	void *cookie)): New.
	(cgraph_node): Inherit from symtab_node; convert to GTY((user)).
	(varpool_node): Likewise.
	(symtab_node_def): Remove.
	(is_a_helper <cgraph_node>::test (symtab_node_def *)): Convert to...
	(is_a_helper <cgraph_node>::test (symtab_node_base *)): ...this.
	(is_a_helper <varpool_node>::test (symtab_node_def *)): Convert to...
	(is_a_helper <varpool_node>::test (symtab_node_base *)): ...this.

	* ipa-ref.h (symtab_node_def): Drop.
	(symtab_node): Change underlying type from symtab_node_def to
	symtab_node_base.
	(const_symtab_node): Likwise.

	* is-a.h: Update examples in comment.

	* symtab.c (symtab_hash): Change symtab_node_def to symtab_node_base.
	(assembler_name_hash): Likewise.
---
 gcc/cgraph.c  | 218 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gcc/cgraph.h  |  48 ++++++-------
 gcc/ipa-ref.h |   6 +-
 gcc/is-a.h    |   6 +-
 gcc/symtab.c  |   4 +-
 5 files changed, 247 insertions(+), 35 deletions(-)

> +/* GTY((user)) hooks for symtab_node_base (and its subclasses).
> +   We could use virtual functions for this, but given the presence of the
> +   "type" field and the trivial size of the class hierarchy, switches are
> +   perhaps simpler and faster.  */

Generally I am not really happy about the hand marking - why can't GTY just handle
it by itself?  Do we have some eisting exmaple of this?

GTY was in a way of getting proper class hiearchy for quite a while and this is
probably less ugly than C-syntax-classes we have now.  But it would be nice to
have some longer term plan what to do here.  Obviously this is going to make
any changes to GGC implementation close to imposible since all the
implementation details are exposed now.
> +/* Workarounds for deficiencies in gengtype's handling of GTY((user)).
> +
> +   gengtype walks the graph of the *types* in the code, and produces
> +   functions in gtype-desc.[ch] for all types that are reachable from a
> +   GTY root.
> +
> +   However, it ignores the contents of GTY((user)) types when walking
> +   this graph.
> +
> +   Hence if you have a subgraph of types that are only reachable
> +   via fields in GTY((user)) types, gengtype won't generate helper code
> +   for those types.
> +
> +   Ideally there would be some way to mark a GTY((user)) type to say
> +   which types it references, to avoid having to mark these types as
> +   GTY((user)).
> +
> +   For now, work around this issue by providing explicit roots of the
> +   missing types, of dummy variables.  */
> +
> +/* Force gengtype into providing GTY hooks for the type of this field:
> +     htab_t GTY((param_is (struct cgraph_edge))) call_site_hash;
> +   within cgraph_node.  */
> +static GTY(()) htab_t GTY((param_is (struct cgraph_edge))) dummy_call_site_hash;

I think call site hash can safely go off GGC memory.  All statements are linked
from CFG.
> +
> +/* Similarly for this field of symtab_node_base:
> +     struct ipa_ref_list ref_list;
> + */
> +static GTY((deletable)) struct ipa_ref_list *dummy_ipa_ref_list;
> +
> +/* Similarly for this field of cgraph_node:
> +     struct cgraph_clone_info clone;
> +*/
> +static GTY((deletable)) struct cgraph_clone_info *dummy_cgraph_clone_info;
> +
> +/* Non-existent pointer, to trick gengtype into (correctly) realizing that
> +   ipa_replace_map is indeed used in the code, and thus should have ggc_alloc_*
> +   routines.  */
> +static GTY((deletable)) struct ipa_replace_map *dummy_ipa_replace_map_pointer;
> +
> +/* Non-existent pointer, to trick gengtype into (correctly) realizing that
> +   varpool_node is indeed used in the code, and thus should have ggc_alloc_*
> +   routines.  */
> +static GTY((deletable)) varpool_node *dummy_varpool_node_ptr;
> +

I would really like some GGC/middle-end or global maintainer to comment on these
changes.

The rest of changes seems fine to me with the comments from first email ;)

Honza

On Tue, Aug 20, 2013 at 11:06 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> +/* GTY((user)) hooks for symtab_node_base (and its subclasses).
>> +   We could use virtual functions for this, but given the presence of the
>> +   "type" field and the trivial size of the class hierarchy, switches are
>> +   perhaps simpler and faster.  */
>
> Generally I am not really happy about the hand marking - why can't GTY just handle
> it by itself?  Do we have some eisting exmaple of this?
>
> GTY was in a way of getting proper class hiearchy for quite a while and this is
> probably less ugly than C-syntax-classes we have now.  But it would be nice to
> have some longer term plan what to do here.  Obviously this is going to make
> any changes to GGC implementation close to imposible since all the
> implementation details are exposed now.

As far as I understand, the intent is to move to user markers, see:
http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00630.html

On the up-side: This would allow explicitly-tracked objects to be
moved back out of GGC-space. Can't wait to put the CFG object back in
pools! :-) And I guess symtab objects are also explicitly tracked and
therefore candidates for a more cache-friendly allocation strategy...

Ciao!
Steven

> On Tue, Aug 20, 2013 at 11:06 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> >> +/* GTY((user)) hooks for symtab_node_base (and its subclasses).
> >> +   We could use virtual functions for this, but given the presence of the
> >> +   "type" field and the trivial size of the class hierarchy, switches are
> >> +   perhaps simpler and faster.  */
> >
> > Generally I am not really happy about the hand marking - why can't GTY just handle
> > it by itself?  Do we have some eisting exmaple of this?
> >
> > GTY was in a way of getting proper class hiearchy for quite a while and this is
> > probably less ugly than C-syntax-classes we have now.  But it would be nice to
> > have some longer term plan what to do here.  Obviously this is going to make
> > any changes to GGC implementation close to imposible since all the
> > implementation details are exposed now.
> 
> As far as I understand, the intent is to move to user markers, see:
> http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00630.html

OK, I have nothing against explicit user markers. It however seem to need a bit
of abstraction - just writting by hand whatever gengtype produces seems ugly.
> 
> On the up-side: This would allow explicitly-tracked objects to be
> moved back out of GGC-space. Can't wait to put the CFG object back in
> pools! :-) And I guess symtab objects are also explicitly tracked and
> therefore candidates for a more cache-friendly allocation strategy...

Indeed, nothing in symbol table needs to be garbage collected and everything is
freed explicitely.  We only have it in GGC because of PCH and the fact that we
hook trees out of that and we do not want them to be garbage collected out.

Honza
> 
> Ciao!
> Steven

> > On Tue, Aug 20, 2013 at 11:06 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> > >> +/* GTY((user)) hooks for symtab_node_base (and its subclasses).
> > >> +   We could use virtual functions for this, but given the presence of the
> > >> +   "type" field and the trivial size of the class hierarchy, switches are
> > >> +   perhaps simpler and faster.  */
> > >
> > > Generally I am not really happy about the hand marking - why can't GTY just handle
> > > it by itself?  Do we have some eisting exmaple of this?
> > >
> > > GTY was in a way of getting proper class hiearchy for quite a while and this is
> > > probably less ugly than C-syntax-classes we have now.  But it would be nice to
> > > have some longer term plan what to do here.  Obviously this is going to make
> > > any changes to GGC implementation close to imposible since all the
> > > implementation details are exposed now.
> > 
> > As far as I understand, the intent is to move to user markers, see:
> > http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00630.html
> 
> OK, I have nothing against explicit user markers. It however seem to need a bit
> of abstraction - just writting by hand whatever gengtype produces seems ugly.

However reading through the thread, I guess we do have agreement on going with user
markers, so I do not think it is a reason to hold the patch.

Honza

On Fri, Aug 16, 2013 at 2:57 AM, David Malcolm <dmalcolm@redhat.com> wrote:
> This patch is the handwritten part of the conversion of these types
> to C++; it requires the followup patch, which is autogenerated.
>
> It converts:
>   struct GTY(()) symtab_node_base
> to:
>   class GTY((user)) symtab_node_base
>
> and converts:
>   struct GTY(()) cgraph_node
> to:
>   struct GTY((user)) cgraph_node : public symtab_node_base
>
> and:
>   struct GTY(()) varpool_node
> to:
>   class GTY((user)) varpool_node : public symtab_node_base
>
> dropping the symtab_node_def union.
>
> Since gengtype is unable to cope with inheritance, we have to mark the
> types with GTY((user)), and handcode the gty field-visiting functions.
> Given the simple hierarchy, we don't need virtual functions for this.
>
> Unfortunately doing so runs into various bugs in gengtype's handling
> of GTY((user)), so the patch also includes workarounds for these bugs.
>
> gengtype walks the graph of the *types* in the code, and produces
> functions in gtype-desc.[ch] for all types that are reachable from a
> GTY root.
>
> However, it ignores the contents of GTY((user)) types when walking
> this graph.
>
> Hence if you have a subgraph of types that are only reachable
> via fields in GTY((user)) types, gengtype won't generate helper code
> for those types.
>
> Ideally there would be some way to mark a GTY((user)) type to say
> which types it references, to avoid having to mark these types as
> GTY((user)).
>
> For now, work around this issue by providing explicit roots of the
> missing types, of dummy variables (see the bottom of cgraph.c)
>
> gcc/
>
>         * cgraph.c (gt_ggc_mx): New.
>         (gt_pch_nx (symtab_node_base *)): New.
>         (gt_pch_nx (symtab_node_base *, gt_pointer_operator, void *)): New.
>         (dummy_call_site_hash): New.
>         (dummy_ipa_ref_list): New.
>         (dummy_cgraph_clone_info): New.
>         (dummy_ipa_replace_map_pointer): New.
>         (dummy_varpool_node_ptr): New.
>
>         * cgraph.h (symtab_node_base): Convert to a class;
>         add GTY((user)).
>         (gt_ggc_mx): New.
>         (gt_pch_nx (symtab_node_base *p)): New.
>         (gt_pch_nx (symtab_node_base *p, gt_pointer_operator op,
>         void *cookie)): New.
>         (cgraph_node): Inherit from symtab_node; convert to GTY((user)).
>         (varpool_node): Likewise.
>         (symtab_node_def): Remove.
>         (is_a_helper <cgraph_node>::test (symtab_node_def *)): Convert to...
>         (is_a_helper <cgraph_node>::test (symtab_node_base *)): ...this.
>         (is_a_helper <varpool_node>::test (symtab_node_def *)): Convert to...
>         (is_a_helper <varpool_node>::test (symtab_node_base *)): ...this.
>
>         * ipa-ref.h (symtab_node_def): Drop.
>         (symtab_node): Change underlying type from symtab_node_def to
>         symtab_node_base.
>         (const_symtab_node): Likwise.
>
>         * is-a.h: Update examples in comment.
>
>         * symtab.c (symtab_hash): Change symtab_node_def to symtab_node_base.
>         (assembler_name_hash): Likewise.
> ---
>  gcc/cgraph.c  | 218 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  gcc/cgraph.h  |  48 ++++++-------
>  gcc/ipa-ref.h |   6 +-
>  gcc/is-a.h    |   6 +-
>  gcc/symtab.c  |   4 +-
>  5 files changed, 247 insertions(+), 35 deletions(-)
>
> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index 50d13ab..5cb6a31 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -3035,4 +3035,222 @@ cgraph_get_body (struct cgraph_node *node)
>    return true;
>  }
>
> +/* GTY((user)) hooks for symtab_node_base (and its subclasses).
> +   We could use virtual functions for this, but given the presence of the
> +   "type" field and the trivial size of the class hierarchy, switches are
> +   perhaps simpler and faster.  */
> +
> +void gt_ggc_mx (symtab_node_base *x)
> +{
> +  /* Hand-written equivalent of the chain_next/chain_prev machinery, to
> +     avoid deep call-stacks.
> +
> +     Locate the neighbors of x (within the linked-list) that haven't been
> +     marked yet, so that x through xlimit give a range suitable for marking.
> +     Note that x (on entry) itself has already been marked by the
> +     gtype-desc.c code, so we first try its successor.
> +  */
> +  symtab_node_base * xlimit = x ? x->next : NULL;
> +  while (ggc_test_and_set_mark (xlimit))
> +   xlimit = xlimit->next;
> +  if (x != xlimit)
> +    for (;;)
> +      {
> +        symtab_node_base * const xprev = x->previous;
> +        if (xprev == NULL) break;
> +        x = xprev;
> +        (void) ggc_test_and_set_mark (xprev);
> +      }
> +  while (x != xlimit)
> +    {
> +      /* Code common to all symtab nodes. */
> +      gt_ggc_m_9tree_node (x->decl);
> +      gt_ggc_mx_symtab_node_base (x->next);
> +      gt_ggc_mx_symtab_node_base (x->previous);
> +      gt_ggc_mx_symtab_node_base (x->next_sharing_asm_name);
> +      gt_ggc_mx_symtab_node_base (x->previous_sharing_asm_name);
> +      gt_ggc_mx_symtab_node_base (x->same_comdat_group);
> +      gt_ggc_m_20vec_ipa_ref_t_va_gc_ (x->ref_list.references);
> +      gt_ggc_m_9tree_node (x->alias_target);
> +      gt_ggc_m_18lto_file_decl_data (x->lto_file_data);

Don't we have template specializations so we just can do

         gt_ggc_mark (x->decl);
         gt_ggc_mark (x->next);
...
etc?

Also all of the symbol table is reachable from the global symbol_table
dynamic array which is a GC root.  So instead of walking ->next/previous
and edges you should have a custom marker for the symbol_table global
which does more efficient marking with loops.

Richard.

> +      /* Extra code, per subclass. */
> +      switch (x->type)
> +        {
> +        case SYMTAB_FUNCTION:
> +          {
> +            cgraph_node *cgn = static_cast <cgraph_node *> (x);
> +            gt_ggc_m_11cgraph_edge (cgn->callees);
> +            gt_ggc_m_11cgraph_edge (cgn->callers);
> +            gt_ggc_m_11cgraph_edge (cgn->indirect_calls);
> +            gt_ggc_m_11cgraph_node (cgn->origin);
> +            gt_ggc_m_11cgraph_node (cgn->nested);
> +            gt_ggc_m_11cgraph_node (cgn->next_nested);
> +            gt_ggc_m_11cgraph_node (cgn->next_sibling_clone);
> +            gt_ggc_m_11cgraph_node (cgn->prev_sibling_clone);
> +            gt_ggc_m_11cgraph_node (cgn->clones);
> +            gt_ggc_m_11cgraph_node (cgn->clone_of);
> +            gt_ggc_m_P11cgraph_edge4htab (cgn->call_site_hash);
> +            gt_ggc_m_9tree_node (cgn->former_clone_of);
> +            gt_ggc_m_11cgraph_node (cgn->global.inlined_to);
> +            gt_ggc_m_28vec_ipa_replace_map_p_va_gc_ (cgn->clone.tree_map);
> +            gt_ggc_m_15bitmap_head_def (cgn->clone.args_to_skip);
> +            gt_ggc_m_15bitmap_head_def (cgn->clone.combined_args_to_skip);
> +            gt_ggc_m_9tree_node (cgn->thunk.alias);
> +          }
> +          break;
> +        default:
> +          break;
> +        }
> +      x = x->next;
> +    }
> +}
> +
> +void gt_pch_nx (symtab_node_base *x)
> +{
> +  symtab_node_base * xlimit = x ? x->next : NULL;
> +  while (gt_pch_note_object (xlimit, xlimit, gt_pch_p_16symtab_node_base))
> +   xlimit = xlimit->next;
> +  if (x != xlimit)
> +    for (;;)
> +      {
> +        symtab_node_base * const xprev = x->previous;
> +        if (xprev == NULL) break;
> +        x = xprev;
> +        (void) gt_pch_note_object (xprev, xprev,
> +                                   gt_pch_p_16symtab_node_base);
> +      }
> +  while (x != xlimit)
> +    {
> +      /* Code common to all symtab nodes. */
> +      gt_pch_n_9tree_node (x->decl);
> +      gt_pch_nx_symtab_node_base (x->next);
> +      gt_pch_nx_symtab_node_base (x->previous);
> +      gt_pch_nx_symtab_node_base (x->next_sharing_asm_name);
> +      gt_pch_nx_symtab_node_base (x->previous_sharing_asm_name);
> +      gt_pch_nx_symtab_node_base (x->same_comdat_group);
> +      gt_pch_n_20vec_ipa_ref_t_va_gc_ (x->ref_list.references);
> +      gt_pch_n_9tree_node (x->alias_target);
> +      gt_pch_n_18lto_file_decl_data (x->lto_file_data);
> +
> +      /* Extra code, per subclass. */
> +      switch (x->type)
> +        {
> +        case SYMTAB_FUNCTION:
> +          {
> +            cgraph_node *cgn = static_cast <cgraph_node *> (x);
> +            gt_pch_n_11cgraph_edge (cgn->callees);
> +            gt_pch_n_11cgraph_edge (cgn->callers);
> +            gt_pch_n_11cgraph_edge (cgn->indirect_calls);
> +            gt_pch_n_11cgraph_node (cgn->origin);
> +            gt_pch_n_11cgraph_node (cgn->nested);
> +            gt_pch_n_11cgraph_node (cgn->next_nested);
> +            gt_pch_n_11cgraph_node (cgn->next_sibling_clone);
> +            gt_pch_n_11cgraph_node (cgn->prev_sibling_clone);
> +            gt_pch_n_11cgraph_node (cgn->clones);
> +            gt_pch_n_11cgraph_node (cgn->clone_of);
> +            gt_pch_n_P11cgraph_edge4htab (cgn->call_site_hash);
> +            gt_pch_n_9tree_node (cgn->former_clone_of);
> +            gt_pch_n_11cgraph_node (cgn->global.inlined_to);
> +            gt_pch_n_28vec_ipa_replace_map_p_va_gc_ (cgn->clone.tree_map);
> +            gt_pch_n_15bitmap_head_def (cgn->clone.args_to_skip);
> +            gt_pch_n_15bitmap_head_def (cgn->clone.combined_args_to_skip);
> +            gt_pch_n_9tree_node (cgn->thunk.alias);
> +          }
> +          break;
> +        default:
> +          break;
> +        }
> +      x = x->next;
> +    }
> +}
> +
> +void gt_pch_nx (symtab_node_base *p, gt_pointer_operator op, void *cookie)
> +{
> +  /* Code common to all symtab nodes. */
> +  op (&(p->decl), cookie);
> +  op (&(p->next), cookie);
> +  op (&(p->previous), cookie);
> +  op (&(p->next_sharing_asm_name), cookie);
> +  op (&(p->previous_sharing_asm_name), cookie);
> +  op (&(p->same_comdat_group), cookie);
> +  op (&(p->ref_list.references), cookie);
> +  op (&(p->alias_target), cookie);
> +  op (&(p->lto_file_data), cookie);
> +
> +  /* Extra code, per subclass. */
> +  switch (p->type)
> +    {
> +    case SYMTAB_FUNCTION:
> +      {
> +        cgraph_node *cgn = static_cast <cgraph_node *> (p);
> +        op (&(cgn->callees), cookie);
> +        op (&(cgn->callers), cookie);
> +        op (&(cgn->indirect_calls), cookie);
> +        op (&(cgn->origin), cookie);
> +        op (&(cgn->nested), cookie);
> +        op (&(cgn->next_nested), cookie);
> +        op (&(cgn->next_sibling_clone), cookie);
> +        op (&(cgn->prev_sibling_clone), cookie);
> +        op (&(cgn->clones), cookie);
> +        op (&(cgn->clone_of), cookie);
> +        op (&(cgn->call_site_hash), cookie);
> +        op (&(cgn->former_clone_of), cookie);
> +        op (&(cgn->global.inlined_to), cookie);
> +        op (&(cgn->clone.tree_map), cookie);
> +        op (&(cgn->clone.args_to_skip), cookie);
> +        op (&(cgn->clone.combined_args_to_skip), cookie);
> +        op (&(cgn->thunk.alias), cookie);
> +      }
> +      break;
> +    default:
> +      break;
> +    }
> +}
> +
> +/* Workarounds for deficiencies in gengtype's handling of GTY((user)).
> +
> +   gengtype walks the graph of the *types* in the code, and produces
> +   functions in gtype-desc.[ch] for all types that are reachable from a
> +   GTY root.
> +
> +   However, it ignores the contents of GTY((user)) types when walking
> +   this graph.
> +
> +   Hence if you have a subgraph of types that are only reachable
> +   via fields in GTY((user)) types, gengtype won't generate helper code
> +   for those types.
> +
> +   Ideally there would be some way to mark a GTY((user)) type to say
> +   which types it references, to avoid having to mark these types as
> +   GTY((user)).
> +
> +   For now, work around this issue by providing explicit roots of the
> +   missing types, of dummy variables.  */
> +
> +/* Force gengtype into providing GTY hooks for the type of this field:
> +     htab_t GTY((param_is (struct cgraph_edge))) call_site_hash;
> +   within cgraph_node.  */
> +static GTY(()) htab_t GTY((param_is (struct cgraph_edge))) dummy_call_site_hash;
> +
> +/* Similarly for this field of symtab_node_base:
> +     struct ipa_ref_list ref_list;
> + */
> +static GTY((deletable)) struct ipa_ref_list *dummy_ipa_ref_list;
> +
> +/* Similarly for this field of cgraph_node:
> +     struct cgraph_clone_info clone;
> +*/
> +static GTY((deletable)) struct cgraph_clone_info *dummy_cgraph_clone_info;
> +
> +/* Non-existent pointer, to trick gengtype into (correctly) realizing that
> +   ipa_replace_map is indeed used in the code, and thus should have ggc_alloc_*
> +   routines.  */
> +static GTY((deletable)) struct ipa_replace_map *dummy_ipa_replace_map_pointer;
> +
> +/* Non-existent pointer, to trick gengtype into (correctly) realizing that
> +   varpool_node is indeed used in the code, and thus should have ggc_alloc_*
> +   routines.  */
> +static GTY((deletable)) varpool_node *dummy_varpool_node_ptr;
> +
>  #include "gt-cgraph.h"
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index e430533..412b5bb 100644
> --- a/gcc/cgraph.h
> +++ b/gcc/cgraph.h
> @@ -40,8 +40,9 @@ enum symtab_type
>
>  /* Base of all entries in the symbol table.
>     The symtab_node is inherited by cgraph and varpol nodes.  */
> -struct GTY(()) symtab_node_base
> +class GTY((user)) symtab_node_base
>  {
> +public:
>    /* Type of the symbol.  */
>    ENUM_BITFIELD (symtab_type) type : 8;
>
> @@ -143,6 +144,16 @@ struct GTY(()) symtab_node_base
>    PTR GTY ((skip)) aux;
>  };
>
> +/* These user-provided hooks are called by code in gtype-desc.c
> +   autogenerated by gengtype.c.
> +
> +   They are called the first time that the given object is visited
> +   within a GC/PCH traversal.
> +*/
> +extern void gt_ggc_mx (symtab_node_base *p);
> +extern void gt_pch_nx (symtab_node_base *p);
> +extern void gt_pch_nx (symtab_node_base *p, gt_pointer_operator op, void *cookie);
> +
>  enum availability
>  {
>    /* Not yet set by cgraph_function_body_availability.  */
> @@ -252,25 +263,19 @@ struct GTY(()) cgraph_clone_info
>  /* The cgraph data structure.
>     Each function decl has assigned cgraph_node listing callees and callers.  */
>
> -struct GTY(()) cgraph_node {
> -  struct symtab_node_base symbol;
> +struct GTY((user)) cgraph_node : public symtab_node_base {
> +public:
>    struct cgraph_edge *callees;
>    struct cgraph_edge *callers;
>    /* List of edges representing indirect calls with a yet undetermined
>       callee.  */
>    struct cgraph_edge *indirect_calls;
>    /* For nested functions points to function the node is nested in.  */
> -  struct cgraph_node *
> -    GTY ((nested_ptr (union symtab_node_def, "(struct cgraph_node *)(%h)", "(symtab_node)%h")))
> -    origin;
> +  struct cgraph_node *origin;
>    /* Points to first nested function, if any.  */
> -  struct cgraph_node *
> -    GTY ((nested_ptr (union symtab_node_def, "(struct cgraph_node *)(%h)", "(symtab_node)%h")))
> -    nested;
> +  struct cgraph_node *nested;
>    /* Pointer to the next function with same origin, if any.  */
> -  struct cgraph_node *
> -    GTY ((nested_ptr (union symtab_node_def, "(struct cgraph_node *)(%h)", "(symtab_node)%h")))
> -    next_nested;
> +  struct cgraph_node *next_nested;
>    /* Pointer to the next clone.  */
>    struct cgraph_node *next_sibling_clone;
>    struct cgraph_node *prev_sibling_clone;
> @@ -518,9 +523,8 @@ typedef struct cgraph_edge *cgraph_edge_p;
>  /* The varpool data structure.
>     Each static variable decl has assigned varpool_node.  */
>
> -struct GTY(()) varpool_node {
> -  struct symtab_node_base symbol;
> -
> +class GTY((user)) varpool_node : public symtab_node_base {
> +public:
>    /* Set when variable is scheduled to be assembled.  */
>    unsigned output : 1;
>  };
> @@ -536,22 +540,12 @@ struct GTY(()) asm_node {
>    int order;
>  };
>
> -/* Symbol table entry.  */
> -union GTY((desc ("%h.symbol.type"), chain_next ("%h.symbol.next"),
> -          chain_prev ("%h.symbol.previous"))) symtab_node_def {
> -  struct symtab_node_base GTY ((tag ("SYMTAB_SYMBOL"))) symbol;
> -  /* To access the following fields,
> -     use the use dyn_cast or as_a to obtain the concrete type.  */
> -  struct cgraph_node GTY ((tag ("SYMTAB_FUNCTION"))) x_function;
> -  struct varpool_node GTY ((tag ("SYMTAB_VARIABLE"))) x_variable;
> -};
> -
>  /* Report whether or not THIS symtab node is a function, aka cgraph_node.  */
>
>  template <>
>  template <>
>  inline bool
> -is_a_helper <cgraph_node>::test (symtab_node_def *p)
> +is_a_helper <cgraph_node>::test (symtab_node_base *p)
>  {
>    return p->symbol.type == SYMTAB_FUNCTION;
>  }
> @@ -561,7 +555,7 @@ is_a_helper <cgraph_node>::test (symtab_node_def *p)
>  template <>
>  template <>
>  inline bool
> -is_a_helper <varpool_node>::test (symtab_node_def *p)
> +is_a_helper <varpool_node>::test (symtab_node_base *p)
>  {
>    return p->symbol.type == SYMTAB_VARIABLE;
>  }
> diff --git a/gcc/ipa-ref.h b/gcc/ipa-ref.h
> index e0553bb..dc6e238 100644
> --- a/gcc/ipa-ref.h
> +++ b/gcc/ipa-ref.h
> @@ -20,9 +20,9 @@ along with GCC; see the file COPYING3.  If not see
>
>  struct cgraph_node;
>  struct varpool_node;
> -union symtab_node_def;
> -typedef union symtab_node_def *symtab_node;
> -typedef const union symtab_node_def *const_symtab_node;
> +class symtab_node_base;
> +typedef symtab_node_base *symtab_node;
> +typedef const symtab_node_base *const_symtab_node;
>
>
>  /* How the reference is done.  */
> diff --git a/gcc/is-a.h b/gcc/is-a.h
> index b5ee854..ccf12be 100644
> --- a/gcc/is-a.h
> +++ b/gcc/is-a.h
> @@ -31,7 +31,7 @@ bool is_a <TYPE> (pointer)
>
>      Tests whether the pointer actually points to a more derived TYPE.
>
> -    Suppose you have a symtab_node_def *ptr, AKA symtab_node ptr.  You can test
> +    Suppose you have a symtab_node_base *ptr, AKA symtab_node ptr.  You can test
>      whether it points to a 'derived' cgraph_node as follows.
>
>        if (is_a <cgraph_node> (ptr))
> @@ -110,7 +110,7 @@ example,
>    template <>
>    template <>
>    inline bool
> -  is_a_helper <cgraph_node>::test (symtab_node_def *p)
> +  is_a_helper <cgraph_node>::test (symtab_node_base *p)
>    {
>      return p->symbol.type == SYMTAB_FUNCTION;
>    }
> @@ -122,7 +122,7 @@ when needed may result in a crash.  For example,
>    template <>
>    template <>
>    inline bool
> -  is_a_helper <cgraph_node>::cast (symtab_node_def *p)
> +  is_a_helper <cgraph_node>::cast (symtab_node_base *p)
>    {
>      return &p->x_function;
>    }
> diff --git a/gcc/symtab.c b/gcc/symtab.c
> index a86bf6b..ba41f89 100644
> --- a/gcc/symtab.c
> +++ b/gcc/symtab.c
> @@ -48,9 +48,9 @@ const char * const ld_plugin_symbol_resolution_names[]=
>  };
>
>  /* Hash table used to convert declarations into nodes.  */
> -static GTY((param_is (union symtab_node_def))) htab_t symtab_hash;
> +static GTY((param_is (symtab_node_base))) htab_t symtab_hash;
>  /* Hash table used to convert assembler names into nodes.  */
> -static GTY((param_is (union symtab_node_def))) htab_t assembler_name_hash;
> +static GTY((param_is (symtab_node_base))) htab_t assembler_name_hash;
>
>  /* Linked list of symbol table nodes.  */
>  symtab_node symtab_nodes;
> --
> 1.7.11.7
>

> > +  while (x != xlimit)
> > +    {
> > +      /* Code common to all symtab nodes. */
> > +      gt_ggc_m_9tree_node (x->decl);
> > +      gt_ggc_mx_symtab_node_base (x->next);
> > +      gt_ggc_mx_symtab_node_base (x->previous);
> > +      gt_ggc_mx_symtab_node_base (x->next_sharing_asm_name);
> > +      gt_ggc_mx_symtab_node_base (x->previous_sharing_asm_name);
> > +      gt_ggc_mx_symtab_node_base (x->same_comdat_group);
> > +      gt_ggc_m_20vec_ipa_ref_t_va_gc_ (x->ref_list.references);
> > +      gt_ggc_m_9tree_node (x->alias_target);
> > +      gt_ggc_m_18lto_file_decl_data (x->lto_file_data);
> 
> Don't we have template specializations so we just can do
> 
>          gt_ggc_mark (x->decl);
>          gt_ggc_mark (x->next);
Yep, that is what I hope for.
> ...
> etc?
> 
> Also all of the symbol table is reachable from the global symbol_table
> dynamic array which is a GC root.  So instead of walking ->next/previous
> and edges you should have a custom marker for the symbol_table global
> which does more efficient marking with loops.

Indeed, good point!
All of the pointers linking symbol table together (i.e. pointer to other symbol
table entries) can be ignored for PCH mark (since we are explicitely allocated,
and all we need is to mark the trees/lto data and other stuff we point to.

These are annotated primarily to get PCH working, where we obviously need
to keep them.

It would be nice to progress on this patch soon :)

Honza
> 
> Richard.

On Tue, Aug 27, 2013 at 1:40 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > +  while (x != xlimit)
>> > +    {
>> > +      /* Code common to all symtab nodes. */
>> > +      gt_ggc_m_9tree_node (x->decl);
>> > +      gt_ggc_mx_symtab_node_base (x->next);
>> > +      gt_ggc_mx_symtab_node_base (x->previous);
>> > +      gt_ggc_mx_symtab_node_base (x->next_sharing_asm_name);
>> > +      gt_ggc_mx_symtab_node_base (x->previous_sharing_asm_name);
>> > +      gt_ggc_mx_symtab_node_base (x->same_comdat_group);
>> > +      gt_ggc_m_20vec_ipa_ref_t_va_gc_ (x->ref_list.references);
>> > +      gt_ggc_m_9tree_node (x->alias_target);
>> > +      gt_ggc_m_18lto_file_decl_data (x->lto_file_data);
>>
>> Don't we have template specializations so we just can do
>>
>>          gt_ggc_mark (x->decl);
>>          gt_ggc_mark (x->next);
> Yep, that is what I hope for.
>> ...
>> etc?
>>
>> Also all of the symbol table is reachable from the global symbol_table
>> dynamic array which is a GC root.  So instead of walking ->next/previous
>> and edges you should have a custom marker for the symbol_table global
>> which does more efficient marking with loops.
>
> Indeed, good point!
> All of the pointers linking symbol table together (i.e. pointer to other symbol
> table entries) can be ignored for PCH mark (since we are explicitely allocated,
> and all we need is to mark the trees/lto data and other stuff we point to.
>
> These are annotated primarily to get PCH working, where we obviously need
> to keep them.
>
> It would be nice to progress on this patch soon :)

Oh, and with a custom marker for the symbol_table GC root the actual
symbol / edge objects need not be GC allocated at all!  Just make sure
to properly mark the GC objects they refer to.

Richard.

> Honza
>>
>> Richard.

> >>
> >> Also all of the symbol table is reachable from the global symbol_table
> >> dynamic array which is a GC root.  So instead of walking ->next/previous
> >> and edges you should have a custom marker for the symbol_table global
> >> which does more efficient marking with loops.
> >
> > Indeed, good point!
> > All of the pointers linking symbol table together (i.e. pointer to other symbol
> > table entries) can be ignored for PCH mark (since we are explicitely allocated,
> > and all we need is to mark the trees/lto data and other stuff we point to.
> >
> > These are annotated primarily to get PCH working, where we obviously need
> > to keep them.
> >
> > It would be nice to progress on this patch soon :)
> 
> Oh, and with a custom marker for the symbol_table GC root the actual
> symbol / edge objects need not be GC allocated at all!  Just make sure
> to properly mark the GC objects they refer to.

Lets play with this incrementally; first nodes are streamed to PCH, so we need
to get them out of that mess first.  Second even edges reffers to trees that needs
to be walked. There will be some surprises.

My previous comment about not walking all node pointers was wrong; we do maintain
list of removed cgraph nodes linked by ->next and not referred from any global
array (or rather hash).
So I guess in first iteration we can go with marker that marks all non-symtabs
and walks the ->next linked chains.

Thanks,
Honza
> 
> Richard.
> 
> > Honza
> >>
> >> Richard.

On Aug 27, 2013, at 4:08 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>> and converts:
>>  struct GTY(()) cgraph_node
>> to:
>>  struct GTY((user)) cgraph_node : public symtab_node_base

GTY didn't like single inheritance for me in in wide-int.h.  I extended GTY to support it better.   See the wide-int branch, if you need more beef here.  It isn't flawless, but, it did left me to things to make it work nice enough for my purposes.  In my case, all my data is in the base class, and the base class and the derived have the same address (single inheritance, no virtual bases), and I have no embedded pointers in my data.  I don't use user, seemed less ideal to me.

I also had to put:

  void gt_ggc_mx(max_wide_int*) { }
  void gt_pch_nx(max_wide_int*,void (*)(void*, void*), void*) { }
  void gt_pch_nx(max_wide_int*) { }

in wide-int.cc and:

  extern void gt_ggc_mx(max_wide_int*);
  extern void gt_pch_nx(max_wide_int*,void (*)(void*, void*), void*);
  extern void gt_pch_nx(max_wide_int*);

into wide-int.h to get it to go.  These I think are merely because I'm using a typedef for a template with template args.  In the wide-int branch, it'd be nice if a GTY god can figure out how to improves things so I don't need to do the above, and to review/approve/fix the GTY code to support single inheritance even better.

>> and:
>>  struct GTY(()) varpool_node
>> to:
>>  class GTY((user)) varpool_node : public symtab_node_base
>> 
>> dropping the symtab_node_def union.
>> 
>> Since gengtype is unable to cope with inheritance, we have to mark the
>> types with GTY((user)), and handcode the gty field-visiting functions.
>> Given the simple hierarchy, we don't need virtual functions for this.

On Tue, Aug 27, 2013 at 5:06 PM, Mike Stump <mikestump@comcast.net> wrote:
> On Aug 27, 2013, at 4:08 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>> and converts:
>>>  struct GTY(()) cgraph_node
>>> to:
>>>  struct GTY((user)) cgraph_node : public symtab_node_base
>
> GTY didn't like single inheritance for me in in wide-int.h.  I extended GTY to support it better.   See the wide-int branch, if you need more beef here.  It isn't flawless, but, it did left me to things to make it work nice enough for my purposes.  In my case, all my data is in the base class, and the base class and the derived have the same address (single inheritance, no virtual bases), and I have no embedded pointers in my data.  I don't use user, seemed less ideal to me.
>
> I also had to put:
>
>   void gt_ggc_mx(max_wide_int*) { }
>   void gt_pch_nx(max_wide_int*,void (*)(void*, void*), void*) { }
>   void gt_pch_nx(max_wide_int*) { }
>
> in wide-int.cc and:
>
>   extern void gt_ggc_mx(max_wide_int*);
>   extern void gt_pch_nx(max_wide_int*,void (*)(void*, void*), void*);
>   extern void gt_pch_nx(max_wide_int*);
>
> into wide-int.h to get it to go.  These I think are merely because I'm using a typedef for a template with template args.  In the wide-int branch, it'd be nice if a GTY god can figure out how to improves things so I don't need to do the above, and to review/approve/fix the GTY code to support single inheritance even better.

Huh?  Why should wide-int need to be marked GTY at all?!

Richard.

>>> and:
>>>  struct GTY(()) varpool_node
>>> to:
>>>  class GTY((user)) varpool_node : public symtab_node_base
>>>
>>> dropping the symtab_node_def union.
>>>
>>> Since gengtype is unable to cope with inheritance, we have to mark the
>>> types with GTY((user)), and handcode the gty field-visiting functions.
>>> Given the simple hierarchy, we don't need virtual functions for this.

On Aug 28, 2013, at 2:34 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> Huh?  Why should wide-int need to be marked GTY at all?!

Can I answer with a question?  Why would nb_iter_bound be GTYed?  Why would dw_val_struct be GTYed?  The first makes little sense to me.  The second, well, it is used to generate debug information…  When the clients no longer want to GTY, we could remove it.  wide_ints seem like they should not make it to the PCH file.

On Wed, Aug 28, 2013 at 7:02 PM, Mike Stump <mikestump@comcast.net> wrote:
> On Aug 28, 2013, at 2:34 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>> Huh?  Why should wide-int need to be marked GTY at all?!
>
> Can I answer with a question?  Why would nb_iter_bound be GTYed?

Because it references a GIMPLE stmt which resides in GC memory.

Why would dw_val_struct be GTYed?

Because it resides in GC memory?

The first makes little sense to me.  The second, well, it is used to
generate debug information…  When the clients no longer want to GTY,
we could remove it.  wide_ints seem like they should not make it to
the PCH file.

I don't think this is PCH related.

Richard.