[PR,45699] Devirtualize to thunks

Hi,

folding of OBJ_TYPE_REFs just takes the function declaration in BINFOs
and puts into the call statement.  Unfortunately BINFOs do not put the
declaration of the proper thunk there and so we might ending up not
adjusting the this pointer like in the testcase below.  On the other
hand, BINFOs do contain the deltas and so the folding code can look up
the right thunk in the call graph if need be.  This is what the patch
below does.

Bootstrapped and tested on x86_64-linux without any issues.  OK for
trunk?

Thanks,

Martin


2010-10-08  Martin Jambor  <mjambor@suse.cz>

	PR middle-end/45699
	* gimple-fold.c (gimple_fold_obj_type_ref_known_binfo): Choose among
	thunks.

	* testsuite/g++.dg/torture/pr45699.C: New test.
	* testsuite/g++.dg/otr-fold-1.C: Adjusted.
	* testsuite/g++.dg/otr-fold-1.C: Likewise.

On Mon, 11 Oct 2010, Martin Jambor wrote:

> Hi,
> 
> folding of OBJ_TYPE_REFs just takes the function declaration in BINFOs
> and puts into the call statement.  Unfortunately BINFOs do not put the
> declaration of the proper thunk there and so we might ending up not
> adjusting the this pointer like in the testcase below.  On the other
> hand, BINFOs do contain the deltas and so the folding code can look up
> the right thunk in the call graph if need be.  This is what the patch
> below does.
> 
> Bootstrapped and tested on x86_64-linux without any issues.  OK for
> trunk?

Ok.

Thanks,
Richard.

> Thanks,
> 
> Martin
> 
> 
> 2010-10-08  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR middle-end/45699
> 	* gimple-fold.c (gimple_fold_obj_type_ref_known_binfo): Choose among
> 	thunks.
> 
> 	* testsuite/g++.dg/torture/pr45699.C: New test.
> 	* testsuite/g++.dg/otr-fold-1.C: Adjusted.
> 	* testsuite/g++.dg/otr-fold-1.C: Likewise.
> 
> 
> Index: icln/gcc/gimple-fold.c
> ===================================================================
> --- icln.orig/gcc/gimple-fold.c
> +++ icln/gcc/gimple-fold.c
> @@ -1463,7 +1463,7 @@ tree
>  gimple_fold_obj_type_ref_known_binfo (HOST_WIDE_INT token, tree known_binfo)
>  {
>    HOST_WIDE_INT i;
> -  tree v, fndecl;
> +  tree v, fndecl, delta;
>  
>    v = BINFO_VIRTUALS (known_binfo);
>    i = 0;
> @@ -1475,6 +1475,25 @@ gimple_fold_obj_type_ref_known_binfo (HO
>      }
>  
>    fndecl = TREE_VALUE (v);
> +  delta = TREE_PURPOSE (v);
> +  gcc_assert (host_integerp (delta, 0));
> +
> +  if (integer_nonzerop (delta))
> +    {
> +      struct cgraph_node *node = cgraph_get_node (fndecl);
> +      HOST_WIDE_INT off = tree_low_cst (delta, 0);
> +
> +      if (!node)
> +        return NULL;
> +      for (node = node->same_body; node; node = node->next)
> +        if (node->thunk.thunk_p && off == node->thunk.fixed_offset)
> +          break;
> +      if (node)
> +        fndecl = node->decl;
> +      else
> +        return NULL;
> +     }
> +
>    /* When cgraph node is missing and function is not public, we cannot
>       devirtualize.  This can happen in WHOPR when the actual method
>       ends up in other partition, because we found devirtualization
> Index: icln/gcc/testsuite/g++.dg/otr-fold-1.C
> ===================================================================
> --- icln.orig/gcc/testsuite/g++.dg/otr-fold-1.C
> +++ icln/gcc/testsuite/g++.dg/otr-fold-1.C
> @@ -72,5 +72,5 @@ int main (int argc, char *argv[])
>    return 0;
>  }
>  
> -/* { dg-final { scan-tree-dump "= B::foo"  "optimized"  } } */
> +/* { dg-final { scan-tree-dump "= B::.*foo"  "optimized"  } } */
>  /* { dg-final { cleanup-tree-dump "optimized" } } */
> Index: icln/gcc/testsuite/g++.dg/otr-fold-2.C
> ===================================================================
> --- icln.orig/gcc/testsuite/g++.dg/otr-fold-2.C
> +++ icln/gcc/testsuite/g++.dg/otr-fold-2.C
> @@ -84,5 +84,5 @@ int main (int argc, char *argv[])
>    return 0;
>  }
>  
> -/* { dg-final { scan-tree-dump "= B::foo"  "optimized"  } } */
> +/* { dg-final { scan-tree-dump "= B::.*foo"  "optimized"  } } */
>  /* { dg-final { cleanup-tree-dump "optimized" } } */
> Index: icln/gcc/testsuite/g++.dg/torture/pr45699.C
> ===================================================================
> --- /dev/null
> +++ icln/gcc/testsuite/g++.dg/torture/pr45699.C
> @@ -0,0 +1,61 @@
> +// { dg-do run }
> +
> +extern "C" void abort ();
> +
> +class A
> +{
> +public:
> +  virtual void foo () {abort();}
> +};
> +
> +class B : public A
> +{
> +public:
> +  int z;
> +  virtual void foo () {abort();}
> +};
> +
> +class C : public A
> +{
> +public:
> +  void *a[32];
> +  unsigned long b;
> +  long c[32];
> +
> +  virtual void foo () {abort();}
> +};
> +
> +class D : public C, public B
> +{
> +public:
> +  D () : C(), B()
> +  {
> +    int i;
> +    for (i = 0; i < 32; i++)
> +      {
> +	a[i] = (void *) 0;
> +	c[i] = 0;
> +      }
> +    b = 0xaaaa;
> +  }
> +
> +  virtual void foo ();
> +};
> +
> +void D::foo()
> +{
> +  if (b != 0xaaaa)
> +    abort();
> +}
> +
> +static inline void bar (B &b)
> +{
> +  b.foo ();
> +}
> +
> +int main()
> +{
> +  D d;
> +  bar (d);
> +  return 0;
> +}
> 
>

> Hi,
> 
> folding of OBJ_TYPE_REFs just takes the function declaration in BINFOs
> and puts into the call statement.  Unfortunately BINFOs do not put the
> declaration of the proper thunk there and so we might ending up not
> adjusting the this pointer like in the testcase below.  On the other
> hand, BINFOs do contain the deltas and so the folding code can look up
> the right thunk in the call graph if need be.  This is what the patch
> below does.
> 
> Bootstrapped and tested on x86_64-linux without any issues.  OK for
> trunk?

I guess we should also add an folder that transforms calls to thunk to call to
the function so inlining and other IPA stuff work?
At the moment i think both ipa-prop and inliner will get direct calls to thunks wrong.

Honza