[PR 47382] We cannot simply fold OBJ_TYPE_REF at all in 4.6

Martin Jambor mjambor@suse.cz
Tue Jan 25 14:49:00 GMT 2011


Hi,

PR 47382 is another devirtualization issue but at least this time I
think I did not cause it.  The problem is that even when I reverted
gimple-folding of OBJ_TYPE_REF to what is being done in 4.5, the IL
now looks different and thus we still miscompile stuff.

For the PR testcase, 4.5 early inliner produces:

int main() ()
{
  struct A * this.1;
  int (*__vtbl_ptr_type) (void) * D.1820;
  int (*__vtbl_ptr_type) (void) D.1819;
  struct B b;
  int D.1793;

<bb 2>:
  b.D.1713._vptr.A = &_ZTV1A[2];
  b.D.1713._vptr.A = &_ZTV1B[2];
  D.1793_1 = 0;
  b.D.1713._vptr.A = &_ZTV1B[2];
  this.1_5 = &b.D.1713;
  this.1_5->_vptr.A = &_ZTV1A[2];
  D.1820_6 = this.1_5->_vptr.A;
  D.1819_7 = *D.1820_6;
  OBJ_TYPE_REF(D.1819_7;this.1_5->0) (this.1_5);

<L2>:
<L3>:
  return D.1793_1;

}

Whereas in 4.6 the IL (after disabling O_T_R folding) looks like:

int main() ()
{
  int (*__vtbl_ptr_type) (void) * D.1819;
  int (*__vtbl_ptr_type) (void) D.1818;
  struct B b;
  int D.1794;

<bb 2>:
  MEM[(struct A *)&b]._vptr.A = &_ZTV1A[2];
  b.D.1714._vptr.A = &_ZTV1B[2];
  D.1794_1 = 0;
  b.D.1714._vptr.A = &_ZTV1B[2];
  MEM[(struct A *)&b]._vptr.A = &_ZTV1A[2];
  D.1819_5 = MEM[(struct A *)&b]._vptr.A;
  D.1818_6 = *D.1819_5;
  OBJ_TYPE_REF(D.1818_6;&b->0) (&b);

<L2>:
<L3>:
  return D.1794_1;

}

Notice the subtle but important difference in the 1st (counted from
zero) argument of the OBJ_TYPE_REFs.

O_T_R folding as we've done it so far (since I first looked at the
source) relied on the fact that &DECL means we do not call a virtual
method of an ancestor but of the whole (outer-most) object, which
avoids many problems, including those with changing dynamic type.
However this apparently does not hold true for the current trunk.

Since the optimizations that lead to this seem all valid and useful in
other circumstances, I guess that we simply cannot fold OBJ_TYPE_REFs
without scanning for dynamic type changes at all and so the patch
below disables it.

Nevertheless, we do have a testcase in our testsuite
(g++.dg/opt/devirt1.C), that actually checks that this folding is
being done and so I have xfailed it.  Without LTO, our only chance of
optimizing code like this (with constructor in a different compilation
unit) is to use types and so I have had a look at what my patches I
have not committed do with this source.  Unfortunately, they bail out
when they try to make sure that the destination of the call is not an
complex type of thunk (only this-adjusting thunks are not too complex
to be represented in the call graph now).  This is done using call
graph nodes and since the call target does not have one, I give up.

Is there any way of determining if the function declaration extracted
from BINFO_VIRTUALS is a thunk?  I use TREE_PURPOSE to get the delta
by which to adjust this pointer already, do all thunks have a non-zero
delta in TREE_PURPOSE in BINFO_VIRTUALS?  It would be better to fail
to devirtualize only for thunks (even simple ones) than for all
methods...

Anyway, the patch below successfully bootstrapped and tested on
x86_64-linux.  OK for trunk?

Thanks,

Martin


