Patchwork [gimplefe] The symbol table for declarations

login
register
mail settings
Submitter Sandeep Soni
Date Nov. 18, 2011, 6:47 a.m.
Message ID <CANY-sXVpyEORwkT56ENdcYvp-EOAGtVK6a93RDV1CXk-VEX8Wg@mail.gmail.com>
Download mbox | patch
Permalink /patch/126350/
State New
Headers show

Comments

Sandeep Soni - Nov. 18, 2011, 6:47 a.m.
On Wed, Oct 12, 2011 at 7:01 PM, Diego Novillo <dnovillo@google.com> wrote:
>
> 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_*.

I have made all the changes to the earlier patch. I have attached the
new patch. In the new patch, hashing is done by IDENTIFIERs. I just
made a small change of putting the gimple_symtab_entry instance in the
symbol table rather than only the declaration. Is that okay?
ChangeLog is as follows:
2011-11-18  Sandeep Soni  <soni.sandeepb@gmail.com>

       * parser.c : Include hashtab.h.
       (struct gimple_symtab_entry_def): New.
       (gimple_symtab): New. The symbol table.
       (gimple_symtab_entry_hash): New.
       (gimple_symtab_eq_hash): New.
       (gp_parse_var_decl): Build the declaration and put it in the symbol
       table.
       (gimple_main): Creates the symbol table
>
>>    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.



--
Cheers
Sandy



--
Cheers
Sandy
Sandeep Soni - Nov. 30, 2011, 4:18 a.m.
Sorry. Wrong patch. New patch attached.

I am getting the following error in the new patch though.

../../gimple-front-end/gcc/gimple/parser.c: In function ‘gp_parse_var_decl’:
../../gimple-front-end/gcc/gimple/parser.c:927:3: error: implicit
declaration of function ‘ggc_alloc_cleared_gimple_symtab_entry_def’
[-Werror=implicit-function-declaration]
../../gimple-front-end/gcc/gimple/parser.c:927:5: error: assignment
makes pointer from integer without a cast [-Werror]
cc1: all warnings being treated as errors

make[3]: *** [gimple/parser.o] Error 1
make[3]: *** Waiting for unfinished jobs....
rm gfdl.pod cpp.pod gcov.pod gfortran.pod fsf-funding.pod gcc.pod
make[3]: Leaving directory `/home/Sandy/gimple_build/gcc'
make[2]: *** [all-stage2-gcc] Error 2
make[2]: Leaving directory `/home/Sandy/gimple_build'
make[1]: *** [stage2-bubble] Error 2
make[1]: Leaving directory `/home/Sandy/gimple_build'
make: *** [all] Error 2

Is there anything that needs to be initialized to use the
ggc_alloc_cleared_* function?

Patch

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,47 @@ 
 /* EOF token.  */
 static gimple_token gl_eof_token = { CPP_EOF, 0, 0, 0 };
 
+/* The GIMPLE symbol table entry.  */
+
+struct GTY (()) gimple_symtab_entry_def 
+{
+  /* symbol table entry key, an identifier.  */
+  tree id;
+
+  /* symbol table entry, a DECL.  */
+  tree decl;
+};
+
+typedef struct gimple_symtab_entry_def gimple_symtab_entry;
+
+/* 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 (base->id);
+}
+
+/* Returns non-zero if ENTRY1 and ENTRY2 point to gimple_symtab_entry_def
+   objects corresponding to the same declaration.  */
+
+static int
+gimple_symtab_eq_hash (const void *entry1, const void *entry2)
+{
+  const struct gimple_symtab_entry_def *base1 =
+    (const struct gimple_symtab_entry_def *)entry1;
+  const struct gimple_symtab_entry_def *base2 =
+    (const struct gimple_symtab_entry_def *)entry2;
+
+  return (base1->id == base2->id);
+}
+
 /* Return the string representation of token TOKEN.  */
 
 static const char *
@@ -807,6 +849,7 @@ 
     }
 }
 
+
 /* The Declaration section within a .gimple file can consist of 
    a) Declaration of variables.
    b) Declaration of functions.
@@ -870,18 +913,34 @@ 
 static void
 gp_parse_var_decl (gimple_parser *parser)
 {
-  const gimple_token *next_token;
+  const gimple_token *next_token, *name_token;
+  const char *name;
   enum tree_code code ;
+  struct gimple_symtab_entry_def e;
+  void **slot;
+  void **new_entry;
 
   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.id = get_identifier(name);
+  slot = htab_find_slot (gimple_symtab, &e, NO_INSERT); 
+  if (!slot)
+    {
+      e.decl = build_decl (name_token.location, VAR_DECL, get_identifier(name), void_type_node);
+      new_entry = htab_find_slot (gimple_symtab, &e, INSERT);
+      gcc_assert (*new_entry == NULL);
+      *new_entry = e;
+    }
+
   gl_consume_expected_token (parser->lexer, CPP_COMMA);
-
   next_token = gl_consume_token (parser->lexer);
   code = gl_tree_code_for_token (next_token);
   switch (code)
     {
     case INTEGER_TYPE:
+      gl_consume_expected_token (parser->lexer, CPP_LESS);
       gl_consume_expected_token (parser->lexer, CPP_NUMBER);
       gl_consume_expected_token (parser->lexer, CPP_RSHIFT);
       break;
@@ -981,6 +1040,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 +1463,9 @@ 
   if (parser->lexer->filename == NULL)
     return;
 
+  gimple_symtab =
+    htab_create_ggc (1021, gimple_symtab_entry_hash,
+		     gimple_symtab_eq_hash, NULL);
   gl_lex (parser->lexer);
   gp_parse (parser);
   gp_finish (parser);