[PATCH, PR 45699] Devirtualize to thunks

[PATCH, PR 45699] Devirtualize to thunks

[Martin Jambor]

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.


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;
+}

Re: [PATCH, PR 45699] Devirtualize to thunks

[Richard Guenther]

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;
> +}
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex

Re: [PATCH, PR 45699] Devirtualize to thunks

[Jan Hubicka]

> 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

Re: [PATCH, PR 45699] Devirtualize to thunks

[Martin Jambor]

Hi,

On Tue, Oct 12, 2010 at 04:22:15PM -0400, Benjamin Redelings I wrote:
> Jan Hubicka 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.
> Thank you!  That's great.  I can now test gcc 4.6 at -O and -O2...
> O3 still gives segfaults.

No worries.  Thanks for testing.

> 
> >>
> >>  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.
> Should I submit a bugzilla bug for this, as is, or do you need a testcase?
> (I'm sure I can get one by whittling down my code, but last time it
> took about 4 hours to do this, so if there's a faster way ... :-P)

Do you mean the segfaults you are experiencing?  Yes, please file a
bug for them.  If not, then we will remember to revisit this
ourselves.

Thanks,

Martin

Re: [PATCH, PR 45699] Devirtualize to thunks

[Benjamin Redelings I]

Jan Hubicka 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.
Thank you!  That's great.  I can now test gcc 4.6 at -O and -O2... O3 
still gives segfaults.

> >
> >  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.
Should I submit a bugzilla bug for this, as is, or do you need a testcase?
(I'm sure I can get one by whittling down my code, but last time it took 
about 4 hours to do this, so if there's a faster way ... :-P)

-BenRI