2011-01-24  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/47382
	* gimple-fold.c (gimple_fold_obj_type_ref_call): Removed.
	(gimple_fold_call): Do not call gimple_fold_obj_type_ref_call.

	* testsuite/g++.dg/torture/pr47382.C: New test.
	* testsuite/g++.dg/opt/devirt1.C: Xfail.

Index: icln/gcc/gimple-fold.c
===================================================================
--- icln.orig/gcc/gimple-fold.c
+++ icln/gcc/gimple-fold.c
@@ -1443,38 +1443,6 @@ gimple_adjust_this_by_delta (gimple_stmt
   gimple_call_set_arg (call_stmt, 0, tmp);
 }
 
-/* Fold a call statement to OBJ_TYPE_REF to a direct call, if possible.  GSI
-   determines the statement, generating new statements is allowed only if
-   INPLACE is false.  Return true iff the statement was changed.  */
-
-static bool
-gimple_fold_obj_type_ref_call (gimple_stmt_iterator *gsi)
-{
-  gimple stmt = gsi_stmt (*gsi);
-  tree ref = gimple_call_fn (stmt);
-  tree obj = OBJ_TYPE_REF_OBJECT (ref);
-  tree binfo, fndecl, delta;
-  HOST_WIDE_INT token;
-
-  if (TREE_CODE (obj) != ADDR_EXPR)
-    return false;
-  obj = TREE_OPERAND (obj, 0);
-  if (!DECL_P (obj)
-      || TREE_CODE (TREE_TYPE (obj)) != RECORD_TYPE)
-    return false;
-  binfo = TYPE_BINFO (TREE_TYPE (obj));
-  if (!binfo)
-    return false;
-
-  token = tree_low_cst (OBJ_TYPE_REF_TOKEN (ref), 1);
-  fndecl = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta, false);
-  if (!fndecl)
-    return false;
-  gcc_assert (integer_zerop (delta));
-  gimple_call_set_fndecl (stmt, fndecl);
-  return true;
-}
-
 /* Attempt to fold a call statement referenced by the statement iterator GSI.
    The statement may be replaced by another statement, e.g., if the call
    simplifies to a constant value. Return true if any changes were made.
@@ -1500,17 +1468,6 @@ gimple_fold_call (gimple_stmt_iterator *
 	  return true;
 	}
     }
-  else
-    {
-      /* ??? Should perhaps do this in fold proper.  However, doing it
-         there requires that we create a new CALL_EXPR, and that requires
-         copying EH region info to the new node.  Easier to just do it
-         here where we can just smash the call operand.  */
-      callee = gimple_call_fn (stmt);
-      if (TREE_CODE (callee) == OBJ_TYPE_REF)
-	return gimple_fold_obj_type_ref_call (gsi);
-    }
-
   return false;
 }
 
Index: icln/gcc/testsuite/g++.dg/torture/pr47382.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/torture/pr47382.C
@@ -0,0 +1,30 @@
+// { dg-do run }
+
+extern "C" void abort ();
+
+struct A
+{
+  inline ~A ();
+  virtual void foo () {}
+};
+
+struct B : A
+{
+  virtual void foo () { abort(); }
+};
+
+static inline void middleman (A *a)
+{
+  a->foo ();
+}
+
+inline A::~A ()
+{
+  middleman (this);
+}
+
+int main ()
+{
+   B b;
+   return 0;
+}
Index: icln/gcc/testsuite/g++.dg/opt/devirt1.C
===================================================================
--- icln.orig/gcc/testsuite/g++.dg/opt/devirt1.C
+++ icln/gcc/testsuite/g++.dg/opt/devirt1.C
@@ -1,6 +1,6 @@
 // { dg-do compile }
 // { dg-options "-O" }
-// { dg-final { scan-assembler "xyzzy" } }
+// { dg-final { scan-assembler "xyzzy" { xfail *-*-* } } }
 
 struct S { S(); virtual void xyzzy(); };
 inline void foo(S *s) { s->xyzzy(); }



More information about the Gcc-patches mailing list