diff mbox

[PR,45875] Fix devirtualization with virtual ancestors

Message ID 20101015121347.GA18481@virgil.arch.suse.de
State New
Headers show

Commit Message

Martin Jambor Oct. 15, 2010, 12:13 p.m. UTC 
Hi,

not unsurprisingly, things work differently with virtual inheritance
and OBJ_TYPE_REF has to take that into account.  I should have
prepared some testcases with it myself, I relied too much on that it
was covered by what we had n the testcase.

Anyway, this is a fix (together with a testcase).  The mechanism does
not have to be very different, we just need to tread virtual ancestors
like those which are not the first ones (with virtual methods).
Instead of testing BINFO_VIRTUAL_P I decided to look instead on
BINFO_FLAG_2 which the front end calls BINFO_NEW_VTABLE_MARKED and
uses to mark that a BINFO has "its own" table of virtual methods.  In
order to do so, I have moved the definition of BINFO_NEW_VTABLE_MARKED
to tree.h because it is effectively a part of communication between
the front and middle ends now.  There are two other benefits of this
approach, we can assert that whatever BINFO we end up with has this
flag set because it really should, and if for whatever reasons the
front end decides to create their own virtual tables for other
ancestor BINFOs, folding will automatically work with that change too.

On the other hand, if you think this is not a good idea or that it
should have been done differently, I'll be glad to address comments
and suggestions.  Nevertheless, the patch passes bootstrap and
testsuite on x86_64-linux so it is committable if you like it as it
is.

Two other minor remarks: 1) There seem to be two different bugs
reported in the PR, I will look at the other straight away.  2) I
guess that the otr-fold-?.C testcases should be in the tree-ssa
subdirectory rather than directly in g++.dg.  I can move them there as
a followup patch but I decide to add the third testcase to the same
place now.

Thanks,

Martin


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

	PR tree-optimization/45875
	* tree.h (BINFO_FLAG_2): Removed.
	(BINFO_NEW_VTABLE_MARKED): Moved here from cp/cp-tree.h.
	* gimple-fold.c (gimple_get_relevant_ref_binfo): Check whether
	base_binfo has BINFO_NEW_VTABLE_MARKED set.
	(gimple_fold_obj_type_ref_known_binfo): Assert that known_binfo has
	BINFO_NEW_VTABLE_MARKED set.

	* cp/cp-tree.h (BINFO_NEW_VTABLE_MARKED): Moved to tree.h.

	* testsuite/g++.dg/torture/pr45875.C: New test.
	* testsuite/g++.dg/otr-fold-3.C: Likewise
diff mbox

Patch

