Jeff Law <l...@redhat.com> writes: > On 06/16/2015 02:55 AM, Richard Sandiford wrote: >> This patch stops pointer_hash from inheriting typed_noop_remove and >> instead creates a new class nofree_ptr_hash that inherits from both. >> It then updates all uses of typed_noop_remove (which are all pointers) >> and pointer_hash so that they use this new class instead. >> >> gcc/ >> * hash-table.h: Update comments. >> * hash-traits.h (pointer_hash): Don't inherit from typed_noop_remove. >> (nofree_ptr_hash): New class. >> * asan.c (asan_mem_ref_hasher): Inherit from nofree_ptr_hash rather >> than typed_noop_remove. Remove redudant typedefs. >> * attribs.c (attribute_hasher): Likewise. >> * cfg.c (bb_copy_hasher): Likewise. >> * cselib.c (cselib_hasher): Likewise. >> * dse.c (invariant_group_base_hasher): Likewise. >> * dwarf2cfi.c (trace_info_hasher): Likewise. >> * dwarf2out.c (macinfo_entry_hasher): Likewise. >> (comdat_type_hasher, loc_list_hasher): Likewise. >> * gcse.c (pre_ldst_expr_hasher): Likewise. >> * genmatch.c (id_base): Likewise. >> * genrecog.c (test_pattern_hasher): Likewise. >> * gimple-ssa-strength-reduction.c (cand_chain_hasher): Likewise. >> * haifa-sched.c (delay_i1_hasher): Likewise. >> * hard-reg-set.h (simplifiable_subregs_hasher): Likewise. >> * ipa-icf.h (congruence_class_group_hash): Likewise. >> * ipa-profile.c (histogram_hash): Likewise. >> * ira-color.c (allocno_hard_regs_hasher): Likewise. >> * lto-streamer.h (string_slot_hasher): Likewise. >> * lto-streamer.c (tree_entry_hasher): Likewise. >> * plugin.c (event_hasher): Likewise. >> * postreload-gcse.c (expr_hasher): Likewise. >> * store-motion.c (st_expr_hasher): Likewise. >> * tree-sra.c (uid_decl_hasher): Likewise. >> * tree-ssa-coalesce.c (coalesce_pair_hasher): Likewise. >> (ssa_name_var_hash): Likewise. >> * tree-ssa-live.c (tree_int_map_hasher): Likewise. >> * tree-ssa-loop-im.c (mem_ref_hasher): Likewise. >> * tree-ssa-pre.c (pre_expr_d): Likewise. >> * tree-ssa-sccvn.c (vn_nary_op_hasher): Likewise. >> * vtable-verify.h (registration_hasher): Likewise. >> * vtable-verify.c (vtbl_map_hasher): Likewise. >> * config/arm/arm.c (libcall_hasher): Likewise. >> * config/i386/winnt.c (wrapped_symbol_hasher): Likewise. >> * config/ia64/ia64.c (bundle_state_hasher): Likewise. >> * config/sol2.c (comdat_entry_hasher): Likewise. >> * fold-const.c (fold): Use nofree_ptr_hash instead of pointer_hash. >> (print_fold_checksum, fold_checksum_tree): Likewise. >> (debug_fold_checksum, fold_build1_stat_loc): Likewise. >> (fold_build2_stat_loc, fold_build3_stat_loc): Likewise. >> (fold_build_call_array_loc): Likewise. >> * tree-ssa-ccp.c (gimple_htab): Likewise. >> * tree-browser.c (tree_upper_hasher): Inherit from nofree_ptr_hash >> rather than pointer_type. >> >> gcc/c/ >> * c-decl.c (detect_field_duplicates_hash): Use nofree_ptr_hash >> instead of pointer_hash. >> (detect_field_duplicates): Likewise. >> >> gcc/cp/ >> * class.c (fixed_type_or_null_ref_ht): Inherit from nofree_ptr_hash >> rather than pointer_hash. >> (fixed_type_or_null): Use nofree_ptr_hash instead of pointer_hash. >> * semantics.c (nrv_data): Likewise. >> * tree.c (verify_stmt_tree_r, verify_stmt_tree): Likewise. >> >> gcc/java/ >> * jcf-io.c (charstar_hash): Inherit from nofree_ptr_hash rather >> than typed_noop_remove. Remove redudant typedefs. >> >> gcc/lto/ >> * lto.c (tree_scc_hasher): Inherit from nofree_ptr_hash rather >> than typed_noop_remove. Remove redudant typedefs. >> >> gcc/objc/ >> * objc-act.c (decl_name_hash): Inherit from nofree_ptr_hash rather >> than typed_noop_remove. Remove redudant typedefs. >> >> libcc1/ >> * plugin.cc (string_hasher): Inherit from nofree_ptr_hash rather >> than typed_noop_remove. Remove redudant typedefs. >> (plugin_context): Use nofree_ptr_hash rather than pointer_hash. >> (plugin_context::mark): Likewise. > So are we allowing multiple inheritance in GCC? It seems like that's > what we've got for nofree_ptr_hash. Is there a better way to achieve > what you're trying to do, or do you think this use ought to fall under > some kind of exception? > > >> Index: gcc/haifa-sched.c >> =================================================================== >> --- gcc/haifa-sched.c 2015-06-16 09:53:47.338092692 +0100 >> +++ gcc/haifa-sched.c 2015-06-16 09:53:47.322092878 +0100 >> @@ -614,9 +614,8 @@ struct delay_pair >> >> /* Helpers for delay hashing. */ >> >> -struct delay_i1_hasher : typed_noop_remove <delay_pair> >> +struct delay_i1_hasher : nofree_ptr_hash <delay_pair> >> { >> - typedef delay_pair *value_type; >> typedef void *compare_type; >> static inline hashval_t hash (const delay_pair *); >> static inline bool equal (const delay_pair *, const void *); > Did you keep compare_type intentionally? Similarly for the changes in > hard-reg-set.h, tree-ssa-loop-im.c, and tree-ssa-sccvn.c. You probably > did, but I just want to be sure.
Yeah. The idea is to keep compare_type if it's different from the type being hashed (value_type) . It's needed by the hash_table implementation to know what type find_with_hash & co. expect. > So I'm holding off on approving this one pending further discussion of > the use of multiple inheritance for nofree_ptr_hash. I thought that might be controversial. :-) My two main defences are: 1) This is multiple inheritance of traits classes, which all just have static member functions, rather than multiple inheritance of data- carrying classes. It's really just a union of two separate groups of functions. 2) This goes away if we move to proper C++ object management for the elements, with constructors and destructors. The remove() would then just be the destructor. I think we want that, but it's a relatively big change, and I think doing this kind of consolidation first will help. Thanks, Richard