> 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  <mjam...@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
> 

Reply via email to