Re: [PATCH] Fix for PR63766 (handle removed functions in do_per_function_toporder)

[Richard Biener <richard dot guenther at gmail dot com>]

On Mon, Nov 10, 2014 at 4:48 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> Hi,
>
> Here is a fix for PR63766.  Currently all functions are transformed into SSA before local optimizations and it allows function to be inlined and removed before it goes through local optimzations.  But this requires removal of these functions from working queue.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-11-10  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         * passes.c (remove_cgraph_node_from_order): New.
>         (do_per_function_toporder): Register cgraph removal
>         hook.
>
> gcc/testsuite/
>
> 2014-11-10  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         * g++.dg/pr63766.C: New.
>
>
> diff --git a/gcc/passes.c b/gcc/passes.c
> index 5e91a79..b6a0b0c 100644
> --- a/gcc/passes.c
> +++ b/gcc/passes.c
> @@ -1609,6 +1609,24 @@ do_per_function (void (*callback) (function *, void *data), void *data)
>  static int nnodes;
>  static GTY ((length ("nnodes"))) cgraph_node **order;
>
> +/* Hook called when NODE is removed and therefore should be
> +   excluded from order vector.  DATA is an array of integers.
> +   DATA[0] holds max index it may be accessed by.  For cgraph
> +   node DATA[node->uid + 1] holds index of this node in order
> +   vector.  */
> +static void
> +remove_cgraph_node_from_order (cgraph_node *node, void *data)
> +{
> +  int *order_idx = (int *)data;
> +
> +  if (node->uid >= order_idx[0])
> +    return;
> +
> +  int idx = order_idx[node->uid + 1];
> +  if (idx >= 0 && idx < nnodes && order[idx] == node)
> +    order[idx] = NULL;
> +}
> +
>  /* 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 +1640,29 @@ do_per_function_toporder (void (*callback) (function *, void *data), void *data)
>      callback (cfun, data);
>    else
>      {
> +      cgraph_node_hook_list *hook;
> +      int *order_idx;
>        gcc_assert (!order);
>        order = ggc_vec_alloc<cgraph_node *> (symtab->cgraph_count);
> +
> +      order_idx = (int *)xmalloc (sizeof(int) * (symtab->cgraph_max_uid + 1));

XALLOCAVEC (int, symtab->graph_max_uid + 1)

Ok with that change.

Thanks,
Richard.

> +      memset (order_idx + 1, -1, sizeof (int) * symtab->cgraph_max_uid);
> +      order_idx[0] = symtab->cgraph_max_uid;
> +
>        nnodes = ipa_reverse_postorder (order);
>        for (i = nnodes - 1; i >= 0; i--)
> -        order[i]->process = 1;
> +       {
> +         order[i]->process = 1;
> +         order_idx[order[i]->uid + 1] = i;
> +       }
> +      hook = symtab->add_cgraph_removal_hook (remove_cgraph_node_from_order,
> +                                             order_idx);
>        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 +1671,8 @@ 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);
> +      free (order_idx);
>      }
>    ggc_free (order);
>    order = NULL;
> diff --git a/gcc/testsuite/g++.dg/pr63766.C b/gcc/testsuite/g++.dg/pr63766.C
> new file mode 100644
> index 0000000..1414fbe
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr63766.C
> @@ -0,0 +1,48 @@
> +/* { dg-do compile } */
> +/* { dg-options "-std=c++11 -O2" } */
> +
> +class A
> +{
> + public:
> +  void
> +    getValueType ()
> +  {
> +  }
> +  void getTypeClass ();
> +};
> +template <typename ImplClass> class B
> +{
> + public:
> +  void
> +    Visit (A *p1)
> +  {
> +    p1->getTypeClass ();
> +    static_cast<ImplClass *> (0)->VisitAtomicType (0);
> +  }
> +};
> +class C : B<C>
> +{
> +  template <typename Fn>
> +  void
> +    dumpChild (Fn p1)
> +    {
> +      p1 ();
> +    }
> +
> + public:
> +  void dumpTypeAsChild (int);
> +  void
> +    VisitAtomicType (A *p1)
> +  {
> +    p1->getValueType ();
> +    dumpTypeAsChild (0);
> +  }
> +};
> +void
> +C::dumpTypeAsChild (int)
> +{
> +  dumpChild ([=]
> +             {
> +               Visit (0);
> +             });
> +}