Re: [PATCH] DWARF: Don't expand hash table when no insert is needed

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

On Sun, Dec 16, 2018 at 9:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> find_slot_with_hash has
>
>  if (insert == INSERT && m_size * 3 <= m_n_elements * 4)
>     expand ();
>
> which may expand hash table even if no insert is neeed and change hash
> table traverse order.  When output_macinfo_op is called, all index
> strings have been added to hash table by save_macinfo_strings, we
> shouldn't expand index string hash table.  Otherwise find_slot_with_hash
> will expand hash table when hash table has the right size.

So the point is that output_macinfo_op is called from code traversing the
hashtable?  I didn't really manage to quickly spot that.

> Tested on i686 and x86-64.  OK for trunk?

OK if called from htab traverse (please clarify that in the comment you add).

Richard.

>
> Thanks.
>
> H.J.
> ---
>         PR debug/79342
>         * dwarf2out.c (find_AT_string_in_table): Add insert argument
>         and replace INSERT.
>         (find_AT_string): Add insert argument defaulting to INSERT
>         and replace INSERT.
>         (output_macinfo_op): Pass NO_INSERT to find_AT_string.
> ---
>  gcc/dwarf2out.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index b2381056991..2fc88c0c12d 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -4618,12 +4618,13 @@ indirect_string_hasher::equal (indirect_string_node *x1, const char *x2)
>
>  static struct indirect_string_node *
>  find_AT_string_in_table (const char *str,
> -                        hash_table<indirect_string_hasher> *table)
> +                        hash_table<indirect_string_hasher> *table,
> +                        enum insert_option insert = INSERT)
>  {
>    struct indirect_string_node *node;
>
>    indirect_string_node **slot
> -    = table->find_slot_with_hash (str, htab_hash_string (str), INSERT);
> +    = table->find_slot_with_hash (str, htab_hash_string (str), insert);
>    if (*slot == NULL)
>      {
>        node = ggc_cleared_alloc<indirect_string_node> ();
> @@ -4640,12 +4641,12 @@ find_AT_string_in_table (const char *str,
>  /* Add STR to the indirect string hash table.  */
>
>  static struct indirect_string_node *
> -find_AT_string (const char *str)
> +find_AT_string (const char *str, enum insert_option insert = INSERT)
>  {
>    if (! debug_str_hash)
>      debug_str_hash = hash_table<indirect_string_hasher>::create_ggc (10);
>
> -  return find_AT_string_in_table (str, debug_str_hash);
> +  return find_AT_string_in_table (str, debug_str_hash, insert);
>  }
>
>  /* Add a string attribute value to a DIE.  */
> @@ -28095,7 +28096,12 @@ output_macinfo_op (macinfo_entry *ref)
>        break;
>      case DW_MACRO_define_strp:
>      case DW_MACRO_undef_strp:
> -      node = find_AT_string (ref->info);
> +      /* NB: output_macinfo_op is called after save_macinfo_strings.
> +        All index strings have been added to hash table at this point.
> +        We can't pass INSERT to find_slot_with_hash which may expand
> +        hash table even if no insert is neeed and change hash table
> +        traverse order.  */
> +      node = find_AT_string (ref->info, NO_INSERT);
>        gcc_assert (node
>                   && (node->form == DW_FORM_strp
>                       || node->form == dwarf_FORM (DW_FORM_strx)));
> --
> 2.19.2
>