[gimplefe][patch] The symbol table for declarations

[Diego Novillo]

On 11-10-10 17:47 , Sandeep Soni wrote:
> Hi,
> The following patch is a basic attempt to build a symbol table that
> stores the names of all the declarations made in the input file.
>
> Index: gcc/gimple/parser.c
> ===================================================================
> --- gcc/gimple/parser.c	(revision 174754)
> +++ gcc/gimple/parser.c	(working copy)
> @@ -28,6 +28,7 @@
>   #include "tree.h"
>   #include "gimple.h"
>   #include "parser.h"
> +#include "hashtab.h"
>   #include "ggc.h"
>
>   /* The GIMPLE parser.  Note: do not use this variable directly.  It is
> @@ -44,6 +45,43 @@
>   /* EOF token.  */
>   static gimple_token gl_eof_token = { CPP_EOF, 0, 0, 0 };
>
> +/* The GIMPLE symbol table entry.  */
> +
> +struct GTY (()) gimple_symtab_entry_def
> +{
> +  /* Variable that is declared.  */
> +  tree decl;
> +
> +};

No blank line before '};'

Add 'typedef struct gimple_symtab_entry_def gimple_symtab_entry;' to 
shorten declarations.

> +
> +/* Gimple symbol table.  */
> +static htab_t gimple_symtab;
> +
> +/* Return the hash value of the declaration name of a gimple_symtab_entry_def
> +   object pointed by ENTRY.  */
> +
> +static hashval_t
> +gimple_symtab_entry_hash (const void *entry)
> +{
> +  const struct gimple_symtab_entry_def *base =
> +    (const struct gimple_symtab_entry_def *)entry;
> +  return IDENTIFIER_HASH_VALUE (DECL_NAME(base->decl));

Space after DECL_NAME.

> +}
> +
> +/* Returns non-zero if ENTRY1 and ENTRY2 points to gimple_symtab_entry_def

s/points/point/

> +   objects corresponding to the same declaration.  */
> +
> +static int
> +gimple_symtab_eq_hash (const void *entry1, const void *entry2)
> +{
> +  const struct gimple_symtab_entry_def *p1 =
> +    (const struct gimple_symtab_entry_def *)entry1;
> +  const struct gimple_symtab_entry_def *p2 =
> +    (const struct gimple_symtab_entry_def *)entry2;
> +
> +  return DECL_NAME(p1->decl) == DECL_NAME(p2->decl);

Space after DECL_NAME.

> +}
> +
>   /* Return the string representation of token TOKEN.  */
>
>   static const char *
> @@ -807,6 +845,7 @@
>       }
>   }
>
> +
>   /* The Declaration section within a .gimple file can consist of
>      a) Declaration of variables.
>      b) Declaration of functions.
> @@ -870,11 +909,17 @@
>   static void
>   gp_parse_var_decl (gimple_parser *parser)
>   {
> -  const gimple_token *next_token;
> +  const gimple_token *next_token, *name_token;
> +  const char* name;

s/char* /char */

>     enum tree_code code ;
> +  struct gimple_symtab_entry_def e;
>
>     gl_consume_expected_token (parser->lexer, CPP_LESS);
> -  gl_consume_expected_token (parser->lexer, CPP_NAME);
> +  name_token = gl_consume_expected_token (parser->lexer, CPP_NAME);
> +  name = gl_token_as_text (name_token);
> +  e.decl =
> +  build_decl (UNKNOWN_LOCATION, VAR_DECL, get_identifier(name),
> void_type_node);

No need to use UNKNOWN_LOCATION.  Get the location for E.DECL from 
name_token.location.

Additionally, before building the decl, you should make sure that the 
symbol table does not already have it.  So, instead of looking up with a 
DECL, you should look it up using IDENTIFIER_NODEs.  There are two 
approaches you can use:

1- Add an identifier field to gimple_symtab_entry_def.  Use that field 
for hash table lookups (in this code you'd then fill E.ID with NAME_TOKEN).

2- Use a pointer_map_t and a VEC().  With this approach, you use a 
pointer map to map identifier nodes to unsigned integers.  These 
integers are the index into the VEC() array where the corresponding decl 
is stored.

In this case, I think #1 is the simplest approach.

> +  htab_find_slot (gimple_symtab,&e, INSERT);

This looks wrong.  Where are you actually filling in the slot?  You need 
to check the returned slot, if it's empty, you fill it in with E.DECL. 
See other uses of htab_*.

>     gl_consume_expected_token (parser->lexer, CPP_COMMA);
>
>     next_token = gl_consume_token (parser->lexer);
> @@ -981,6 +1027,7 @@
>     gimple_parser *parser = ggc_alloc_cleared_gimple_parser ();
>     line_table = parser->line_table = ggc_alloc_cleared_line_maps ();
>     parser->ident_hash = ident_hash;
> +
>     linemap_init (parser->line_table);
>     parser->lexer = gl_init (parser, fname);
>
> @@ -1403,6 +1450,9 @@
>     if (parser->lexer->filename == NULL)
>       return;
>
> +  gimple_symtab =
> +    htab_create_ggc (1021, gimple_symtab_entry_hash,
> +		     gimple_symtab_eq_hash, NULL);

Do you need to indent it this way?  Seems to me that the call to 
htab_create_ggc can fit in the line above.


Diego.