[PATCH] Fix incorrect devirtualization (PR middle-end/48661)

[Richard Guenther]

On Mon, Apr 18, 2011 at 11:40 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> If TREE_BINFO has BV_VCALL_INDEX set, this needs to be dynamically
> adjusted, but none of the callers are prepared to handle that.
> Thus, this patch makes devirtualization give up in those cases.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, trunk and 4.6.
> On the trunk the testcase ICEs before and after the patch in some new callgraph
> checking (added today or so, Honza?), on the branch it works just fine.
>
> Ok for trunk/4.6?

Ok.

Thanks,
Richard.

> 2011-04-18  Jakub Jelinek  <jakub@redhat.com>
>
>        PR middle-end/48661
>        * gimple-fold.c (gimple_get_virt_method_for_binfo): Return NULL
>        if TREE_TYPE (v) is non-NULL.
>
>        * gimple-fold.c (gimple_get_virt_method_for_binfo): Renamed from
>        gimple_get_virt_mehtod_for_binfo.
>        * gimple.h (gimple_get_virt_method_for_binfo): Likewise.
>        * ipa-cp.c (ipcp_process_devirtualization_opportunities): Adjust
>        callers.
>        * ipa-prop.c (try_make_edge_direct_virtual_call): Likewise.
>
>        * g++.dg/torture/pr48661.C: New test.
>
> --- gcc/gimple-fold.c.jj        2011-03-14 14:12:15.000000000 +0100
> +++ gcc/gimple-fold.c   2011-04-18 18:35:22.000000000 +0200
> @@ -1374,7 +1374,7 @@ gimple_fold_builtin (gimple stmt)
>    is a thunk (other than a this adjustment which is dealt with by DELTA). */
>
>  tree
> -gimple_get_virt_mehtod_for_binfo (HOST_WIDE_INT token, tree known_binfo,
> +gimple_get_virt_method_for_binfo (HOST_WIDE_INT token, tree known_binfo,
>                                  tree *delta, bool refuse_thunks)
>  {
>   HOST_WIDE_INT i;
> @@ -1393,6 +1393,10 @@ gimple_get_virt_mehtod_for_binfo (HOST_W
>       v = TREE_CHAIN (v);
>     }
>
> +  /* If BV_VCALL_INDEX is non-NULL, give up.  */
> +  if (TREE_TYPE (v))
> +    return NULL_TREE;
> +
>   fndecl = TREE_VALUE (v);
>   node = cgraph_get_node_or_alias (fndecl);
>   if (refuse_thunks
> --- gcc/gimple.h.jj     2011-03-14 14:12:15.000000000 +0100
> +++ gcc/gimple.h        2011-04-18 18:35:40.000000000 +0200
> @@ -892,7 +892,7 @@ unsigned get_gimple_rhs_num_ops (enum tr
>  gimple gimple_alloc_stat (enum gimple_code, unsigned MEM_STAT_DECL);
>  const char *gimple_decl_printable_name (tree, int);
>  bool gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace);
> -tree gimple_get_virt_mehtod_for_binfo (HOST_WIDE_INT, tree, tree *, bool);
> +tree gimple_get_virt_method_for_binfo (HOST_WIDE_INT, tree, tree *, bool);
>  void gimple_adjust_this_by_delta (gimple_stmt_iterator *, tree);
>  /* Returns true iff T is a valid GIMPLE statement.  */
>  extern bool is_gimple_stmt (tree);
> --- gcc/ipa-cp.c.jj     2011-04-13 12:39:28.000000000 +0200
> +++ gcc/ipa-cp.c        2011-04-18 18:36:11.000000000 +0200
> @@ -1242,7 +1242,7 @@ ipcp_process_devirtualization_opportunit
>        {
>          tree binfo = VEC_index (tree, info->params[param_index].types, j);
>          tree d;
> -         tree t = gimple_get_virt_mehtod_for_binfo (token, binfo, &d, true);
> +         tree t = gimple_get_virt_method_for_binfo (token, binfo, &d, true);
>
>          if (!t)
>            {
> --- gcc/ipa-prop.c.jj   2011-04-13 12:39:28.000000000 +0200
> +++ gcc/ipa-prop.c      2011-04-18 18:36:30.000000000 +0200
> @@ -1730,7 +1730,7 @@ try_make_edge_direct_virtual_call (struc
>   type = ie->indirect_info->otr_type;
>   binfo = get_binfo_at_offset (binfo, ie->indirect_info->anc_offset, type);
>   if (binfo)
> -    target = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta, true);
> +    target = gimple_get_virt_method_for_binfo (token, binfo, &delta, true);
>   else
>     return NULL;
>
> --- gcc/testsuite/g++.dg/torture/pr48661.C.jj   2011-04-18 18:50:49.000000000 +0200
> +++ gcc/testsuite/g++.dg/torture/pr48661.C      2011-04-18 18:50:11.000000000 +0200
> @@ -0,0 +1,77 @@
> +// PR middle-end/48661
> +// { dg-do run }
> +
> +extern "C" void abort ();
> +
> +__attribute__((noinline))
> +double
> +foo (double x, double y)
> +{
> +  asm volatile ("" : : : "memory");
> +  return x + y;
> +}
> +
> +__attribute__((noinline, noclone))
> +void
> +bar (int x)
> +{
> +  if (x != 123)
> +    abort ();
> +}
> +
> +struct A
> +{
> +  double a1, a2;
> +};
> +
> +struct B
> +{
> +  virtual int m () const = 0 ;
> +};
> +
> +struct C
> +{
> +  virtual ~C () {}
> +};
> +
> +struct D : virtual public B, public C
> +{
> +  explicit D (const A &x) : d(123) { foo (x.a2, x.a1); }
> +  int m () const { return d; }
> +  int d;
> +};
> +
> +struct E
> +{
> +  E () : d(0) {}
> +  virtual void n (const B &x) { d = x.m (); x.m (); x.m (); }
> +  int d;
> +};
> +
> +void
> +test ()
> +{
> +  A a;
> +  a.a1 = 0;
> +  a.a2 = 1;
> +  E p;
> +  D q (a);
> +  const B &b = q;
> +  bar (b.m ());
> +  p.n (b);
> +  bar (p.d);
> +}
> +
> +void
> +baz ()
> +{
> +  A a;
> +  D p2 (a);
> +}
> +
> +int
> +main ()
> +{
> +  test ();
> +  return 0;
> +}
>
>        Jakub
>