Re: [patch] Hash table changes from cxx-conversion branch

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Apr 24, 2013 at 9:58 PM, Lawrence Crowl <crowl@googlers.com> wrote:
> On 4/24/13, Lawrence Crowl <crowl@googlers.com> wrote:
>> Updated hash table patch.  This patch does not include the config part,
>> and does not include the entries already approved.
>>
>> The patch is attached due to size.
>
> Trying again.  Patch attached compressed.  Introductory text below.
>
> ===
>
> This patch is a consolodation of the hash_table patches to the
> cxx-conversion branch for files not under gcc/config.
>
> Update various hash tables from htab_t to hash_table.
> Modify types and calls to match.
>
> * tree-parloops.c'reduction
> * tree-parloops.c'name_to_copy
>
> Fold reduction_info_hash and reduction_info_eq into new struct
> reduction_hasher.  Fold name_to_copy_elt_eq and name_to_copy_elt_hash
> into new struct name_to_copy_hasher.
>
> * var-tracking.c'emit_note_data_def.vars
> * var-tracking.c'shared_hash_def.htab
> * var-tracking.c'changed_variables
>
> Fold variable_htab_hash, variable_htab_eq, variable_htab_free
>   into new struct variable_hasher.
> Add typedef variable_table_type.
> Add typedef variable_iterator_type.
>
> * trans-mem.c'tm_log
>
> Fold tm_log_hash, tm_log_eq, tm_log_free into new struct log_entry_hasher.
>
> * trans-mem.c'tm_memopt_value_numbers
>
> Fold tm_memop_hash, tm_memop_eq into new struct tm_memop_hasher.
>
> * tree-ssa-strlen.c'decl_to_stridxlist_htab
>
> Fold decl_to_stridxlist_hash into new struct stridxlist_hasher.
>
> * tree-ssa-loop-ivopts.c'ivopts_data::inv_expr_tab
>
> Fold htab_inv_expr_hash and htab_inv_expr_eq into new struct
> iv_inv_expr_hasher.
>
> * tree-ssa-uncprop.c'equiv
>
> Equiv renamed to val_ssa_equiv because of name ambiguity with local variables.
>
> Fold equiv_hash, equiv_eq and equiv_free into new struct val_ssa_equiv_hasher.
>
> Renamed variables equiv_hash_elt to an_equiv_elt because of name ambiguity
> with struct type.  Changed equiv_hash_elt_p to an_equiv_elt_p to match.
>
> * tree-ssa-phiopt.c'seen_ssa_names
>
> Fold name_to_bb_hash and name_to_bb_eq into new struct ssa_names_hasher.
>
> * tree-ssa-structalias.c'pointer_equiv_class_table
> * tree-ssa-structalias.c'location_equiv_class_table
>
> Fold equiv_class_label_hash and equiv_class_label_eq into new
> struct equiv_class_hasher.
>
> * tree-ssa-structalias.c'shared_bitmap_table
>
> Fold shared_bitmap_hash and shared_bitmap_eq into new struct
> shared_bitmap_hasher.
>
> * tree-ssa-live.c'var_map_base_init::tree_to_index
>
> New struct tree_int_map_hasher.
>
> * tree-sra.c'candidates
>
> 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
>
> tree-browser.c
>
> Remove stale declaration of removed TB_parent_eq.
> Fix template parameter for base class to match value_type.
>
> gimple.h
>
> Use gimplify_hasher::hash rather than gimple_tree_hash in the
>   assertion check.
> Change return values to match return type. (I.e. no conversions.)
>
> * graphite-clast-to-gimple.c'ivs_params::newivs_index
> * graphite-clast-to-gimple.c'ivs_params::params_index
> * graphite-clast-to-gimple.c'print_generated_program::params_index
> * graphite-clast-to-gimple.c'gloog::newivs_index
> * graphite-clast-to-gimple.c'gloog::params_index
> * graphite.c graphite_transform_loops::bb_pbb_mapping
> * sese.c copy_bb_and_scalar_dependences::rename_map
>
> Move hash table declarations to a new graphite-htab.h, because they
> are used in few places.
>
> Remove unused:
>
> htab_t scop::original_pddrs
> SCOP_ORIGINAL_PDDRS
>
> Remove unused:
>
> insert_loop_close_phis
> insert_guard_phis
> debug_ivtype_map
> ivtype_map_elt_info
> new_ivtype_map_elt
>
> * gimplify.c'gimplify_ctx::temp_htab
>
> Move struct gimple_temp_hash_elt and struct gimplify_ctx to a new
> gimplify-ctx.h, because they are used few places.
>
> * cselib.c'cselib_hash_table
> * gcse.c'pre_ldst_table
> * gimple-ssa-strength-reduction.c'base_cand_map
> * haifa-sched.c'delay_htab
> * haifa-sched.c'delay_htab_i2
> * ira-color.c'allocno_hard_regs_htab
> * ira-costs.c'cost_classes_htab
> * loop-invariant.c'merge_identical_invariants::eq
> * loop-iv.c'bivs
> * loop-unroll.c'opt_info::insns_to_split
> * loop-unroll.c'opt_info::insns_with_var_to_expand
> * passes.c'name_to_pass_map
> * plugin.c'event_tab
> * postreload-gcse.c'expr_table
> * store-motion.c'store_motion_mems_table
> * tree-cfg.c'discriminator_per_locus
> * tree-scalar-evolution.c'resolve_mixers::cache
> * tree-ssa-dom.c'avail_exprs
>
> Remove unused:
> dse.c bitmap clear_alias_sets
> dse.c bitmap disqualified_clear_alias_sets
> dse.c alloc_pool clear_alias_mode_pool
> dse.c dse_step2_spill
> dse.c dse_step5_spill
> graphds.h htab_t graph::indices
>
> * attribs.c'scoped_attributes::attribute_hash
> * bitmap.c'bitmap_desc_hash
> * dwarf2cfi.c'trace_index
> * dwarf2out.c'break_out_includes::cu_hash_table
> * dwarf2out.c'copy_decls_for_unworthy_types::decl_table
> * dwarf2out.c'optimize_external_refs::map
> * dwarf2out.c'output_comp_unit::extern_map
> * dwarf2out.c'output_comdat_type_unit::extern_map
> * dwarf2out.c'output_macinfo::macinfo_htab
> * dwarf2out.c'optimize_location_lists::htab
> * dwarf2out.c'dwarf2out_finish::comdat_type_table
> * except.c'ehspec_hash_type
> * except.c'assign_filter_values::ttypes
> * except.c'assign_filter_values::ehspec
> * except.c'sjlj_assign_call_site_values::ar_hash
> * except.c'convert_to_eh_region_ranges::ar_hash
> * trans-mem.c'tm_new_mem_hash
> * tree-browser.c'TB_up_ht
> * tree-eh.c'finally_tree
>
> Move the declaration of hash_table <alloc_pool_hasher> alloc_pool_hash
> in alloc-pool.c to after the method definitions for its parameter
> class.
>
> * ggc-common.c'loc_hash
> * ggc-common.c'ptr_hash
>
> Add a new hash_table method elements_with_deleted to meet the needs of
> gcc-common.c.
>
> Correct many methods with parameter types compare_type to the correct
> value_type.  (Correct code was unlikely to notice the change, but
> incorrect code will.)
>
> * tree-complex.c'complex_variable_components
> * tree-parloops.c'eliminate_local_variables_stmt::decl_address
> * tree-parloops.c'separate_decls_in_region::decl_copies
>
> Move hash table declarations to a new tree-hasher.h, to resolve
> compilation dependences and because they are used in few places.
>
> * lto-streamer.h'output_block::string_hash_table
> * lto-streamer-in.c'file_name_hash_table
> * lto-streamer.c'tree_htab
>
> The struct string_slot moves from data-streamer.h to lto-streamer.h to
> resolve compilation dependences.
>
> Tested on x86_64.

