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 ?
> #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?
> +#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?
You could also just manually inline init_hash_table into the single caller,
after
all, it is just one statement.
512 might be too much, how many types we put there typically? 10, 20?
> -/* 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?
> +/* 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?
Otherwise looks good.
Jakub
