* [PATCH][3/n] dwarf2out refactoring for early (LTO) debug
@ 2015-08-26 11:48 Richard Biener
2016-09-21 8:52 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2015-08-26 11:48 UTC (permalink / raw)
To: gcc-patches; +Cc: jason
The following fixes a GC issue I run into when doing
prune_unused_types_prune early. The issue is that the DIE struct
has a chain_circular marked field (die_sib) which cannot tolerate
spurious extra entries from old removed entries into the circular
chain. Otherwise we fail to properly mark parts of the chain.
Those stray entries are kept live referenced from TYPE_SYMTAB_DIE.
So the following patch that makes sure to clear ->die_sib for
nodes we remove. (these DIEs remaining in TYPE_SYMTAB_DIE also
means we may end up re-using them which is probably not what we
want ... in the original LTO experiment I had a ->removed flag
in the DIE struct and removed DIEs from the cache at cache lookup
time if I hit a removed DIE)
Bootstrapped and tested on x86_64-unknown-linux-gnu, gdb tested there
as well.
Ok for trunk?
Thanks,
Richard.
2015-08-26 Richard Biener <rguenther@suse.de>
* dwarf2out.c (remove_child_with_prev): Clear child->die_sib.
(replace_child): Likewise.
(remove_child_TAG): Adjust.
(move_marked_base_types): Likewise.
(prune_unused_types_prune): Clear die_sib of removed children.
Index: trunk/gcc/dwarf2out.c
===================================================================
--- trunk.orig/gcc/dwarf2out.c 2015-08-26 09:30:54.679185817 +0200
+++ trunk/gcc/dwarf2out.c 2015-08-25 16:54:09.150506037 +0200
@@ -4827,6 +4827,7 @@ remove_child_with_prev (dw_die_ref child
prev->die_sib = child->die_sib;
if (child->die_parent->die_child == child)
child->die_parent->die_child = prev;
+ child->die_sib = NULL;
}
/* Replace OLD_CHILD with NEW_CHILD. PREV must have the property that
@@ -4853,6 +4854,7 @@ replace_child (dw_die_ref old_child, dw_
}
if (old_child->die_parent->die_child == old_child)
old_child->die_parent->die_child = new_child;
+ old_child->die_sib = NULL;
}
/* Move all children from OLD_PARENT to NEW_PARENT. */
@@ -4883,9 +4885,9 @@ remove_child_TAG (dw_die_ref die, enum d
remove_child_with_prev (c, prev);
c->die_parent = NULL;
/* Might have removed every child. */
- if (c == c->die_sib)
+ if (die->die_child == NULL)
return;
- c = c->die_sib;
+ c = prev->die_sib;
}
} while (c != die->die_child);
}
@@ -24565,8 +24590,8 @@ prune_unused_types_prune (dw_die_ref die
c = die->die_child;
do {
- dw_die_ref prev = c;
- for (c = c->die_sib; ! c->die_mark; c = c->die_sib)
+ dw_die_ref prev = c, next;
+ for (c = c->die_sib; ! c->die_mark; c = next)
if (c == die->die_child)
{
/* No marked children between 'prev' and the end of the list. */
@@ -24578,8 +24603,14 @@ prune_unused_types_prune (dw_die_ref die
prev->die_sib = c->die_sib;
die->die_child = prev;
}
+ c->die_sib = NULL;
return;
}
+ else
+ {
+ next = c->die_sib;
+ c->die_sib = NULL;
+ }
if (c != prev->die_sib)
prev->die_sib = c;
@@ -24824,8 +24855,8 @@ move_marked_base_types (void)
remove_child_with_prev (c, prev);
/* As base types got marked, there must be at least
one node other than DW_TAG_base_type. */
- gcc_assert (c != c->die_sib);
- c = c->die_sib;
+ gcc_assert (die->die_child != NULL);
+ c = prev->die_sib;
}
}
while (c != die->die_child);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH][3/n] dwarf2out refactoring for early (LTO) debug
2015-08-26 11:48 [PATCH][3/n] dwarf2out refactoring for early (LTO) debug Richard Biener
@ 2016-09-21 8:52 ` Richard Biener
2016-09-21 16:13 ` Jason Merrill
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2016-09-21 8:52 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, Jason Merrill
On Wed, Aug 26, 2015 at 1:34 PM, Richard Biener <rguenther@suse.de> wrote:
>
> The following fixes a GC issue I run into when doing
> prune_unused_types_prune early. The issue is that the DIE struct
> has a chain_circular marked field (die_sib) which cannot tolerate
> spurious extra entries from old removed entries into the circular
> chain. Otherwise we fail to properly mark parts of the chain.
> Those stray entries are kept live referenced from TYPE_SYMTAB_DIE.
>
> So the following patch that makes sure to clear ->die_sib for
> nodes we remove. (these DIEs remaining in TYPE_SYMTAB_DIE also
> means we may end up re-using them which is probably not what we
> want ... in the original LTO experiment I had a ->removed flag
> in the DIE struct and removed DIEs from the cache at cache lookup
> time if I hit a removed DIE)
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, gdb tested there
> as well.
>
> Ok for trunk?
I am re-testing this now and will commit it as part of the piecewise early LTO
merge.
Richard.
> Thanks,
> Richard.
>
> 2015-08-26 Richard Biener <rguenther@suse.de>
>
> * dwarf2out.c (remove_child_with_prev): Clear child->die_sib.
> (replace_child): Likewise.
> (remove_child_TAG): Adjust.
> (move_marked_base_types): Likewise.
> (prune_unused_types_prune): Clear die_sib of removed children.
>
> Index: trunk/gcc/dwarf2out.c
> ===================================================================
> --- trunk.orig/gcc/dwarf2out.c 2015-08-26 09:30:54.679185817 +0200
> +++ trunk/gcc/dwarf2out.c 2015-08-25 16:54:09.150506037 +0200
> @@ -4827,6 +4827,7 @@ remove_child_with_prev (dw_die_ref child
> prev->die_sib = child->die_sib;
> if (child->die_parent->die_child == child)
> child->die_parent->die_child = prev;
> + child->die_sib = NULL;
> }
>
> /* Replace OLD_CHILD with NEW_CHILD. PREV must have the property that
> @@ -4853,6 +4854,7 @@ replace_child (dw_die_ref old_child, dw_
> }
> if (old_child->die_parent->die_child == old_child)
> old_child->die_parent->die_child = new_child;
> + old_child->die_sib = NULL;
> }
>
> /* Move all children from OLD_PARENT to NEW_PARENT. */
> @@ -4883,9 +4885,9 @@ remove_child_TAG (dw_die_ref die, enum d
> remove_child_with_prev (c, prev);
> c->die_parent = NULL;
> /* Might have removed every child. */
> - if (c == c->die_sib)
> + if (die->die_child == NULL)
> return;
> - c = c->die_sib;
> + c = prev->die_sib;
> }
> } while (c != die->die_child);
> }
> @@ -24565,8 +24590,8 @@ prune_unused_types_prune (dw_die_ref die
>
> c = die->die_child;
> do {
> - dw_die_ref prev = c;
> - for (c = c->die_sib; ! c->die_mark; c = c->die_sib)
> + dw_die_ref prev = c, next;
> + for (c = c->die_sib; ! c->die_mark; c = next)
> if (c == die->die_child)
> {
> /* No marked children between 'prev' and the end of the list. */
> @@ -24578,8 +24603,14 @@ prune_unused_types_prune (dw_die_ref die
> prev->die_sib = c->die_sib;
> die->die_child = prev;
> }
> + c->die_sib = NULL;
> return;
> }
> + else
> + {
> + next = c->die_sib;
> + c->die_sib = NULL;
> + }
>
> if (c != prev->die_sib)
> prev->die_sib = c;
> @@ -24824,8 +24855,8 @@ move_marked_base_types (void)
> remove_child_with_prev (c, prev);
> /* As base types got marked, there must be at least
> one node other than DW_TAG_base_type. */
> - gcc_assert (c != c->die_sib);
> - c = c->die_sib;
> + gcc_assert (die->die_child != NULL);
> + c = prev->die_sib;
> }
> }
> while (c != die->die_child);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH][3/n] dwarf2out refactoring for early (LTO) debug
2016-09-21 8:52 ` Richard Biener
@ 2016-09-21 16:13 ` Jason Merrill
0 siblings, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2016-09-21 16:13 UTC (permalink / raw)
To: Richard Biener; +Cc: Richard Biener, GCC Patches
Looks good.
On Wed, Sep 21, 2016 at 4:37 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Wed, Aug 26, 2015 at 1:34 PM, Richard Biener <rguenther@suse.de> wrote:
>>
>> The following fixes a GC issue I run into when doing
>> prune_unused_types_prune early. The issue is that the DIE struct
>> has a chain_circular marked field (die_sib) which cannot tolerate
>> spurious extra entries from old removed entries into the circular
>> chain. Otherwise we fail to properly mark parts of the chain.
>> Those stray entries are kept live referenced from TYPE_SYMTAB_DIE.
>>
>> So the following patch that makes sure to clear ->die_sib for
>> nodes we remove. (these DIEs remaining in TYPE_SYMTAB_DIE also
>> means we may end up re-using them which is probably not what we
>> want ... in the original LTO experiment I had a ->removed flag
>> in the DIE struct and removed DIEs from the cache at cache lookup
>> time if I hit a removed DIE)
>>
>> Bootstrapped and tested on x86_64-unknown-linux-gnu, gdb tested there
>> as well.
>>
>> Ok for trunk?
>
> I am re-testing this now and will commit it as part of the piecewise early LTO
> merge.
>
> Richard.
>
>> Thanks,
>> Richard.
>>
>> 2015-08-26 Richard Biener <rguenther@suse.de>
>>
>> * dwarf2out.c (remove_child_with_prev): Clear child->die_sib.
>> (replace_child): Likewise.
>> (remove_child_TAG): Adjust.
>> (move_marked_base_types): Likewise.
>> (prune_unused_types_prune): Clear die_sib of removed children.
>>
>> Index: trunk/gcc/dwarf2out.c
>> ===================================================================
>> --- trunk.orig/gcc/dwarf2out.c 2015-08-26 09:30:54.679185817 +0200
>> +++ trunk/gcc/dwarf2out.c 2015-08-25 16:54:09.150506037 +0200
>> @@ -4827,6 +4827,7 @@ remove_child_with_prev (dw_die_ref child
>> prev->die_sib = child->die_sib;
>> if (child->die_parent->die_child == child)
>> child->die_parent->die_child = prev;
>> + child->die_sib = NULL;
>> }
>>
>> /* Replace OLD_CHILD with NEW_CHILD. PREV must have the property that
>> @@ -4853,6 +4854,7 @@ replace_child (dw_die_ref old_child, dw_
>> }
>> if (old_child->die_parent->die_child == old_child)
>> old_child->die_parent->die_child = new_child;
>> + old_child->die_sib = NULL;
>> }
>>
>> /* Move all children from OLD_PARENT to NEW_PARENT. */
>> @@ -4883,9 +4885,9 @@ remove_child_TAG (dw_die_ref die, enum d
>> remove_child_with_prev (c, prev);
>> c->die_parent = NULL;
>> /* Might have removed every child. */
>> - if (c == c->die_sib)
>> + if (die->die_child == NULL)
>> return;
>> - c = c->die_sib;
>> + c = prev->die_sib;
>> }
>> } while (c != die->die_child);
>> }
>> @@ -24565,8 +24590,8 @@ prune_unused_types_prune (dw_die_ref die
>>
>> c = die->die_child;
>> do {
>> - dw_die_ref prev = c;
>> - for (c = c->die_sib; ! c->die_mark; c = c->die_sib)
>> + dw_die_ref prev = c, next;
>> + for (c = c->die_sib; ! c->die_mark; c = next)
>> if (c == die->die_child)
>> {
>> /* No marked children between 'prev' and the end of the list. */
>> @@ -24578,8 +24603,14 @@ prune_unused_types_prune (dw_die_ref die
>> prev->die_sib = c->die_sib;
>> die->die_child = prev;
>> }
>> + c->die_sib = NULL;
>> return;
>> }
>> + else
>> + {
>> + next = c->die_sib;
>> + c->die_sib = NULL;
>> + }
>>
>> if (c != prev->die_sib)
>> prev->die_sib = c;
>> @@ -24824,8 +24855,8 @@ move_marked_base_types (void)
>> remove_child_with_prev (c, prev);
>> /* As base types got marked, there must be at least
>> one node other than DW_TAG_base_type. */
>> - gcc_assert (c != c->die_sib);
>> - c = c->die_sib;
>> + gcc_assert (die->die_child != NULL);
>> + c = prev->die_sib;
>> }
>> }
>> while (c != die->die_child);
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-09-21 15:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-26 11:48 [PATCH][3/n] dwarf2out refactoring for early (LTO) debug Richard Biener
2016-09-21 8:52 ` Richard Biener
2016-09-21 16:13 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).