| Message ID | CAGqM8fbEVf+zLCy62egAvxDCHLy7MJcqg3FZ16gfOSNSav4Beg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Dec 11, 2012 at 5:44 PM, Lawrence Crowl <crowl@googlers.com> wrote: > Convert tree-sra.c'candidates from htab_t to hash_table. > > Fold uid_decl_map_hash and uid_decl_map_eq into new struct > uid_decl_hasher. This change moves the definitions from tree-ssa.c > into tree-sra.c and removes the declarations from tree-flow.h > > Update dependent calls and types to hash_table. > > Tested on x86_64. > > Okay for branch? > > > Index: gcc/tree-sra.c > =================================================================== > --- gcc/tree-sra.c (revision 194381) > +++ gcc/tree-sra.c (working copy) > @@ -74,6 +74,7 @@ along with GCC; see the file COPYING3. > #include "config.h" > #include "system.h" > #include "coretypes.h" > +#include "hash-table.h" > #include "alloc-pool.h" > #include "tm.h" > #include "tree.h" > @@ -269,18 +270,44 @@ static alloc_pool link_pool; > /* Base (tree) -> Vector (vec<access_p> *) map. */ > static struct pointer_map_t *base_access_vec; > > +/* Candidate ashtable helpers. */ s/ashtable/hash table/ > + > +struct uid_decl_hasher : typed_noop_remove <tree_node> > +{ > + typedef tree_node value_type; > + typedef tree_node compare_type; > + static inline hashval_t hash (const value_type *); > + static inline bool equal (const value_type *, const compare_type *); > +}; > + > +/* Hash a tree in a uid_decl_map. */ > + > +inline hashval_t > +uid_decl_hasher::hash (const value_type *item) > +{ > + return item->decl_minimal.uid; > +} > + > +/* Return true if the DECL_UID in both trees are equal. */ > + > +inline bool > +uid_decl_hasher::equal (const value_type *a, const compare_type *b) > +{ > + return (a->decl_minimal.uid == b->decl_minimal.uid); > +} > + ISTR other places where we hash decls. I wonder if we shouldn't move this to some common spot. Maybe later. The patch is OK. Diego.
On 12/12/12, Diego Novillo <dnovillo@google.com> wrote: > On Dec 11, 2012 Lawrence Crowl <crowl@googlers.com> wrote: > > Convert tree-sra.c'candidates from htab_t to hash_table. > > > > Fold uid_decl_map_hash and uid_decl_map_eq into new struct > > uid_decl_hasher. This change moves the definitions from tree-ssa.c > > into tree-sra.c and removes the declarations from tree-flow.h > > > > Update dependent calls and types to hash_table. > > > > Tested on x86_64. > > > > Okay for branch? > > > > > > Index: gcc/tree-sra.c > > +/* Candidate ashtable helpers. */ > > s/ashtable/hash table/ > > > +struct uid_decl_hasher : typed_noop_remove <tree_node> > > +{ > > + typedef tree_node value_type; > > + typedef tree_node compare_type; > > + static inline hashval_t hash (const value_type *); > > + static inline bool equal (const value_type *, const compare_type *); > > +}; > > + > > +/* Hash a tree in a uid_decl_map. */ > > + > > +inline hashval_t > > +uid_decl_hasher::hash (const value_type *item) > > +{ > > + return item->decl_minimal.uid; > > +} > > + > > +/* Return true if the DECL_UID in both trees are equal. */ > > + > > +inline bool > > +uid_decl_hasher::equal (const value_type *a, const compare_type *b) > > +{ > > + return (a->decl_minimal.uid == b->decl_minimal.uid); > > +} > > ISTR other places where we hash decls. I wonder if we shouldn't > move this to some common spot. Maybe later. Decls are hashed in different ways to match the equality function. There probably are duplicates in there, but I'm sure they aren't all duplicates. > The patch is OK.
On Tue, Dec 11, 2012 at 02:44:41PM -0800, Lawrence Crowl wrote: > +/* Hash a tree in a uid_decl_map. */ > + > +inline hashval_t > +uid_decl_hasher::hash (const value_type *item) > +{ > + return item->decl_minimal.uid; Ugh, why aren't you using DECL_UID here? The fact that DECL_UID is right now defined to (DECL_MINIMAL_CHECK (NODE)->decl_minimal.uid) might change, we really don't want to update thousands of locations should we need to change that. Jakub
On 12/12/12, Jakub Jelinek <jakub@redhat.com> wrote: > On Tue, Dec 11, 2012 at 02:44:41PM -0800, Lawrence Crowl wrote: > > +/* Hash a tree in a uid_decl_map. */ > > + > > +inline hashval_t > > +uid_decl_hasher::hash (const value_type *item) > > +{ > > + return item->decl_minimal.uid; > > Ugh, why aren't you using DECL_UID here? The fact that DECL_UID is > right now defined to (DECL_MINIMAL_CHECK (NODE)->decl_minimal.uid) > might change, we really don't want to update thousands of locations > should we need to change that. That is what the code looked like in its original location. I have resisted making changes not essential to the purpose of the patch. However, if you want me to change it to use DECL_UID, I will.
Hi, On Thu, Dec 13, 2012 at 11:05:49AM -0800, Lawrence Crowl wrote: > On 12/12/12, Jakub Jelinek <jakub@redhat.com> wrote: > > On Tue, Dec 11, 2012 at 02:44:41PM -0800, Lawrence Crowl wrote: > > > +/* Hash a tree in a uid_decl_map. */ > > > + > > > +inline hashval_t > > > +uid_decl_hasher::hash (const value_type *item) > > > +{ > > > + return item->decl_minimal.uid; > > > > Ugh, why aren't you using DECL_UID here? The fact that DECL_UID is > > right now defined to (DECL_MINIMAL_CHECK (NODE)->decl_minimal.uid) > > might change, we really don't want to update thousands of locations > > should we need to change that. > > That is what the code looked like in its original location. I have > resisted making changes not essential to the purpose of the patch. > However, if you want me to change it to use DECL_UID, I will. Yes, I think it ought to be changed. Thanks for the conversion, Martin
On 12/13/12, Martin Jambor <mjambor@suse.cz> wrote: > On Thu, Dec 13, 2012 at 11:05:49AM -0800, Lawrence Crowl wrote: > > On 12/12/12, Jakub Jelinek <jakub@redhat.com> wrote: > > > On Tue, Dec 11, 2012 at 02:44:41PM -0800, Lawrence Crowl wrote: > > > > +/* Hash a tree in a uid_decl_map. */ > > > > + > > > > +inline hashval_t > > > > +uid_decl_hasher::hash (const value_type *item) > > > > +{ > > > > + return item->decl_minimal.uid; > > > > > > Ugh, why aren't you using DECL_UID here? The fact that DECL_UID is > > > right now defined to (DECL_MINIMAL_CHECK (NODE)->decl_minimal.uid) > > > might change, we really don't want to update thousands of locations > > > should we need to change that. > > > > That is what the code looked like in its original location. I have > > resisted making changes not essential to the purpose of the patch. > > However, if you want me to change it to use DECL_UID, I will. > > Yes, I think it ought to be changed. > Thanks for the conversion, It turns out that making the change causes the check to fail. Therefore, I will leave the code alone and let someone more familiar with the code investigate.
Index: gcc/tree-sra.c =================================================================== --- gcc/tree-sra.c (revision 194381) +++ gcc/tree-sra.c (working copy) @@ -74,6 +74,7 @@ along with GCC; see the file COPYING3. #include "config.h" #include "system.h" #include "coretypes.h" +#include "hash-table.h" #include "alloc-pool.h" #include "tm.h" #include "tree.h" @@ -269,18 +270,44 @@ static alloc_pool link_pool; /* Base (tree) -> Vector (vec<access_p> *) map. */ static struct pointer_map_t *base_access_vec; +/* Candidate ashtable helpers. */ + +struct uid_decl_hasher : typed_noop_remove <tree_node> +{ + typedef tree_node value_type; + typedef tree_node compare_type; + static inline hashval_t hash (const value_type *); + static inline bool equal (const value_type *, const compare_type *); +}; + +/* Hash a tree in a uid_decl_map. */ + +inline hashval_t +uid_decl_hasher::hash (const value_type *item) +{ + return item->decl_minimal.uid; +} + +/* Return true if the DECL_UID in both trees are equal. */ + +inline bool +uid_decl_hasher::equal (const value_type *a, const compare_type *b) +{ + return (a->decl_minimal.uid == b->decl_minimal.uid); +} + /* Set of candidates. */ static bitmap candidate_bitmap; -static htab_t candidates; +static hash_table <uid_decl_hasher> candidates; /* For a candidate UID return the candidates decl. */ static inline tree candidate (unsigned uid) { - struct tree_decl_minimal t; - t.uid = uid; - return (tree) htab_find_with_hash (candidates, &t, uid); + tree_node t; + t.decl_minimal.uid = uid; + return candidates.find_with_hash (&t, static_cast <hashval_t> (uid)); } /* Bitmap of candidates which we should try to entirely scalarize away and @@ -611,8 +638,7 @@ static void sra_initialize (void) { candidate_bitmap = BITMAP_ALLOC (NULL); - candidates = htab_create (vec_safe_length (cfun->local_decls) / 2, - uid_decl_map_hash, uid_decl_map_eq, NULL); + candidates.create (vec_safe_length (cfun->local_decls) / 2); should_scalarize_away_bitmap = BITMAP_ALLOC (NULL); cannot_scalarize_away_bitmap = BITMAP_ALLOC (NULL); gcc_obstack_init (&name_obstack); @@ -642,7 +668,7 @@ static void sra_deinitialize (void) { BITMAP_FREE (candidate_bitmap); - htab_delete (candidates); + candidates.dispose (); BITMAP_FREE (should_scalarize_away_bitmap); BITMAP_FREE (cannot_scalarize_away_bitmap); free_alloc_pool (access_pool); @@ -659,9 +685,9 @@ static void disqualify_candidate (tree decl, const char *reason) { if (bitmap_clear_bit (candidate_bitmap, DECL_UID (decl))) - htab_clear_slot (candidates, - htab_find_slot_with_hash (candidates, decl, - DECL_UID (decl), NO_INSERT)); + candidates.clear_slot (candidates.find_slot_with_hash (decl, + DECL_UID (decl), + NO_INSERT)); if (dump_file && (dump_flags & TDF_DETAILS)) { @@ -1687,7 +1713,7 @@ maybe_add_sra_candidate (tree var) { tree type = TREE_TYPE (var); const char *msg; - void **slot; + tree_node **slot; if (!AGGREGATE_TYPE_P (type)) { @@ -1735,8 +1761,8 @@ maybe_add_sra_candidate (tree var) } bitmap_set_bit (candidate_bitmap, DECL_UID (var)); - slot = htab_find_slot_with_hash (candidates, var, DECL_UID (var), INSERT); - *slot = (void *) var; + slot = candidates.find_slot_with_hash (var, DECL_UID (var), INSERT); + *slot = var; if (dump_file && (dump_flags & TDF_DETAILS)) { @@ -3589,7 +3615,7 @@ find_param_candidates (void) parm = DECL_CHAIN (parm)) { tree type = TREE_TYPE (parm); - void **slot; + tree_node **slot; count++; @@ -3628,9 +3654,8 @@ find_param_candidates (void) continue; bitmap_set_bit (candidate_bitmap, DECL_UID (parm)); - slot = htab_find_slot_with_hash (candidates, parm, - DECL_UID (parm), INSERT); - *slot = (void *) parm; + slot = candidates.find_slot_with_hash (parm, DECL_UID (parm), INSERT); + *slot = parm; ret = true; if (dump_file && (dump_flags & TDF_DETAILS)) Index: gcc/tree-ssa.c =================================================================== --- gcc/tree-ssa.c (revision 194381) +++ gcc/tree-ssa.c (working copy) @@ -1052,24 +1052,6 @@ err: /* Return true if the DECL_UID in both trees are equal. */ -int -uid_decl_map_eq (const void *va, const void *vb) -{ - const_tree a = (const_tree) va; - const_tree b = (const_tree) vb; - return (a->decl_minimal.uid == b->decl_minimal.uid); -} - -/* Hash a tree in a uid_decl_map. */ - -unsigned int -uid_decl_map_hash (const void *item) -{ - return ((const_tree)item)->decl_minimal.uid; -} - -/* Return true if the DECL_UID in both trees are equal. */ - static int uid_ssaname_map_eq (const void *va, const void *vb) { Index: gcc/tree-flow.h =================================================================== --- gcc/tree-flow.h (revision 194381) +++ gcc/tree-flow.h (working copy) @@ -283,9 +283,6 @@ struct int_tree_map { tree to; }; -extern unsigned int uid_decl_map_hash (const void *); -extern int uid_decl_map_eq (const void *, const void *); - #define num_ssa_names (vec_safe_length (cfun->gimple_df->ssa_names)) #define ssa_name(i) ((*cfun->gimple_df->ssa_names)[(i)]) Index: gcc/Makefile.in =================================================================== --- gcc/Makefile.in (revision 194381) +++ gcc/Makefile.in (working copy) @@ -3034,7 +3034,7 @@ tree-ssa-strlen.o : tree-ssa-strlen.c $( $(TREE_FLOW_H) $(TREE_PASS_H) domwalk.h alloc-pool.h tree-ssa-propagate.h \ $(GIMPLE_PRETTY_PRINT_H) $(PARAMS_H) $(EXPR_H) tree-sra.o : tree-sra.c $(CONFIG_H) $(SYSTEM_H) coretypes.h alloc-pool.h \ - $(TM_H) $(TREE_H) $(GIMPLE_H) $(CGRAPH_H) $(TREE_FLOW_H) \ + $(HASH_TABLE_H) $(TM_H) $(TREE_H) $(GIMPLE_H) $(CGRAPH_H) $(TREE_FLOW_H) \ $(IPA_PROP_H) $(DIAGNOSTIC_H) statistics.h \ $(PARAMS_H) $(TARGET_H) $(FLAGS_H) \ $(DBGCNT_H) $(TREE_INLINE_H) $(GIMPLE_PRETTY_PRINT_H)