This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] DWARF: Don't expand hash table when no insert is needed
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Jason Merrill <jason at redhat dot com>, Cary Coutant <ccoutant at gmail dot com>
- Date: Mon, 17 Dec 2018 05:25:45 -0800
- Subject: Re: [PATCH] DWARF: Don't expand hash table when no insert is needed
- References: <20181216203259.GA22157@gmail.com> <CAFiYyc2yLJiwMRpuOh=H8m7tHCinswSsTxcANb+htttsMqR8oA@mail.gmail.com>
On Mon, Dec 17, 2018 at 2:00 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> 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.
dwarf2out_finish performs:
1. save_macinfo_strings
2. hash table traverse of index_string
3. output_macinfo -> output_macinfo_op
4. output_indirect_strings -> hash table traverse of output_index_string
find_slot_with_hash has
if (insert == INSERT && m_size * 3 <= m_n_elements * 4)
expand ();
which may expand hash table even if no insertion 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 and we shouldn't
expand index string hash table. Otherwise find_slot_with_hash will expand
hash table when hash table has the right size and hash table traverse of
output_index_string will have a different traverse order from index_string.
This is the updated patch I am checking in.
--
H.J.
From 18caa0346c36e4e6a9d556d5a1256f6dc99e8598 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 16 Dec 2018 10:12:01 -0800
Subject: [PATCH] DWARF: Don't expand hash table when no insertion is needed
dwarf2out_finish performs:
1. save_macinfo_strings
2. hash table traverse of index_string
3. output_macinfo -> output_macinfo_op
4. output_indirect_strings -> hash table traverse of output_index_string
find_slot_with_hash has
if (insert == INSERT && m_size * 3 <= m_n_elements * 4)
expand ();
which may expand hash table even if no insertion 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 and we shouldn't
expand index string hash table. Otherwise find_slot_with_hash will expand
hash table when hash table has the right size and hash table traverse of
output_index_string will have a different traverse order from index_string.
PR debug/79342
* dwarf2out.c (find_AT_string_in_table): Add insert argument
defaulting to INSERT and replace INSERT.
(find_AT_string): Likewise.
(output_macinfo_op): Pass NO_INSERT to find_AT_string.
---
gcc/dwarf2out.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index b2381056991..49d8a0af33b 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,19 @@ output_macinfo_op (macinfo_entry *ref)
break;
case DW_MACRO_define_strp:
case DW_MACRO_undef_strp:
- node = find_AT_string (ref->info);
+ /* NB: dwarf2out_finish performs:
+ 1. save_macinfo_strings
+ 2. hash table traverse of index_string
+ 3. output_macinfo -> output_macinfo_op
+ 4. output_indirect_strings
+ -> hash table traverse of output_index_string
+
+ When output_macinfo_op is called, all index strings have been
+ added to hash table by save_macinfo_strings and we can't pass
+ INSERT to find_slot_with_hash which may expand hash table, even
+ if no insertion is needed, and change hash table traverse order
+ between index_string and output_index_string. */
+ 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