This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]