Patchwork [ubsan] Use pointer map instead of hash table.

login
register
mail settings
Submitter Marek Polacek
Date Aug. 29, 2013, 1:28 p.m.
Message ID <20130829132817.GB23899@redhat.com>
Download mbox | patch
Permalink /patch/270825/
State New
Headers show

Comments

Marek Polacek - Aug. 29, 2013, 1:28 p.m.
On Thu, Aug 29, 2013 at 03:05:59PM +0200, Jakub Jelinek wrote:
> On Thu, Aug 29, 2013 at 02:56:58PM +0200, Marek Polacek wrote:
> > --- gcc/Makefile.in.mp	2013-08-29 14:24:49.839578625 +0200
> > +++ gcc/Makefile.in	2013-08-29 14:54:39.354258737 +0200
> > @@ -2273,7 +2273,7 @@ tsan.o : $(CONFIG_H) $(SYSTEM_H) $(TREE_
> >     intl.h cfghooks.h output.h options.h $(C_COMMON_H) tsan.h asan.h \
> >     tree-ssa-propagate.h
> >  ubsan.o : ubsan.c ubsan.h $(CONFIG_H) $(SYSTEM_H) $(GIMPLE_H) \
> > -   output.h coretypes.h $(TREE_H) $(CGRAPH_H) pointer-set.h \
> > +   output.h coretypes.h $(TREE_H) $(CGRAPH_H) $(HASH_TABLE_H) gt-ubsan.h \
> >     toplev.h $(C_COMMON_H)
> >  tree-ssa-tail-merge.o: tree-ssa-tail-merge.c \
> >     $(SYSTEM_H) $(CONFIG_H) coretypes.h $(TM_H) $(BITMAP_H) \
> > --- gcc/ubsan.c.mp	2013-08-29 14:24:49.840578629 +0200
> > +++ gcc/ubsan.c	2013-08-29 14:24:54.848597440 +0200
> > @@ -24,39 +24,71 @@ along with GCC; see the file COPYING3.
> >  #include "tree.h"
> >  #include "cgraph.h"
> >  #include "gimple.h"
> > +#include "hash-table.h"
> 
> Why not #include "hashtab.h" instead (and corresponding change in
> Makefile.in ?

Fixed.

> >  #include "pointer-set.h"
> >  #include "output.h"
> >  #include "toplev.h"
> >  #include "ubsan.h"
> >  #include "c-family/c-common.h"
> >  
> > -/* Map a TYPE to an ubsan type descriptor VAR_DECL for that type.  */
> > -static pointer_map<tree> *typedesc_map;
> > +/* Map from a tree to a VAR_DECL tree.  */
> >  
> > -/* Insert DECL as the VAR_DECL for TYPE in the TYPEDESC_MAP.  */
> > +struct GTY(()) tree_type_map {
> > +  struct tree_map_base type;
> > +  unsigned int hash;
> 
> You use TYPE_UID as hash, and never use hash field, so why you are adding
> it?

Removed the hash field.

> > +#define tree_type_map_eq tree_map_base_eq
> > +#define tree_type_map_marked_p tree_map_base_marked_p
> > +
> > +static GTY ((if_marked ("tree_type_map_marked_p"), param_is (struct tree_type_map)))
> > +     htab_t decl_tree_for_type;
> > +
> > +/* Initialize the hash table.  */
> >  
> >  static void
> > -insert_decl_for_type (tree decl, tree type)
> > +init_hash_table (void)
> >  {
> > -  *typedesc_map->insert (type) = decl;
> > +  decl_tree_for_type = htab_create_ggc (512, tree_decl_map_hash,
> > +					tree_decl_map_eq, 0);
> 
> tree_type_map_hash and tree_type_map_eq here, right?

Yeah.  Fixed.

> You could also just manually inline init_hash_table into the single caller, after
> all, it is just one statement.

Done.

> 512 might be too much, how many types we put there typically?  10, 20?

Of course, 10 should be enough.

> > -/* Find the VAR_DECL for TYPE in TYPEDESC_MAP.  If TYPE does not
> > -   exist in the map, return NULL_TREE, otherwise, return the VAR_DECL
> > -   we found.  */
> > +/* Lookup a VAR_DECL for TYPE, and return it if we find one.  */
> >  
> > -static tree
> > -lookup_decl_for_type (tree type)
> > +tree
> > +decl_for_type_lookup (tree type)
> 
> Why are you dropping the static here?

:( No clue.  Fixed.

> > +/* Insert a mapping TYPE->DECL in the VAR_DECL for type hashtable.  */
> > +
> > +void
> > +decl_for_type_insert (tree type, tree decl)
> 
> Again, can't this be static?

Fixed.

> Otherwise looks good.

Thanks, will give it another bootstrap-ubsan round and if that's fine,
I'll commit the following.

2013-08-29  Marek Polacek  <polacek@redhat.com>

	* Makefile.in (ubsan.o): Add HASHAB_H and gt-ubsan.h
	dependencies.  Remove pointer-set.h dependency.
	* ubsan.c: Convert to C style hash table.


	Marek

Patch

--- gcc/Makefile.in.mp	2013-08-29 14:24:49.839578625 +0200
+++ gcc/Makefile.in	2013-08-29 15:13:16.538323950 +0200
@@ -2273,7 +2273,7 @@  tsan.o : $(CONFIG_H) $(SYSTEM_H) $(TREE_
    intl.h cfghooks.h output.h options.h $(C_COMMON_H) tsan.h asan.h \
    tree-ssa-propagate.h
 ubsan.o : ubsan.c ubsan.h $(CONFIG_H) $(SYSTEM_H) $(GIMPLE_H) \
-   output.h coretypes.h $(TREE_H) $(CGRAPH_H) pointer-set.h \
+   output.h coretypes.h $(TREE_H) $(CGRAPH_H) $(HASHTAB_H) gt-ubsan.h \
    toplev.h $(C_COMMON_H)
 tree-ssa-tail-merge.o: tree-ssa-tail-merge.c \
    $(SYSTEM_H) $(CONFIG_H) coretypes.h $(TM_H) $(BITMAP_H) \
--- gcc/ubsan.c.mp	2013-08-29 14:24:49.840578629 +0200
+++ gcc/ubsan.c	2013-08-29 15:22:42.368445186 +0200
@@ -24,39 +24,63 @@  along with GCC; see the file COPYING3.
 #include "tree.h"
 #include "cgraph.h"
 #include "gimple.h"
+#include "hashtab.h"
 #include "pointer-set.h"
 #include "output.h"
 #include "toplev.h"
 #include "ubsan.h"
 #include "c-family/c-common.h"
 
-/* Map a TYPE to an ubsan type descriptor VAR_DECL for that type.  */
-static pointer_map<tree> *typedesc_map;
+/* Map from a tree to a VAR_DECL tree.  */
 
-/* Insert DECL as the VAR_DECL for TYPE in the TYPEDESC_MAP.  */
+struct GTY(()) tree_type_map {
+  struct tree_map_base type;
+  tree decl;
+};
 
-static void
-insert_decl_for_type (tree decl, tree type)
-{
-  *typedesc_map->insert (type) = decl;
-}
+#define tree_type_map_eq tree_map_base_eq
+#define tree_type_map_hash tree_map_base_hash
+#define tree_type_map_marked_p tree_map_base_marked_p
+
+static GTY ((if_marked ("tree_type_map_marked_p"), param_is (struct tree_type_map)))
+     htab_t decl_tree_for_type;
 
-/* Find the VAR_DECL for TYPE in TYPEDESC_MAP.  If TYPE does not
-   exist in the map, return NULL_TREE, otherwise, return the VAR_DECL
-   we found.  */
+/* Lookup a VAR_DECL for TYPE, and return it if we find one.  */
 
 static tree
-lookup_decl_for_type (tree type)
+decl_for_type_lookup (tree type)
 {
-  /* If the pointer map is not initialized yet, create it now.  */
-  if (typedesc_map == NULL)
+  /* If the hash table is not initialized yet, create it now.  */
+  if (decl_tree_for_type == NULL)
     {
-      typedesc_map = new pointer_map<tree>;
+      decl_tree_for_type = htab_create_ggc (10, tree_type_map_hash,
+					    tree_type_map_eq, 0);
       /* That also means we don't have to bother with the lookup.  */
       return NULL_TREE;
     }
-  tree *t = typedesc_map->contains (type);
-  return t ? *t : NULL_TREE;
+
+  struct tree_type_map *h, in;
+  in.type.from = type;
+
+  h = (struct tree_type_map *)
+      htab_find_with_hash (decl_tree_for_type, &in, TYPE_UID (type));
+  return h ? h->decl : NULL_TREE;
+}
+
+/* Insert a mapping TYPE->DECL in the VAR_DECL for type hashtable.  */
+
+static void
+decl_for_type_insert (tree type, tree decl)
+{
+  struct tree_type_map *h;
+  void **slot;
+
+  h = ggc_alloc_tree_type_map ();
+  h->type.from = type;
+  h->decl = decl;
+  slot = htab_find_slot_with_hash (decl_tree_for_type, h, TYPE_UID (type),
+                                  INSERT);
+  *(struct tree_type_map **) slot = h;
 }
 
 /* Helper routine, which encodes a value in the pointer_sized_int_node.
@@ -226,7 +250,7 @@  ubsan_type_descriptor (tree type)
   /* See through any typedefs.  */
   type = TYPE_MAIN_VARIANT (type);
 
-  tree decl = lookup_decl_for_type (type);
+  tree decl = decl_for_type_lookup (type);
   if (decl != NULL_TREE)
     return decl;
 
@@ -282,7 +306,7 @@  ubsan_type_descriptor (tree type)
 
   /* Save the address of the VAR_DECL into the pointer map.  */
   decl = build_fold_addr_expr (decl);
-  insert_decl_for_type (decl, type);
+  decl_for_type_insert (type, decl);
 
   return decl;
 }
@@ -388,3 +412,5 @@  is_ubsan_builtin_p (tree t)
   return strncmp (IDENTIFIER_POINTER (DECL_NAME (t)),
 		  "__builtin___ubsan_", 18) == 0;
 }
+
+#include "gt-ubsan.h"