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