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

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

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
>