Patchwork [cxx-conversion] LTO-related hash tables

login
register
mail settings
Submitter Lawrence Crowl
Date Dec. 2, 2012, 1:40 a.m.
Message ID <CAGqM8fYWKNpf48R1_Q+Y6PECK=uzMPWsjosay9z5x2=o4Hvjvg@mail.gmail.com>
Download mbox | patch
Permalink /patch/203168/
State New
Headers show

Comments

Lawrence Crowl - Dec. 2, 2012, 1:40 a.m.
Change LTO-related hash tables from htab_t to hash_table:

lto-streamer.h output_block::string_hash_table
lto-streamer-in.c file_name_hash_table
lto-streamer.c tree_htab

The struct string_slot moves from data-streamer.h to lto-streamer.h to
resolve compilation dependences.


Tested on x86-64.

Okay for branch?
Diego Novillo - Dec. 3, 2012, 8:04 p.m.
On 2012-12-01 20:40 , Lawrence Crowl wrote:
> Change LTO-related hash tables from htab_t to hash_table:
>
> lto-streamer.h output_block::string_hash_table
> lto-streamer-in.c file_name_hash_table
> lto-streamer.c tree_htab
>
> The struct string_slot moves from data-streamer.h to lto-streamer.h to
> resolve compilation dependences.

What compilation dependences?  If it was fine before in data-streamer.h 
why does it need to move to lto-streamer.h?  I'm missing that connection.


Diego.
Lawrence Crowl - Dec. 3, 2012, 9:10 p.m.
On 12/3/12, Diego Novillo <dnovillo@google.com> wrote:
> On 2012-12-01 20:40 , Lawrence Crowl wrote:
> > Change LTO-related hash tables from htab_t to hash_table:
> >
> > lto-streamer.h output_block::string_hash_table
> > lto-streamer-in.c file_name_hash_table
> > lto-streamer.c tree_htab
> >
> > The struct string_slot moves from data-streamer.h to
> > lto-streamer.h to resolve compilation dependences.
>
> What compilation dependences?  If it was fine before in
> data-streamer.h why does it need to move to lto-streamer.h?
> I'm missing that connection.

Before the change, lto-streamer.h output_block::string_hash_table was
an htab_t.  The element type was opaque, i.e. implicit in the void*
casting in the hash functions provided at the htab_create call site.

With the change to hash_table, the output_block::string_hash_table
declaration needed string_slot_hasher which in turn needed
string_slot.  But string_slot was defined in data-streamer.h after
output_block in lto-streamer.h.  So, something had to move.
Diego Novillo - Dec. 4, 2012, 1:57 p.m.
On Mon, Dec 3, 2012 at 4:10 PM, Lawrence Crowl <crowl@googlers.com> wrote:

> Before the change, lto-streamer.h output_block::string_hash_table was
> an htab_t.  The element type was opaque, i.e. implicit in the void*
> casting in the hash functions provided at the htab_create call site.
>
> With the change to hash_table, the output_block::string_hash_table
> declaration needed string_slot_hasher which in turn needed
> string_slot.  But string_slot was defined in data-streamer.h after
> output_block in lto-streamer.h.  So, something had to move.

Ah, thanks.


Diego.

Patch

Index: gcc/ChangeLog

2012-11-30  Lawrence Crowl  <crowl@google.com>

	* data-streamer.h (struct string_slot): Move to lto-streamer.h.
	(hash_string_slot_node): Move implementation into lto-streamer.h
	struct string_slot_hasher.
	(eq_string_slot_node): Likewise.

	* data-streamer-out.c: Update output_block::string_hash_table
	dependent calls and types.

	* lto-streamer.h (struct string_slot): Move from data-streamer.h
	(struct string_slot_hasher): New.
	(htab_t output_block::string_hash_table):
	Change type to hash_table.  Update dependent calls and types.

	* lto-streamer-in.c (freeing_string_slot_hasher): New.
	(htab_t file_name_hash_table):
	Change type to hash_table.  Update dependent calls and types.

	* lto-streamer-out.c: Update output_block::string_hash_table dependent
	calls and types.

	* lto-streamer.c (htab_t tree_htab):
	Change type to hash_table.  Update dependent calls and types.

	* Makefile.in: Update to changes above.

