[Bug middle-end/63766] [5 Regression] ICE: in gimple_predict_edge, at predict.c:578

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63766

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Ilya Enkovich from comment #2)
> Problem caused by the fact that now all function come to local optimizations
> in SSA form.  It affects inline parameters computation and therefore
> inlining order.
> 
> During early SRA we call convert_callers_for_node which recomputes inline
> parameters for functions in SSA form.  Previously it was computed only for
> function already processed by all early local passes.  Now all functions are
> in SSA form and it means we may recompute inline parameters for function not
> yet processed by local optimizations.
> 
> In this test we have function marked as inlinable which is not yet processed
> in do_per_function_toporder called for local_optimization_passes.  It allows
> this function to be inlined and removed before it is actually processed (and
> still sit in order vector).  Another cgraph_node created by SRA is allocated
> at the same slot as removed one and thus the same function is processed
> twice, which causes ICE in profiling pass.
> 
> Solution here would be to either use another condition for inline_parameters
> recomputation or to handle nodes removal in do_per_function_toporder by
> registering proper node removal hook.  Suppose the latter is better because
> allows more early inlining.
> 
> Here is a possible fix (works for reproducer, not fully tested):
> 
> diff --git a/gcc/passes.c b/gcc/passes.c
> index 5e91a79..4799efa 100644
> --- a/gcc/passes.c
> +++ b/gcc/passes.c
> @@ -1609,6 +1609,19 @@ do_per_function (void (*callback) (function *, void
> *data), void *data)
>  static int nnodes;
>  static GTY ((length ("nnodes"))) cgraph_node **order;
> 
> +static void
> +remove_cgraph_node_from_order (cgraph_node *node, void *)
> +{
> +  int i;
> +
> +  for (i = 0; i < nnodes; i++)
> +    if (order[i] == node)
> +      {
> +       order[i] = NULL;
> +       return;
> +      }
> +}

That's quadratic in the number of nodes and thus a no-go.  Why not delay
removing of unreachable nodes instead?  If you go with the above then
you need to change that data-structure used.

>  /* If we are in IPA mode (i.e., current_function_decl is NULL), call
>     function CALLBACK for every function in the call graph.  Otherwise,
>     call CALLBACK on the current function.
> @@ -1622,13 +1635,20 @@ do_per_function_toporder (void (*callback) (function
> *, void *data), void *data)
>      callback (cfun, data);
>    else
>      {
> +      cgraph_node_hook_list *hook;
>        gcc_assert (!order);
>        order = ggc_vec_alloc<cgraph_node *> (symtab->cgraph_count);
>        nnodes = ipa_reverse_postorder (order);
>        for (i = nnodes - 1; i >= 0; i--)
>          order[i]->process = 1;
> +      hook = symtab->add_cgraph_removal_hook (remove_cgraph_node_from_order,
> +                                             NULL);
>        for (i = nnodes - 1; i >= 0; i--)
>         {
> +         /* Function could be inlined and removed as unreachable.  */
> +         if (!order[i])
> +           continue;
> +
>           struct cgraph_node *node = order[i];
> 
>           /* Allow possibly removed nodes to be garbage collected.  */
> @@ -1637,6 +1657,7 @@ do_per_function_toporder (void (*callback) (function
> *, void *data), void *data)
>           if (node->has_gimple_body_p ())
>             callback (DECL_STRUCT_FUNCTION (node->decl), data);
>         }
> +      symtab->remove_cgraph_removal_hook (hook);
>      }
>    ggc_free (order);
>    order = NULL;