Please send context diffs - the following is an especially awkward example:

-/* Structure for passing some other parameters to function
-   emit_note_insn_var_location.  */
-typedef struct emit_note_data_def
+/* Return true if a decl_or_value DV is a DECL or NULL.  */
+static inline bool
+dv_is_decl_p (decl_or_value dv)
 {
-  /* The instruction which the note will be emitted before/after.  */
-  rtx insn;
+  return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
+}

The changes to the following files are ok (just in case I will not finish review
due to time constraints):

gcc/tree-ssa-loop-ivopts.c

I'll leave the var-tracking.c changes to respective maintainers, maybe
it's a good
idea to send that heavy reorg separately.

gcc/tree-ssa-strlen.c
gcc/loop-iv.c
gcc/trans-mem.c
gcc/lto-streamer.c
gcc/lto-streamer.h
gcc/loop-unroll.c
gcc/tree-parloops.c
gcc/tree-complex.c
gcc/gcse.c
gcc/gimple-ssa-strength-reduction.c
gcc/data-streamer-out.c
gcc/data-streamer.h
gcc/ira-costs.c
gcc/dwarf2cfi.c
gcc/tree-flow.h
gcc/lto-streamer-in.c
gcc/sese.c

-static int
-debug_rename_map_1 (void **slot, void *s ATTRIBUTE_UNUSED)
+int
+debug_rename_map_1 (rename_map_elt_s **slot, void *s ATTRIBUTE_UNUSED)
 {

...
 DEBUG_FUNCTION void
-debug_rename_map (htab_t rename_map)
+debug_rename_map (rename_map_type rename_map)
 {
-  htab_traverse (rename_map, debug_rename_map_1, NULL);
+  rename_map.traverse <void *, debug_rename_map_1> (NULL);
 }

aww ... one reason why I dislike template function arguments ;)
Functors are so much nicer.

gcc/sese.h
gcc/attribs.c
gcc/graphite-htab.h
gcc/tree-ssa-live.c
gcc/tree-scalar-evolution.c
gcc/graphite-dependences.c
gcc/except.c
gcc/alloc-pool.c
gcc/cselib.c
gcc/tree-ssa-phiopt.c
gcc/graphite-poly.h
gcc/gimplify.c
gcc/tree-ssa-dom.c
gcc/graphite.c
gcc/tree-browser.c
gcc/tree-sra.c
gcc/plugin.c
gcc/graphite-clast-to-gimple.c
gcc/graphite-clast-to-gimple.h
gcc/tree-ssa-uncprop.c
gcc/tree-hasher.h  -- I'd expected tree_int, int_tree and uid_decl
hashers to be here btw
gcc/gimple.h
gcc/tree-ssa.c
gcc/ggc-common.c
gcc/passes.c
gcc/lto-streamer-out.c
gcc/tree-cfg.c
gcc/store-motion.c
gcc/ira-color.c
gcc/tree-eh.c
gcc/dwarf2out.c
gcc/haifa-sched.c
gcc/bitmap.c
gcc/postreload-gcse.c
gcc/loop-invariant.c

Thus, the patch is ok apart from the var-tracking.c bits which I defer to
respective maintainers.

Btw, a hint on future such re-orgs - smaller patches are easily reviewed
when going through mail - large ones such as this are queued somewhere
for eventual review (that happens for me).  So I suggest to submit independent
bits independently and you get a high chance of getting very quick approvals.

Thanks,
Richard.

> Lawrence Crowlgcc/tree-eh.c