From patchwork Fri Jan 18 17:46:30 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 213696 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 DC20B2C007A for ; Sat, 19 Jan 2013 04:46:42 +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=1359136003; 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=ao0NTR6 I+23IQGct+oNFagDgI6w=; b=CCJeCqbdX06fSIzDJ91iSi1+Wage4KF6WVvvqph tVvW4ZfHlgna1qauOm+Ypyos6UNlbBY4+VTI6cUl8KW6cdqn6WrqqI47WmHPmQhl id4KQyKftfK0J4bIt+AIvKBNhoadKTDcinsGdGKpPJz1mk/LgfNGgka9PUSDz3XJ eiOg= 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=uv5LCnk4/b+aIKxgyCj8S1ybq6vuAiNJect9FpvCIOoebuDde+X5CdNfHCLvWi DT/RaDudBEx82yFO0O2A4jyoR82+5o87sXRfmSrp0aKpEbtzGuVn5/bgmoVMruYr qPm7JXZ1Zf/ygomHzmdVFTlqOWDTpY9pDRDnWzzqzUvTg=; Received: (qmail 4893 invoked by alias); 18 Jan 2013 17:46:38 -0000 Received: (qmail 4882 invoked by uid 22791); 18 Jan 2013 17:46:38 -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:46:32 +0000 Received: from relay2.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 5FCE7A3A4A for ; Fri, 18 Jan 2013 18:46:31 +0100 (CET) Date: Fri, 18 Jan 2013 18:46:30 +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: <20130118174630.GC26626@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.7 branch on Monday unless someone objects. It passes bootstrap and testsuite without any issues. 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_or_aliased_p): Return false for virtual functions. * ipa-inline-transform.c (can_remove_node_now_p_1): Never return true for virtual methods. * 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 9cc3690..78e131b 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -2269,6 +2269,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; DECL_STATIC_CONSTRUCTOR (new_node->decl) = 0; DECL_STATIC_DESTRUCTOR (new_node->decl) = 0; new_node->clone.tree_map = tree_map; diff --git a/gcc/cgraph.h b/gcc/cgraph.h index 122a44c..7abfb8a 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -935,6 +935,7 @@ cgraph_only_called_directly_or_aliased_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-transform.c b/gcc/ipa-inline-transform.c index 75b8e9d..ce1bc6e 100644 --- a/gcc/ipa-inline-transform.c +++ b/gcc/ipa-inline-transform.c @@ -95,9 +95,7 @@ can_remove_node_now_p_1 (struct cgraph_node *node) those only after all devirtualizable virtual calls are processed. Lacking may edges in callgraph we just preserve them post inlining. */ - && (!DECL_VIRTUAL_P (node->decl) - || (!DECL_COMDAT (node->decl) - && !DECL_EXTERNAL (node->decl))) + && !DECL_VIRTUAL_P (node->decl) /* During early inlining some unanalyzed cgraph nodes might be in the callgraph and they might reffer the function in question. */ && !cgraph_new_nodes); diff --git a/gcc/ipa.c b/gcc/ipa.c index 388291a..8c2b3bd 100644 --- a/gcc/ipa.c +++ b/gcc/ipa.c @@ -194,9 +194,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)))) { gcc_assert (!node->global.inlined_to); enqueue_cgraph_node (node, &first); 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 (); +}