[v2,1/6] 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(-)

On Mon, Sep 9, 2013 at 9:32 PM, 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.

In this case, which are the types not processed and which are the
helpers that are missing?

> 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)).

All types that are supposed to be tracked by gengtype need to be
marked with GTY(()) in their definition.  So maybe we are simply
missing such annotations?

Richard.

> 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 f12bf1b..4b12163 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -2994,4 +2994,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);
> +
> +      /* 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 5a7a949..2bd2dc4 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 8dc61d0..adc7c90 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
>

> 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 f12bf1b..4b12163 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -2994,4 +2994,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);
Aren't you marking next twice? Once by xlimit walk and one by recursion?

> +      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);

You can skip marking these.  They only point within symbol table and
not externally.

> +      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);
> +
> +      /* Extra code, per subclass. */
> +      switch (x->type)
Didn't we agreed on the is_a API?
> +        {
> +        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);
Same as here.

> +            gt_ggc_m_P11cgraph_edge4htab (cgn->call_site_hash);
Move call site hash out of GGC rather than introducing hack.

> +            gt_ggc_m_9tree_node (cgn->former_clone_of);
> +            gt_ggc_m_11cgraph_node (cgn->global.inlined_to);
And here

> +            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);

We can skip good part of those. Just small of those is build at PCH time. But lets do that incrementally, PCH is touchy business.

The patch is OK with those changes.  The dummy workarounds are ugly :(
Honza

On Tue, 2013-09-10 at 15:34 +0200, Jan Hubicka wrote:

Thanks for reviewing this, and sorry for the late response (I lost most
of last week to illness).  Some questions inline below...

> > 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)
> > 
[..]

> > diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> > index f12bf1b..4b12163 100644
> > --- a/gcc/cgraph.c
> > +++ b/gcc/cgraph.c
> > @@ -2994,4 +2994,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);
> Aren't you marking next twice? Once by xlimit walk and one by recursion?
Good catch; removed.

> > +      gt_ggc_mx_symtab_node_base (x->previous);
The comment above also applies to "previous", so I've removed this also.

> > +      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);
> 
> You can skip marking these.  They only point within symbol table and
> not externally.
OK; removed.

> > +      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);
> > +
> > +      /* Extra code, per subclass. */
> > +      switch (x->type)
> Didn't we agreed on the is_a API?

There's just one interesting subclass here, so I've converted this to:

          if (cgraph_node *cgn = dyn_cast <cgraph_node *> (x))
            {

eliminating the switch and static_cast.

> > +        {
> > +        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);
> Same as here.

Sorry, it's not clear to me what you meant by "Same as here." here.  Do
you mean that I can skip marking them because they're within the symbol
table?   If so, are you referring to all 10 fields above ("callees"
through "clone_of")?


> 
> > +            gt_ggc_m_P11cgraph_edge4htab (cgn->call_site_hash);
> Move call site hash out of GGC rather than introducing hack.

I see that this is allocated in cgraph_edge (), and it appears to be an
index.  I can create it with htab_create rather than htab_create_ggc,
but if so, there doesn't seem a natural place to call htab_delete.  Is
that OK?


> > +            gt_ggc_m_9tree_node (cgn->former_clone_of);
> > +            gt_ggc_m_11cgraph_node (cgn->global.inlined_to);
> And here

Again, do you mean that "inlined_to" can be skipped since it's in the
symbol table?


> > +            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);
> 
> We can skip good part of those. Just small of those is build at PCH time. But lets do that incrementally, PCH is touchy business.

OK.  I'll just make analogous changes here to those made to the
gt_ggc_mx function.

> 
> The patch is OK with those changes.  The dummy workarounds are ugly :(
> Honza

Thanks.

> > > +            cgraph_node *cgn = static_cast <cgraph_node *> (x);
> > > +            gt_ggc_m_11cgraph_edge (cgn->callers);
> > > +            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);
> > Same as here.
> 
> Sorry, it's not clear to me what you meant by "Same as here." here.  Do
> you mean that I can skip marking them because they're within the symbol
> table?   If so, are you referring to all 10 fields above ("callees"
> through "clone_of")?

All those pointers above points inside symbol table, so none of them needs
to be marked.  Actually those two needs however, I missed them with prevoius
reading:
> > > +            gt_ggc_m_11cgraph_edge (cgn->indirect_calls);
> > > +            gt_ggc_m_11cgraph_edge (cgn->callees);

Otherwise edges will probably get lost.
> 
> 
> > 
> > > +            gt_ggc_m_P11cgraph_edge4htab (cgn->call_site_hash);
> > Move call site hash out of GGC rather than introducing hack.
> 
> I see that this is allocated in cgraph_edge (), and it appears to be an
> index.  I can create it with htab_create rather than htab_create_ggc,
> but if so, there doesn't seem a natural place to call htab_delete.  Is
> that OK?

Delete it in cgraph_remove_node.
> 
> 
> > > +            gt_ggc_m_9tree_node (cgn->former_clone_of);
> > > +            gt_ggc_m_11cgraph_node (cgn->global.inlined_to);
> > And here
> 
> Again, do you mean that "inlined_to" can be skipped since it's in the
> symbol table?

yes.
> > > +            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);
> > 
> > We can skip good part of those. Just small of those is build at PCH time. But lets do that incrementally, PCH is touchy business.
> 
> OK.  I'll just make analogous changes here to those made to the
> gt_ggc_mx function.

I think some of the pointers that do not need to be marked still needs to be
streamed and fixed by PCH, so we probably want to keep them.  All the cloning
pointers above are not live at PCH time (they are always NULL).  Only reason
why PCH lives at that point is that C++ FE produces Cgraph nodes early for
funny reason. The nodes are not analyzed at that point and call edges do not
exist (if I recall right) Probably I can do this incrementally - we do not want
to break PCH with a large patch and then spend weeks analyzing it.  That things
can bite ;)

Honza

On Wed, Sep 18, 2013 at 9:24 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > > +            cgraph_node *cgn = static_cast <cgraph_node *> (x);
>> > > +            gt_ggc_m_11cgraph_edge (cgn->callers);
>> > > +            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);
>> > Same as here.
>>
>> Sorry, it's not clear to me what you meant by "Same as here." here.  Do
>> you mean that I can skip marking them because they're within the symbol
>> table?   If so, are you referring to all 10 fields above ("callees"
>> through "clone_of")?
>
> All those pointers above points inside symbol table, so none of them needs
> to be marked.  Actually those two needs however, I missed them with prevoius
> reading:
>> > > +            gt_ggc_m_11cgraph_edge (cgn->indirect_calls);
>> > > +            gt_ggc_m_11cgraph_edge (cgn->callees);
>
> Otherwise edges will probably get lost.
>>
>>
>> >
>> > > +            gt_ggc_m_P11cgraph_edge4htab (cgn->call_site_hash);
>> > Move call site hash out of GGC rather than introducing hack.
>>
>> I see that this is allocated in cgraph_edge (), and it appears to be an
>> index.  I can create it with htab_create rather than htab_create_ggc,
>> but if so, there doesn't seem a natural place to call htab_delete.  Is
>> that OK?
>
> Delete it in cgraph_remove_node.
>>
>>
>> > > +            gt_ggc_m_9tree_node (cgn->former_clone_of);
>> > > +            gt_ggc_m_11cgraph_node (cgn->global.inlined_to);
>> > And here
>>
>> Again, do you mean that "inlined_to" can be skipped since it's in the
>> symbol table?
>
> yes.
>> > > +            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);
>> >
>> > We can skip good part of those. Just small of those is build at PCH time. But lets do that incrementally, PCH is touchy business.
>>
>> OK.  I'll just make analogous changes here to those made to the
>> gt_ggc_mx function.
>
> I think some of the pointers that do not need to be marked still needs to be
> streamed and fixed by PCH, so we probably want to keep them.  All the cloning
> pointers above are not live at PCH time (they are always NULL).  Only reason
> why PCH lives at that point is that C++ FE produces Cgraph nodes early for
> funny reason. The nodes are not analyzed at that point and call edges do not
> exist (if I recall right) Probably I can do this incrementally - we do not want
> to break PCH with a large patch and then spend weeks analyzing it.  That things
> can bite ;)

Debugging omissions / errors in manually written markers will indeed be a
PITA :/

Richard.

> Honza