From patchwork Wed Mar 5 17:19:02 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 327088 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 79D1F2C00B9 for ; Thu, 6 Mar 2014 04:19:13 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:content-type; q=dns; s=default; b=TUMos7CTpOjv7ofQiLo8ZakEHtd/2YxLlq14IawB1jV LNRkouK8BKaOGfRFDoX5XPRPuhbMu+c7e3yBI6YercVDBjCuqspHoA8VYPZOhSIc iNmu15aggTem1w/dyW70rR9b/WuYMKoWkHoepRWOnlxqIsjLEy1n4aDvFjw/fPws = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:content-type; s=default; bh=+dD54z9TJzglQLSZrJBaVrNQC/M=; b=dGOvasb4KhC1Joc8p Od6rQwAf20wyUS8BxvaPzRHfFOSAcSaqBMnHqvW0ohGtbfMveLnf1CkI7Vv4FtBf hnqPXgScwdUST0R4V3GBdRaSVVVt2ROjwq/pfohiViU9xL+NS2pBZEO+1v7l/pC0 mQVvpHxzQOplF6XUSl+NKXFK1E= Received: (qmail 3012 invoked by alias); 5 Mar 2014 17:19:07 -0000 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 Received: (qmail 3000 invoked by uid 89); 5 Mar 2014 17:19:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.0 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 05 Mar 2014 17:19:05 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s25HJ4Q6010734 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 5 Mar 2014 12:19:04 -0500 Received: from [10.10.116.21] ([10.10.116.21]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s25HJ3aX022819; Wed, 5 Mar 2014 12:19:03 -0500 Message-ID: <53175C86.8020806@redhat.com> Date: Wed, 05 Mar 2014 12:19:02 -0500 From: Jason Merrill User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 MIME-Version: 1.0 To: Jan Hubicka CC: gcc-patches List Subject: RFA: New ipa-devirt PATCH for c++/58678 (devirt vs. KDE) This patch fixes the latest 58678 testcase by avoiding speculative devirtualization to the destructor of an abstract class, which can never succeed: you can't create an object of an abstract class, so the pointer must point to an object of a derived class, and the derived class necessarily has its own destructor. Other virtual member functions of an abstract class are OK for devirtualization: the destructor is the only virtual function that is always overridden in every class. We could also detect an abstract class by searching through the vtable for __cxa_pure_virtual, but I figured it was easy enough for the front end to set a flag on the BINFO. Tested x86_64-pc-linux-gnu. OK for trunk? commit b64f52066f3f4cdc9d5a30e2d48aaf6dd5efd3d4 Author: Jason Merrill Date: Wed Mar 5 11:35:07 2014 -0500 PR c++/58678 gcc/ * tree.h (BINFO_ABSTRACT_P): New. * ipa-devirt.c (abstract_class_dtor_p): New. (likely_target_p): Check it. gcc/cp/ * search.c (get_pure_virtuals): Set BINFO_ABSTRACT_P. * tree.c (copy_binfo): Copy it. diff --git a/gcc/cp/search.c b/gcc/cp/search.c index c3eed90..7a3ea4b 100644 --- a/gcc/cp/search.c +++ b/gcc/cp/search.c @@ -2115,6 +2115,8 @@ get_pure_virtuals (tree type) which it is a primary base will contain vtable entries for the pure virtuals in the base class. */ dfs_walk_once (TYPE_BINFO (type), NULL, dfs_get_pure_virtuals, type); + if (CLASSTYPE_PURE_VIRTUALS (type)) + BINFO_ABSTRACT_P (TYPE_BINFO (type)) = true; } /* Debug info for C++ classes can get very large; try to avoid diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index 5567253..3836e16 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -1554,6 +1554,7 @@ copy_binfo (tree binfo, tree type, tree t, tree *igo_prev, int virt) BINFO_OFFSET (new_binfo) = BINFO_OFFSET (binfo); BINFO_VIRTUALS (new_binfo) = BINFO_VIRTUALS (binfo); + BINFO_ABSTRACT_P (new_binfo) = BINFO_ABSTRACT_P (binfo); /* We do not need to copy the accesses, as they are read only. */ BINFO_BASE_ACCESSES (new_binfo) = BINFO_BASE_ACCESSES (binfo); diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c index 2f84f17..3f4a1d5 100644 --- a/gcc/ipa-devirt.c +++ b/gcc/ipa-devirt.c @@ -1674,6 +1674,19 @@ update_type_inheritance_graph (void) timevar_pop (TV_IPA_INHERITANCE); } +/* A destructor for an abstract class is not likely because it can never be + called through the vtable; any actual object will have a derived type, + which will have its own destructor. */ + +static bool +abstract_class_dtor_p (tree fn) +{ + if (!DECL_CXX_DESTRUCTOR_P (fn)) + return false; + tree type = DECL_CONTEXT (fn); + tree binfo = TYPE_BINFO (type); + return BINFO_ABSTRACT_P (binfo); +} /* Return true if N looks like likely target of a polymorphic call. Rule out cxa_pure_virtual, noreturns, function declared cold and @@ -1694,6 +1707,8 @@ likely_target_p (struct cgraph_node *n) return false; if (n->frequency < NODE_FREQUENCY_NORMAL) return false; + if (abstract_class_dtor_p (n->decl)) + return false; return true; } diff --git a/gcc/testsuite/g++.dg/ipa/devirt-30.C b/gcc/testsuite/g++.dg/ipa/devirt-30.C new file mode 100644 index 0000000..c4ac694 --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/devirt-30.C @@ -0,0 +1,25 @@ +// PR c++/58678 +// { dg-options "-O3 -fdump-ipa-devirt" } + +// We shouldn't speculatively devirtualize to ~B because B is an abstract +// class; any actual object passed to f will be of some derived class which +// has its own destructor. + +struct A +{ + virtual void f() = 0; + virtual ~A(); +}; + +struct B : A +{ + virtual ~B() {} +}; + +void f(B* b) +{ + delete b; +} + +// { dg-final { scan-ipa-dump-not "Speculatively devirtualizing" "devirt" } } +// { dg-final { cleanup-ipa-dump "devirt" } } diff --git a/gcc/tree-core.h b/gcc/tree-core.h index e548a0d..708a8ab 100644 --- a/gcc/tree-core.h +++ b/gcc/tree-core.h @@ -786,6 +786,9 @@ struct GTY(()) tree_base { PREDICT_EXPR_OUTCOME in PREDICT_EXPR + BINFO_ABSTRACT_P in + TREE_BINFO + static_flag: TREE_STATIC in diff --git a/gcc/tree.h b/gcc/tree.h index 0dc8d0d..01e23ec 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -1804,6 +1804,9 @@ extern void protected_set_expr_location (tree, location_t); /* Nonzero means that the derivation chain is via a `virtual' declaration. */ #define BINFO_VIRTUAL_P(NODE) (TREE_BINFO_CHECK (NODE)->base.static_flag) +/* Nonzero means that the base is an abstract class. */ +#define BINFO_ABSTRACT_P(NODE) (TREE_BINFO_CHECK (NODE)->base.addressable_flag) + /* 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))