| Message ID | ri6y2w7g9lp.fsf@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | ipa: Prevent materialization of clones with removed bodies (PR 92109) | expand |
> Hi, > > this bug is an interesting one. The immediate cause of the ICE is that > IPA-SRA modification phase baked into clone materialization was > expecting to see a body already changed by IPA-CP while it was actually > looking at the original function. > > That was because we were materializing the (offline) clone in an ltrans > from which it was called but where it did not belonged at all. > remove_unreachable_nodes realized this and wanted to keep just the > declaration and remove the body - and therefor also all traces of the > IPA-CP clone which was necessary just for materialization. But > materialization code still attempted to resurrect the clone. > > Honza proposed immediately removing any node which only needs to have a > declaration from the tree of clones to prevent materialization code from > even considering them which is what the following patch does. > > Unfortunately, I was not able to come up with a small testcase, the bug > is triggered by fragile partitioning decisions. I have bootstrapped, > LTO-bootstrapped and tested the patch on an x86_64-linux, OK for trunk? > > Thanks, > > Martin > > > > 2019-11-21 Martin Jambor <mjambor@suse.cz> > > PR ipa/92109 > * cgraph.h (cgraph_node::remove_from_clone_tree): Declare. > * cgraphclones.c (cgraph_node::remove_from_clone_tree): New method. > (cgraph_materialize_clone): Move removel from clone tree to the > the new method and use it instead. > * ipa.c (symbol_table::remove_unreachable_nodes): When removing > bodies of clones, also remove it from the clone tree. OK, thanks! Honza > --- > gcc/cgraph.h | 4 ++++ > gcc/cgraphclones.c | 35 ++++++++++++++++++++++------------- > gcc/ipa.c | 11 ++++++++--- > 3 files changed, 34 insertions(+), 16 deletions(-) > > diff --git a/gcc/cgraph.h b/gcc/cgraph.h > index a4f14743f00..0d2442c997c 100644 > --- a/gcc/cgraph.h > +++ b/gcc/cgraph.h > @@ -967,6 +967,10 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : public symtab_node > ipa_param_adjustments *param_adjustments, > const char * suffix, unsigned num_suffix); > > + /* Remove the node from the tree of virtual and inline clones and make it a > + standalone node - not a clone any more. */ > + void remove_from_clone_tree (); > + > /* cgraph node being removed from symbol table; see if its entry can be > replaced by other inline clone. */ > cgraph_node *find_replacement (void); > diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c > index bfcebb20495..ac5c57a47aa 100644 > --- a/gcc/cgraphclones.c > +++ b/gcc/cgraphclones.c > @@ -1013,6 +1013,22 @@ cgraph_node::create_version_clone_with_body > return new_version_node; > } > > +/* Remove the node from the tree of virtual and inline clones and make it a > + standalone node - not a clone any more. */ > + > +void cgraph_node::remove_from_clone_tree () > +{ > + if (next_sibling_clone) > + next_sibling_clone->prev_sibling_clone = prev_sibling_clone; > + if (prev_sibling_clone) > + prev_sibling_clone->next_sibling_clone = next_sibling_clone; > + else > + clone_of->clones = next_sibling_clone; > + next_sibling_clone = NULL; > + prev_sibling_clone = NULL; > + clone_of = NULL; > +} > + > /* Given virtual clone, turn it into actual clone. */ > > static void > @@ -1033,22 +1049,15 @@ cgraph_materialize_clone (cgraph_node *node) > dump_function_to_file (node->decl, symtab->dump_file, dump_flags); > } > > + cgraph_node *clone_of = node->clone_of; > /* Function is no longer clone. */ > - if (node->next_sibling_clone) > - node->next_sibling_clone->prev_sibling_clone = node->prev_sibling_clone; > - if (node->prev_sibling_clone) > - node->prev_sibling_clone->next_sibling_clone = node->next_sibling_clone; > - else > - node->clone_of->clones = node->next_sibling_clone; > - node->next_sibling_clone = NULL; > - node->prev_sibling_clone = NULL; > - if (!node->clone_of->analyzed && !node->clone_of->clones) > + node->remove_from_clone_tree (); > + if (!clone_of->analyzed && !clone_of->clones) > { > - node->clone_of->release_body (); > - node->clone_of->remove_callees (); > - node->clone_of->remove_all_references (); > + clone_of->release_body (); > + clone_of->remove_callees (); > + clone_of->remove_all_references (); > } > - node->clone_of = NULL; > bitmap_obstack_release (NULL); > } > > diff --git a/gcc/ipa.c b/gcc/ipa.c > index 0c92980db46..2404024d722 100644 > --- a/gcc/ipa.c > +++ b/gcc/ipa.c > @@ -520,9 +520,14 @@ symbol_table::remove_unreachable_nodes (FILE *file) > reliably. */ > if (node->alias || node->thunk.thunk_p) > ; > - else if (!body_needed_for_clonning.contains (node->decl) > - && !node->alias && !node->thunk.thunk_p) > - node->release_body (); > + else if (!body_needed_for_clonning.contains (node->decl)) > + { > + /* Make the node a non-clone so that we do not attempt to > + materialize it later. */ > + if (node->clone_of) > + node->remove_from_clone_tree (); > + node->release_body (); > + } > else if (!node->clone_of) > gcc_assert (in_lto_p || DECL_RESULT (node->decl)); > if (node->definition && !node->alias && !node->thunk.thunk_p) > -- > 2.23.0 >
diff --git a/gcc/cgraph.h b/gcc/cgraph.h index a4f14743f00..0d2442c997c 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -967,6 +967,10 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : public symtab_node ipa_param_adjustments *param_adjustments, const char * suffix, unsigned num_suffix); + /* Remove the node from the tree of virtual and inline clones and make it a + standalone node - not a clone any more. */ + void remove_from_clone_tree (); + /* cgraph node being removed from symbol table; see if its entry can be replaced by other inline clone. */ cgraph_node *find_replacement (void); diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c index bfcebb20495..ac5c57a47aa 100644 --- a/gcc/cgraphclones.c +++ b/gcc/cgraphclones.c @@ -1013,6 +1013,22 @@ cgraph_node::create_version_clone_with_body return new_version_node; } +/* Remove the node from the tree of virtual and inline clones and make it a + standalone node - not a clone any more. */ + +void cgraph_node::remove_from_clone_tree () +{ + if (next_sibling_clone) + next_sibling_clone->prev_sibling_clone = prev_sibling_clone; + if (prev_sibling_clone) + prev_sibling_clone->next_sibling_clone = next_sibling_clone; + else + clone_of->clones = next_sibling_clone; + next_sibling_clone = NULL; + prev_sibling_clone = NULL; + clone_of = NULL; +} + /* Given virtual clone, turn it into actual clone. */ static void @@ -1033,22 +1049,15 @@ cgraph_materialize_clone (cgraph_node *node) dump_function_to_file (node->decl, symtab->dump_file, dump_flags); } + cgraph_node *clone_of = node->clone_of; /* Function is no longer clone. */ - if (node->next_sibling_clone) - node->next_sibling_clone->prev_sibling_clone = node->prev_sibling_clone; - if (node->prev_sibling_clone) - node->prev_sibling_clone->next_sibling_clone = node->next_sibling_clone; - else - node->clone_of->clones = node->next_sibling_clone; - node->next_sibling_clone = NULL; - node->prev_sibling_clone = NULL; - if (!node->clone_of->analyzed && !node->clone_of->clones) + node->remove_from_clone_tree (); + if (!clone_of->analyzed && !clone_of->clones) { - node->clone_of->release_body (); - node->clone_of->remove_callees (); - node->clone_of->remove_all_references (); + clone_of->release_body (); + clone_of->remove_callees (); + clone_of->remove_all_references (); } - node->clone_of = NULL; bitmap_obstack_release (NULL); } diff --git a/gcc/ipa.c b/gcc/ipa.c index 0c92980db46..2404024d722 100644 --- a/gcc/ipa.c +++ b/gcc/ipa.c @@ -520,9 +520,14 @@ symbol_table::remove_unreachable_nodes (FILE *file) reliably. */ if (node->alias || node->thunk.thunk_p) ; - else if (!body_needed_for_clonning.contains (node->decl) - && !node->alias && !node->thunk.thunk_p) - node->release_body (); + else if (!body_needed_for_clonning.contains (node->decl)) + { + /* Make the node a non-clone so that we do not attempt to + materialize it later. */ + if (node->clone_of) + node->remove_from_clone_tree (); + node->release_body (); + } else if (!node->clone_of) gcc_assert (in_lto_p || DECL_RESULT (node->decl)); if (node->definition && !node->alias && !node->thunk.thunk_p)