Re: Symbol table 1/many: symtab_nodes

[Richard Guenther <rguenther at suse dot de>]

On Tue, 10 Apr 2012, Jan Hubicka wrote:

> Hi,
> this is very basis of the symbol table work.  The basic idea is:
> 
> 1) give cgraph node and varpool node common base (symtab node)
> 2) Move common data there: that is type, declaration, visibility flags,
>    force_output, ipa references and other stuff.
> 3) Introduce symtab.c for basic symbol table manipulation that in first run
>    is represented as simple linked list of cgraph/varpool node entries.
> 4) Update ipa-reference API for the new class structure
> 5) Significantly clanup/rework old cgraph reachability and construction code
>    that has grown up very convoluted to meet non-unit-at-a-time needs.
>    Merge most of logic done on varpool and cgraph into common grounds
> 6) reorg WHOPR partitioning code
> 7) Add new types of symbol table entries - labels, aliases and possibly
>    constant pools and symbols defined by asm statements so existing WHOPR bugs
>    have chance to be solved.
> 8) Add actual symbol name hash - this is trickier than it sounds given that
>    GCC currently don't really know the symbol names on non-LTO targets.
>    I plan to simply take the existing lto-symtab logic and slowly require
>    targtets to provide hook to translate ASSEMBLER_NAME into symbol name.
>    It would be nice to get rid of ASSEMBLER_NAME and replace it by SYMBOL_NAME
>    + set of flags, but this is very hard to do since assembler name goes
>    everywhere into target machinery.  I am not really volunteering to do that
>    at this moment.
> 9) Reorg most of lto-symtab to work on symtab keeping the prevailing and
>    other stuff there.
> 
> I would like to get most of the changes into mainline for this stage1.  In worst
> case at least 1-6.
> 
> I prototytped all except for lto-symtab reorg, but I would like to re-do
> everything incrementally for mainline with cleanups on the way.  Many
> of things can be significantly simplified over current implementation
> and I hope it will make it easier to follow the changes and get me some useful
> feedback.  It is kind of second chance for cleaner cgraph API and it would
> be nice to get it mostly right.
> 
> This patch takes the very first step by adding the base object.  It implements
> inheritance in C via cgraph/varpool accessors. C++ would make sense here, but I
> think I still can't because of gengtype.  Once C++ is not only allowed, but
> also useful, I don't think changing the syntactical shugar to actual classes
> would be a significant project.

I agree.  I hope we get to 8), as lto-symtab requires that ;)

This is ok.

Thanks for finally pushing this,
Richard.

> Honza
> 
> 	* cgraph.h: Remove misledaing comment on ipa-ref.h.
> 	(symtab_type): New enum.
> 	(symtab_node): New structure.
> 	(cgraph_node, varpool_node): Add symbol base type.
> 	(cgraph, varpool): New accestor functions.
> 	* cgraph.c (cgraph_create_node_1): Set symbol type.
> 	* varpool.c (varpool_node): Set symbol type.
> Index: cgraph.h
> ===================================================================
> --- cgraph.h	(revision 186252)
> +++ cgraph.h	(working copy)
> @@ -27,7 +27,23 @@ along with GCC; see the file COPYING3.
>  #include "tree.h"
>  #include "basic-block.h"
>  #include "function.h"
> -#include "ipa-ref.h"	/* FIXME: inappropriate dependency of cgraph on IPA.  */
> +#include "ipa-ref.h"
> +
> +/* Symbol table consists of functions and variables.
> +   TODO: add labels, constant pool and aliases.  */
> +enum symtab_type
> +{
> +  SYMTAB_FUNCTION,
> +  SYMTAB_VARIABLE
> +};
> +
> +/* Base of all entries in the symbol table.
> +   The symtab_node is inherited by cgraph and varpol nodes.  */
> +struct GTY(()) symtab_node
> +{
> +  /* Type of the symbol.  */
> +  enum symtab_type type;
> +};
>  
>  enum availability
>  {
> @@ -150,6 +166,7 @@ struct GTY(()) cgraph_clone_info
>     Each function decl has assigned cgraph_node listing callees and callers.  */
>  
>  struct GTY((chain_next ("%h.next"), chain_prev ("%h.previous"))) cgraph_node {
> +  struct symtab_node symbol;
>    tree decl;
>    struct cgraph_edge *callees;
>    struct cgraph_edge *callers;
> @@ -387,6 +404,7 @@ DEF_VEC_ALLOC_P(cgraph_edge_p,heap);
>     Each static variable decl has assigned varpool_node.  */
>  
>  struct GTY((chain_next ("%h.next"), chain_prev ("%h.prev"))) varpool_node {
> +  struct symtab_node symbol;
>    tree decl;
>    /* For aliases points to declaration DECL is alias of.  */
>    tree alias_of;
> @@ -688,6 +706,23 @@ void varpool_add_new_variable (tree);
>  #define FOR_EACH_STATIC_VARIABLE(node) \
>     for ((node) = varpool_nodes_queue; (node); (node) = (node)->next_needed)
>  
> +/* Return callgraph node for given symbol and check it is a function. */
> +static inline struct cgraph_node *
> +cgraph (struct symtab_node *node)
> +{
> +  gcc_checking_assert (node->type == SYMTAB_FUNCTION);
> +  return (struct cgraph_node *)node;
> +}
> +
> +/* Return varpool node for given symbol and check it is a variable.  */
> +static inline struct varpool_node *
> +varpool (struct symtab_node *node)
> +{
> +  gcc_checking_assert (node->type == SYMTAB_FUNCTION);
> +  return (struct varpool_node *)node;
> +}
> +
> +
>  /* Return first reachable static variable with initializer.  */
>  static inline struct varpool_node *
>  varpool_first_static_initializer (void)
> Index: cgraph.c
> ===================================================================
> --- cgraph.c	(revision 186252)
> +++ cgraph.c	(working copy)
> @@ -473,6 +473,7 @@ cgraph_create_node_1 (void)
>  {
>    struct cgraph_node *node = cgraph_allocate_node ();
>  
> +  node->symbol.type = SYMTAB_FUNCTION;
>    node->next = cgraph_nodes;
>    node->order = cgraph_order++;
>    if (cgraph_nodes)
> Index: varpool.c
> ===================================================================
> --- varpool.c	(revision 186252)
> +++ varpool.c	(working copy)
> @@ -142,6 +142,7 @@ varpool_node (tree decl)
>    if (*slot)
>      return *slot;
>    node = ggc_alloc_cleared_varpool_node ();
> +  node->symbol.type = SYMTAB_VARIABLE;
>    node->decl = decl;
>    node->order = cgraph_order++;
>    node->next = varpool_nodes;
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer