On Wed, Oct 12, 2011 at 7:01 PM, Diego Novillo <dnovi...@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.sande...@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
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);
