From patchwork Fri Oct 15 12:13:47 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 67943 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id BDC87B70E7 for ; Fri, 15 Oct 2010 23:14:20 +1100 (EST) Received: (qmail 8268 invoked by alias); 15 Oct 2010 12:14:02 -0000 Received: (qmail 8258 invoked by uid 22791); 15 Oct 2010 12:13:58 -0000 X-SWARE-Spam-Status: No, hits=-6.2 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI X-Spam-Check-By: sourceware.org Received: from cantor.suse.de (HELO mx1.suse.de) (195.135.220.2) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 15 Oct 2010 12:13:51 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.suse.de (Postfix) with ESMTP id 5C15E94746; Fri, 15 Oct 2010 14:13:48 +0200 (CEST) Date: Fri, 15 Oct 2010 14:13:47 +0200 From: Martin Jambor To: GCC Patches Cc: Richard Guenther , Jason Merrill Subject: [PATCH, PR 45875] Fix devirtualization with virtual ancestors Message-ID: <20101015121347.GA18481@virgil.arch.suse.de> Mail-Followup-To: GCC Patches , Richard Guenther , Jason Merrill MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 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 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" } } */