This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [ubsan] Use pointer map instead of hash table.


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]