Index: gcc/data-streamer-out.c
===================================================================
--- gcc/data-streamer-out.c	(revision 193902)
+++ gcc/data-streamer-out.c	(working copy)
@@ -42,8 +42,7 @@  streamer_string_index (struct output_blo
   s_slot.len = len;
   s_slot.slot_num = 0;

-  slot = (struct string_slot **) htab_find_slot (ob->string_hash_table,
-						 &s_slot, INSERT);
+  slot = ob->string_hash_table.find_slot (&s_slot, INSERT);
   if (*slot == NULL)
     {
       struct lto_output_stream *string_stream = ob->string_stream;
Index: gcc/lto-streamer-out.c
===================================================================
--- gcc/lto-streamer-out.c	(revision 193902)
+++ gcc/lto-streamer-out.c	(working copy)
@@ -77,8 +77,7 @@  create_output_block (enum lto_section_ty

   clear_line_info (ob);

-  ob->string_hash_table = htab_create (37, hash_string_slot_node,
-				       eq_string_slot_node, NULL);
+  ob->string_hash_table.create (37);
   gcc_obstack_init (&ob->obstack);

   return ob;
@@ -92,7 +91,7 @@  destroy_output_block (struct output_bloc
 {
   enum lto_section_type section_type = ob->section_type;

-  htab_delete (ob->string_hash_table);
+  ob->string_hash_table.dispose ();

   free (ob->main_stream);
   free (ob->string_stream);
Index: gcc/lto-streamer-in.c
===================================================================
--- gcc/lto-streamer-in.c	(revision 193902)
+++ gcc/lto-streamer-in.c	(working copy)
@@ -49,8 +49,19 @@  along with GCC; see the file COPYING3.
 #include "tree-pass.h"
 #include "streamer-hooks.h"

+struct freeing_string_slot_hasher : string_slot_hasher
+{
+  static inline void remove (value_type *);
+};
+
+inline void
+freeing_string_slot_hasher::remove (value_type *v)
+{
+  free (v);
+}
+
 /* The table to hold the file names.  */
-static htab_t file_name_hash_table;
+static hash_table <freeing_string_slot_hasher> file_name_hash_table;


 /* Check that tag ACTUAL has one of the given values.  NUM_TAGS is the
@@ -94,14 +105,14 @@  lto_input_data_block (struct lto_input_b
 static const char *
 canon_file_name (const char *string)
 {
-  void **slot;
+  string_slot **slot;
   struct string_slot s_slot;
   size_t len = strlen (string);

   s_slot.s = string;
   s_slot.len = len;

-  slot = htab_find_slot (file_name_hash_table, &s_slot, INSERT);
+  slot = file_name_hash_table.find_slot (&s_slot, INSERT);
   if (*slot == NULL)
     {
       char *saved_string;
@@ -117,7 +128,7 @@  canon_file_name (const char *string)
     }
   else
     {
-      struct string_slot *old_slot = (struct string_slot *) *slot;
+      struct string_slot *old_slot = *slot;
       return old_slot->s;
     }
 }
@@ -1150,8 +1161,7 @@  void
 lto_reader_init (void)
 {
   lto_streamer_init ();
-  file_name_hash_table = htab_create (37, hash_string_slot_node,
-				      eq_string_slot_node, free);
+  file_name_hash_table.create (37);
 }


Index: gcc/data-streamer.h
===================================================================
--- gcc/data-streamer.h	(revision 193902)
+++ gcc/data-streamer.h	(working copy)
@@ -44,15 +44,6 @@  struct bitpack_d
   void *stream;
 };

-
-/* String hashing.  */
-struct string_slot
-{
-  const char *s;
-  int len;
-  unsigned int slot_num;
-};
-
 /* In data-streamer.c  */
 void bp_pack_var_len_unsigned (struct bitpack_d *, unsigned HOST_WIDE_INT);
 void bp_pack_var_len_int (struct bitpack_d *, HOST_WIDE_INT);
@@ -90,35 +81,6 @@  const char *bp_unpack_string (struct dat
 unsigned HOST_WIDE_INT streamer_read_uhwi (struct lto_input_block *);
 HOST_WIDE_INT streamer_read_hwi (struct lto_input_block *);

-/* Returns a hash code for P.  Adapted from libiberty's htab_hash_string
-   to support strings that may not end in '\0'.  */
-
-static inline hashval_t
-hash_string_slot_node (const void *p)
-{
-  const struct string_slot *ds = (const struct string_slot *) p;
-  hashval_t r = ds->len;
-  int i;
-
-  for (i = 0; i < ds->len; i++)
-     r = r * 67 + (unsigned)ds->s[i] - 113;
-  return r;
-}
-
-/* Returns nonzero if P1 and P2 are equal.  */
-
-static inline int
-eq_string_slot_node (const void *p1, const void *p2)
-{
-  const struct string_slot *ds1 = (const struct string_slot *) p1;
-  const struct string_slot *ds2 = (const struct string_slot *) p2;
-
-  if (ds1->len == ds2->len)
-    return memcmp (ds1->s, ds2->s, ds1->len) == 0;
-
-  return 0;
-}
-
 /* Returns a new bit-packing context for bit-packing into S.  */
 static inline struct bitpack_d
 bitpack_create (struct lto_output_stream *s)
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in	(revision 193902)
+++ gcc/Makefile.in	(working copy)
@@ -958,7 +958,7 @@  IRA_INT_H = ira.h ira-int.h $(CFGLOOP_H)
 LRA_INT_H = lra.h $(BITMAP_H) $(RECOG_H) $(INSN_ATTR_H) insn-codes.h lra-int.h
 DBGCNT_H = dbgcnt.h dbgcnt.def
 LTO_STREAMER_H = lto-streamer.h $(LINKER_PLUGIN_API_H) $(TARGET_H) \
-		$(CGRAPH_H) $(VEC_H) $(TREE_H) $(GIMPLE_H) \
+		$(CGRAPH_H) $(VEC_H) $(HASH_TABLE_H) $(TREE_H) $(GIMPLE_H) \
 		$(GCOV_IO_H) $(DIAGNOSTIC_H) alloc-pool.h
 DATA_STREAMER_H = data-streamer.h $(VEC_H) $(LTO_STREAMER_H)
 GIMPLE_STREAMER_H = gimple-streamer.h $(LTO_STREAMER_H) $(BASIC_BLOCK_H) \
Index: gcc/lto-streamer.c
===================================================================
--- gcc/lto-streamer.c	(revision 193902)
+++ gcc/lto-streamer.c	(working copy)
@@ -253,28 +253,33 @@  print_lto_report (const char *s)


 #ifdef LTO_STREAMER_DEBUG
-static htab_t tree_htab;
-
 struct tree_hash_entry
 {
   tree key;
   intptr_t value;
 };

-static hashval_t
-hash_tree (const void *p)
+struct tree_entry_hasher : typed_noop_remove <tree_hash_entry>
+{
+  typedef tree_hash_entry value_type;
+  typedef tree_hash_entry compare_type;
+  static inline hashval_t hash (const value_type *);
+  static inline bool equal (const value_type *, const compare_type *);
+};
+
+inline hashval_t
+tree_entry_hasher::hash (const value_type *e)
 {
-  const struct tree_hash_entry *e = (const struct tree_hash_entry *) p;
   return htab_hash_pointer (e->key);
 }

-static int
-eq_tree (const void *p1, const void *p2)
+inline bool
+tree_entry_hasher::equal (const value_type *e1, const compare_type *e2)
 {
-  const struct tree_hash_entry *e1 = (const struct tree_hash_entry *) p1;
-  const struct tree_hash_entry *e2 = (const struct tree_hash_entry *) p2;
   return (e1->key == e2->key);
 }
+
+static hash_table <tree_hash_entry> tree_htab;
 #endif

 /* Initialization common to the LTO reader and writer.  */
@@ -289,7 +294,7 @@  lto_streamer_init (void)
   streamer_check_handled_ts_structures ();

 #ifdef LTO_STREAMER_DEBUG
-  tree_htab = htab_create (31, hash_tree, eq_tree, NULL);
+  tree_htab.create (31);
 #endif
 }

@@ -324,8 +329,7 @@  lto_orig_address_map (tree t, intptr_t o

   ent.key = t;
   ent.value = orig_t;
-  slot
-    = (struct tree_hash_entry **) htab_find_slot (tree_htab, &ent, INSERT);
+  slot = tree_htab.find_slot (&ent, INSERT);
   gcc_assert (!*slot);
   *slot = XNEW (struct tree_hash_entry);
   **slot = ent;
@@ -342,8 +346,7 @@  lto_orig_address_get (tree t)
   struct tree_hash_entry **slot;

   ent.key = t;
-  slot
-    = (struct tree_hash_entry **) htab_find_slot (tree_htab, &ent, NO_INSERT);
+  slot = tree_htab.find_slot (&ent, NO_INSERT);
   return (slot ? (*slot)->value : 0);
 }

@@ -357,11 +360,10 @@  lto_orig_address_remove (tree t)
   struct tree_hash_entry **slot;

   ent.key = t;
-  slot
-    = (struct tree_hash_entry **) htab_find_slot (tree_htab, &ent, NO_INSERT);
+  slot = tree_htab.find_slot (&ent, NO_INSERT);
   gcc_assert (slot);
   free (*slot);
-  htab_clear_slot (tree_htab, (PTR *)slot);
+  tree_htab.clear_slot (slot);
 }
 #endif

Index: gcc/lto-streamer.h
===================================================================
--- gcc/lto-streamer.h	(revision 193902)
+++ gcc/lto-streamer.h	(working copy)
@@ -24,6 +24,7 @@  along with GCC; see the file COPYING3.
 #define GCC_LTO_STREAMER_H

 #include "plugin-api.h"
+#include "hash-table.h"
 #include "tree.h"
 #include "gimple.h"
 #include "target.h"
@@ -626,6 +627,50 @@  struct lto_simple_output_block
   struct lto_output_stream *main_stream;
 };

+/* String hashing.  */
+
+struct string_slot
+{
+  const char *s;
+  int len;
+  unsigned int slot_num;
+};
+
+/* Hashtable helpers.  */
+
+struct string_slot_hasher : typed_noop_remove <string_slot>
+{
+  typedef string_slot value_type;
+  typedef string_slot compare_type;
+  static inline hashval_t hash (const value_type *);
+  static inline bool equal (const value_type *, const compare_type *);
+};
+
+/* Returns a hash code for DS.  Adapted from libiberty's htab_hash_string
+   to support strings that may not end in '\0'.  */
+
+inline hashval_t
+string_slot_hasher::hash (const value_type *ds)
+{
+  hashval_t r = ds->len;
+  int i;
+
+  for (i = 0; i < ds->len; i++)
+     r = r * 67 + (unsigned)ds->s[i] - 113;
+  return r;
+}
+
+/* Returns nonzero if DS1 and DS2 are equal.  */
+
+inline bool
+string_slot_hasher::equal (const value_type *ds1, const compare_type *ds2)
+{
+  if (ds1->len == ds2->len)
+    return memcmp (ds1->s, ds2->s, ds1->len) == 0;
+
+  return 0;
+}
+
 /* Data structure holding all the data and descriptors used when writing
    an LTO file.  */
 struct output_block
@@ -644,7 +689,7 @@  struct output_block

   /* The hash table that contains the set of strings we have seen so
      far and the indexes assigned to them.  */
-  htab_t string_hash_table;
+  hash_table <string_slot_hasher> string_hash_table;

   /* The current cgraph_node that we are currently serializing.  Null
      if we are serializing something else.  */