From patchwork Fri Jan 18 17:47:50 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 213698 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 3A84D2C0118 for ; Sat, 19 Jan 2013 04:48:03 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1359136084; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Date: From:To:Subject:Message-ID:Mail-Followup-To:References: MIME-Version:Content-Type:Content-Disposition:In-Reply-To: User-Agent:Mailing-List:Precedence:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:Sender:Delivered-To; bh=v8rER2p ZhCoJF83B3Mb78NkoWMw=; b=ofdXMn8KlQ0V0AbjAKql3QOjZs6YwoDDo37qYhj OtGYWW3Pz5l/2LzzXvTI7qKZrw456VwH55yS7uvxW7q3TD++HENq2mXQfTu5opoQ R4u47WknQuNWReXa09cCOp3FuNCuZmbAj6vrH3xOKXpS/zXQ5GX5GV14ibu20yOq 862U= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Date:From:To:Subject:Message-ID:Mail-Followup-To:References:MIME-Version:Content-Type:Content-Disposition:In-Reply-To:User-Agent:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=wNRNVjQZbCeA+HgId1tcurBkxTO/p3+QvFv2+FZSso6QzI9JkWaxqZOYl7HvWA oAoP/ZkJJY5ONDFFtkAU8PYwV34MesqnA1g63NZDMB5xNMsVauaP37UyS1WrhRMj 2uTH2g5y5OChKCNLu6UoEb7daLI6UMw1hWe3mcn3z2Tio=; Received: (qmail 5766 invoked by alias); 18 Jan 2013 17:48:00 -0000 Received: (qmail 5758 invoked by uid 22791); 18 Jan 2013 17:47:59 -0000 X-SWARE-Spam-Status: No, hits=-5.4 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_DNSWL_HI X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 18 Jan 2013 17:47:52 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id EFE91A4386 for ; Fri, 18 Jan 2013 18:47:50 +0100 (CET) Date: Fri, 18 Jan 2013 18:47:50 +0100 From: Martin Jambor To: gcc-patches@gcc.gnu.org Subject: Re: [PATCH, PR 55264] Do not remove as unreachable any virtual methods before inlining Message-ID: <20130118174750.GD26626@virgil.suse> Mail-Followup-To: gcc-patches@gcc.gnu.org References: <20130116100127.GA21071@virgil.suse> <20130116112121.GA11708@kam.mff.cuni.cz> <20130116124420.GA26629@kam.mff.cuni.cz> <20130116184224.GA26626@virgil.suse> <20130116212410.GB23309@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20130116212410.GB23309@kam.mff.cuni.cz> User-Agent: Mutt/1.5.21 (2010-09-15) 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, On Wed, Jan 16, 2013 at 10:24:10PM +0100, Jan Hubicka wrote: > > On Wed, Jan 16, 2013 at 01:44:20PM +0100, Jan Hubicka wrote: > > > > Perhaps could you first change cgraph_non_local_node_p_1 and try to check some code > > > > if codegen differs significantly? It should not at all. > > > > ipa-cp is the sole user of this flag in IPA passes, so you should know what it does. > > > > > > Thinking deeper of ipa-cp and local virtuals, I think this is all slipperly. > > > Local means that all calls to the functions are explicit and known. Obviously > > > if function is virutal and new calls may appear by devirtualization, the local > > > flag is bogus. I guess the external functions are the only that may be local > > > and virtual because somewhere there must be a vtable reference, but to play > > > safe, I would suggest marking all virtuals non-local. > > > > > > > Right, as discussed on IRC, the patch below therfore modifies > > cgraph_only_called_directly_or_aliased_p to return false for virtual > > functions (which translates into cleared local flag) and the cloning > > machinery to clear that flag. > > > > Bootstrapped and tested on x86_64-linux without any problems. OK for > > trunk? > OK, thanks! > > Honza Great, thanks. I will therefore also commit the following equivalent to the 4.6 branch on Monday unless someone objects. It passes bootstrap and testsuite without any issues, the differences are very small in both backports (and also compared to the trunk version). Thanks, Martin 2013-01-17 Martin Jambor PR tree-optimizations/55264 * cgraph.c (cgraph_create_virtual_clone): Mark clones as non-virtual. * cgraph.h (cgraph_only_called_directly_p): Return false for virtual functions. * ipa-inline.c (cgraph_clone_inlined_nodes): Do reuse nodes of any virtual function. * ipa.c (cgraph_remove_unreachable_nodes): Never return true for virtual methods before inlining is over. testsuite/ * g++.dg/ipa/pr55264.C: New test. diff --git a/gcc/cgraph.c b/gcc/cgraph.c index d62674a..201e77d 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -2342,6 +2342,7 @@ cgraph_create_virtual_clone (struct cgraph_node *old_node, TREE_PUBLIC (new_node->decl) = 0; DECL_COMDAT (new_node->decl) = 0; DECL_WEAK (new_node->decl) = 0; + DECL_VIRTUAL_P (new_node->decl) = 0; new_node->clone.tree_map = tree_map; new_node->clone.args_to_skip = args_to_skip; FOR_EACH_VEC_ELT (ipa_replace_map_p, tree_map, i, map) diff --git a/gcc/cgraph.h b/gcc/cgraph.h index c83b4d3..bad1bb9 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -912,6 +912,7 @@ cgraph_only_called_directly_p (struct cgraph_node *node) gcc_assert (!node->global.inlined_to); return (!node->needed && !node->address_taken && !node->reachable_from_other_partition + && !DECL_VIRTUAL_P (node->decl) && !DECL_STATIC_CONSTRUCTOR (node->decl) && !DECL_STATIC_DESTRUCTOR (node->decl) && !node->local.externally_visible); diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c index 62e1610..3fc796a 100644 --- a/gcc/ipa-inline.c +++ b/gcc/ipa-inline.c @@ -243,8 +243,7 @@ cgraph_clone_inlined_nodes (struct cgraph_edge *e, bool duplicate, those only after all devirtualizable virtual calls are processed. Lacking may edges in callgraph we just preserve them post inlining. */ - && (!DECL_VIRTUAL_P (e->callee->decl) - || (!DECL_COMDAT (e->callee->decl) && !DECL_EXTERNAL (e->callee->decl))) + && !DECL_VIRTUAL_P (e->callee->decl) /* Don't reuse if more than one function shares a comdat group. If the other function(s) are needed, we need to emit even this function out of line. */ diff --git a/gcc/ipa.c b/gcc/ipa.c index 4955408..ade8706 100644 --- a/gcc/ipa.c +++ b/gcc/ipa.c @@ -235,9 +235,7 @@ cgraph_remove_unreachable_nodes (bool before_inlining_p, FILE *file) if (node->analyzed && !node->global.inlined_to && (!cgraph_can_remove_if_no_direct_calls_and_refs_p (node) /* Keep around virtual functions for possible devirtualization. */ - || (before_inlining_p - && DECL_VIRTUAL_P (node->decl) - && (DECL_COMDAT (node->decl) || DECL_EXTERNAL (node->decl))) + || (before_inlining_p && DECL_VIRTUAL_P (node->decl)) /* Also external functions with address taken are better to stay for indirect inlining. */ || (before_inlining_p diff --git a/gcc/testsuite/g++.dg/ipa/pr55264.C b/gcc/testsuite/g++.dg/ipa/pr55264.C new file mode 100644 index 0000000..cf54d6a --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr55264.C @@ -0,0 +1,17 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-early-inlining -fno-weak" } */ + +struct S +{ + S(); + virtual inline void foo () + { + foo(); + } +}; + +void +B () +{ + S().foo (); +}