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
