[PATCH 2/2] convert tm_restart to hash_table

[Richard Biener]

On Fri, Nov 14, 2014 at 10:12 PM,  <tsaunders@mozilla.com> wrote:
> From: Trevor Saunders <tsaunders@mozilla.com>
>
>
> Hi,
>
> $subject, logically this really is a hash map, but it turns out that doesn't work.  There's a bug in the way elements are inserted into the table at trans-mem.c:3093 which means nothing is ever actually inserted into the table.  I tried converting this to hash_map, and so fixing the bug, but that caused a bunch of test failures related to transactional memory.  SO it seems like someone who knows something about this should either say the hash table is useless, or fix the bug and the test fall out.  For now it seems simplest to just change to hash_table preserving the bug as is so we can move forward with removal of param_is (this is the last user).

Ok.  RTH?

Thanks,
Richard.

> Trev
>
>
> gcc/
>
>         * cfgexpand.c, gimple-ssa.h, trans-mem.c: Replace htab with
>         hash_table.
>
>
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index 2df8ce3..a71dcdb 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -2229,16 +2229,16 @@ static void
>  mark_transaction_restart_calls (gimple stmt)
>  {
>    struct tm_restart_node dummy;
> -  void **slot;
> +  tm_restart_node **slot;
>
>    if (!cfun->gimple_df->tm_restart)
>      return;
>
>    dummy.stmt = stmt;
> -  slot = htab_find_slot (cfun->gimple_df->tm_restart, &dummy, NO_INSERT);
> +  slot = cfun->gimple_df->tm_restart->find_slot (&dummy, NO_INSERT);
>    if (slot)
>      {
> -      struct tm_restart_node *n = (struct tm_restart_node *) *slot;
> +      struct tm_restart_node *n = *slot;
>        tree list = n->label_or_list;
>        rtx_insn *insn;
>
> @@ -6055,10 +6055,7 @@ pass_expand::execute (function *fun)
>
>    /* After expanding, the tm_restart map is no longer needed.  */
>    if (fun->gimple_df->tm_restart)
> -    {
> -      htab_delete (fun->gimple_df->tm_restart);
> -      fun->gimple_df->tm_restart = NULL;
> -    }
> +    fun->gimple_df->tm_restart = NULL;
>
>    /* Tag the blocks with a depth number so that change_scope can find
>       the common parent easily.  */
> diff --git a/gcc/gimple-ssa.h b/gcc/gimple-ssa.h
> index c023956..9bdb233 100644
> --- a/gcc/gimple-ssa.h
> +++ b/gcc/gimple-ssa.h
> @@ -28,11 +28,24 @@ along with GCC; see the file COPYING3.  If not see
>  /* This structure is used to map a gimple statement to a label,
>     or list of labels to represent transaction restart.  */
>
> -struct GTY(()) tm_restart_node {
> +struct GTY((for_user)) tm_restart_node {
>    gimple stmt;
>    tree label_or_list;
>  };
>
> +/* Hasher for tm_restart_node.  */
> +
> +struct tm_restart_hasher : ggc_hasher<tm_restart_node *>
> +{
> +  static hashval_t hash (tm_restart_node *n) { return htab_hash_pointer (n); }
> +
> +  static bool
> +  equal (tm_restart_node *a, tm_restart_node *b)
> +  {
> +    return a == b;
> +  }
> +};
> +
>  struct ssa_name_hasher : ggc_hasher<tree>
>  {
>    /* Hash a tree in a uid_decl_map.  */
> @@ -101,7 +114,7 @@ struct GTY(()) gimple_df {
>
>    /* Map gimple stmt to tree label (or list of labels) for transaction
>       restart and abort.  */
> -  htab_t GTY ((param_is (struct tm_restart_node))) tm_restart;
> +  hash_table<tm_restart_hasher> *tm_restart;
>  };
>
>
> diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c
> index 766b14e..954a661 100644
> --- a/gcc/trans-mem.c
> +++ b/gcc/trans-mem.c
> @@ -3083,15 +3083,16 @@ split_bb_make_tm_edge (gimple stmt, basic_block dest_bb,
>
>    // Record the need for the edge for the benefit of the rtl passes.
>    if (cfun->gimple_df->tm_restart == NULL)
> -    cfun->gimple_df->tm_restart = htab_create_ggc (31, struct_ptr_hash,
> -                                                  struct_ptr_eq, ggc_free);
> +    cfun->gimple_df->tm_restart
> +      = hash_table<tm_restart_hasher>::create_ggc (31);
>
>    struct tm_restart_node dummy;
>    dummy.stmt = stmt;
>    dummy.label_or_list = gimple_block_label (dest_bb);
>
> -  void **slot = htab_find_slot (cfun->gimple_df->tm_restart, &dummy, INSERT);
> -  struct tm_restart_node *n = (struct tm_restart_node *) *slot;
> +  tm_restart_node **slot = cfun->gimple_df->tm_restart->find_slot (&dummy,
> +                                                                  INSERT);
> +  struct tm_restart_node *n = *slot;
>    if (n == NULL)
>      {
>        n = ggc_alloc<tm_restart_node> ();
> @@ -3166,7 +3167,7 @@ expand_block_edges (struct tm_region *const region, basic_block bb)
>
>        if (cfun->gimple_df->tm_restart == NULL)
>         cfun->gimple_df->tm_restart
> -         = htab_create_ggc (31, struct_ptr_hash, struct_ptr_eq, ggc_free);
> +         = hash_table<tm_restart_hasher>::create_ggc (31);
>
>        // All TM builtins have an abnormal edge to the outer-most transaction.
>        // We never restart inner transactions.
> --
> 2.1.3
>