Index: icln/gcc/cp/cp-tree.h
===================================================================
--- icln.orig/gcc/cp/cp-tree.h
+++ icln/gcc/cp/cp-tree.h
@@ -1699,10 +1699,6 @@  struct GTY((variable_size)) lang_type {
 /* Nonzero means that this class is on a path leading to a new vtable.  */
 #define BINFO_VTABLE_PATH_MARKED(NODE) BINFO_FLAG_1 (NODE)
 
-/* Nonzero means B (a BINFO) has its own vtable.  Any copies will not
-   have this flag set.  */
-#define BINFO_NEW_VTABLE_MARKED(B) (BINFO_FLAG_2 (B))
-
 /* Compare a BINFO_TYPE with another type for equality.  For a binfo,
    this is functionally equivalent to using same_type_p, but
    measurably faster.  At least one of the arguments must be a
Index: icln/gcc/gimple-fold.c
===================================================================
--- icln.orig/gcc/gimple-fold.c
+++ icln/gcc/gimple-fold.c
@@ -1430,7 +1430,9 @@  gimple_get_relevant_ref_binfo (tree ref,
 	    return NULL_TREE;
 
 	  base_binfo = get_first_base_binfo_with_virtuals (binfo);
-	  if (base_binfo && BINFO_TYPE (base_binfo) != TREE_TYPE (field))
+	  if (base_binfo
+	      && (BINFO_TYPE (base_binfo) != TREE_TYPE (field)
+		  || BINFO_NEW_VTABLE_MARKED (base_binfo)))
 	    {
 	      tree d_binfo;
 
@@ -1465,6 +1467,7 @@  gimple_fold_obj_type_ref_known_binfo (HO
   HOST_WIDE_INT i;
   tree v, fndecl, delta;
 
+  gcc_assert (BINFO_NEW_VTABLE_MARKED (known_binfo));
   v = BINFO_VIRTUALS (known_binfo);
   i = 0;
   while (i != token)
Index: icln/gcc/testsuite/g++.dg/torture/pr45875.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/torture/pr45875.C
@@ -0,0 +1,25 @@ 
+// { dg-do compile }
+
+struct c1 {};
+
+struct c10 : c1
+{
+  virtual void foo ();
+};
+
+struct c11 : c10, c1		//  // { dg-warning "" }
+{
+  virtual void f6 ();
+};
+
+struct c28 : virtual c11
+{
+  void f6 ();
+};
+
+void check_c28 ()
+{
+  c28 obj;
+  c11 *ptr = &obj;
+  ptr->f6 ();
+}
Index: icln/gcc/tree.h
===================================================================
--- icln.orig/gcc/tree.h
+++ icln/gcc/tree.h
@@ -2365,7 +2365,6 @@  struct GTY(()) tree_type {
 /* Flags for language dependent use.  */
 #define BINFO_MARKED(NODE) TREE_LANG_FLAG_0(TREE_BINFO_CHECK(NODE))
 #define BINFO_FLAG_1(NODE) TREE_LANG_FLAG_1(TREE_BINFO_CHECK(NODE))
-#define BINFO_FLAG_2(NODE) TREE_LANG_FLAG_2(TREE_BINFO_CHECK(NODE))
 #define BINFO_FLAG_3(NODE) TREE_LANG_FLAG_3(TREE_BINFO_CHECK(NODE))
 #define BINFO_FLAG_4(NODE) TREE_LANG_FLAG_4(TREE_BINFO_CHECK(NODE))
 #define BINFO_FLAG_5(NODE) TREE_LANG_FLAG_5(TREE_BINFO_CHECK(NODE))
@@ -2374,6 +2373,10 @@  struct GTY(()) tree_type {
 /* The actual data type node being inherited in this basetype.  */
 #define BINFO_TYPE(NODE) TREE_TYPE (TREE_BINFO_CHECK(NODE))
 
+/* Nonzero means B (a BINFO) has its own vtable.  Any copies will not
+   have this flag set.  */
+#define BINFO_NEW_VTABLE_MARKED(NODE) TREE_LANG_FLAG_2(TREE_BINFO_CHECK(NODE))
+
 /* The offset where this basetype appears in its containing type.
    BINFO_OFFSET slot holds the offset (in bytes)
    from the base of the complete object to the base of the part of the
Index: icln/gcc/testsuite/g++.dg/otr-fold-3.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/otr-fold-3.C
@@ -0,0 +1,68 @@ 
+/* Verify that virtual calls are folded even when a typecast to an
+   ancestor is involved along the way.  */
+/* { dg-do run } */
+/* { dg-options "-O -fdump-tree-optimized-slim"  } */
+
+extern "C" void abort (void);
+
+class A
+{
+public:
+  int data;
+  virtual int foo (int i);
+};
+
+class A_2 : public A
+{
+public:
+  int data_2;
+  virtual int baz (int i);
+};
+
+class B : public virtual A_2
+{
+public:
+  virtual int bah (int i);
+};
+
+int A::foo (int i)
+{
+  return i + 2;
+}
+
+int A_2::baz (int i)
+{
+  return i * 15;
+}
+
+int B::bah (int i)
+{
+  return i + 10;
+}
+
+int __attribute__ ((noinline,noclone)) get_input(void)
+{
+  return 1;
+}
+
+static inline int middleman_1 (class A *obj, int i)
+{
+  return obj->foo (i);
+}
+
+static inline int middleman_2 (class A *obj, int i)
+{
+  return middleman_1 (obj, i);
+}
+
+int main (int argc, char *argv[])
+{
+  class B b;
+
+  if (middleman_2 (&b, get_input ()) != 3)
+    abort ();
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "= B::.*foo"  "optimized"  } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */