From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 94406 invoked by alias); 18 Dec 2019 15:08:13 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 94391 invoked by uid 89); 18 Dec 2019 15:08:13 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS autolearn=ham version=3.3.1 spammy= X-HELO: nikam.ms.mff.cuni.cz Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 18 Dec 2019 15:08:12 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 453F328226A; Wed, 18 Dec 2019 16:08:10 +0100 (CET) Date: Wed, 18 Dec 2019 15:27:00 -0000 From: Jan Hubicka To: Xiong Hu Luo Cc: gcc-patches@gcc.gnu.org, segher@kernel.crashing.org, wschmidt@linux.ibm.com, guojiufu@linux.ibm.com, linkw@gcc.gnu.org Subject: Re: [PATCH] [RFC] ipa: duplicate ipa_size_summary for cloned nodes Message-ID: <20191218150810.vs3omzjyfwatcyg6@kam.mff.cuni.cz> References: <20191218034116.100185-1-luoxhu@linux.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191218034116.100185-1-luoxhu@linux.ibm.com> User-Agent: NeoMutt/20170113 (1.7.2) X-SW-Source: 2019-12/txt/msg01314.txt.bz2 > The size_info of ipa_size_summary are created by r277424. It should be > duplicated for cloned nodes, otherwise self_size and estimated_self_stack_size > would be 0, causing param large-function-insns and large-function-growth working > inaccurate when ipa-inline. > > gcc/ChangeLog: > > 2019-12-18 Luo Xiong Hu > > * ipa-fnsummary.c (ipa_fn_summary_t::duplicate): Copy > ipa_size_summary for cloned nodes. > --- > gcc/ipa-fnsummary.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c > index a46b1445765..9a01be1708b 100644 > --- a/gcc/ipa-fnsummary.c > +++ b/gcc/ipa-fnsummary.c > @@ -868,7 +868,12 @@ ipa_fn_summary_t::duplicate (cgraph_node *src, > } > } > if (!dst->inlined_to) > + { > + class ipa_size_summary *src_size = ipa_size_summaries->get_create (src); > + class ipa_size_summary *dst_size = ipa_size_summaries->get_create (dst); > + *dst_size = *src_size; > ipa_update_overall_fn_summary (dst); > + } Thanks for spotting this! It is quite bad bug. The summaries are supposed to be copied by duplicate method. However it seems that the default duplicate implementation doesn't do the copy (I wonder why) and moreover copy constructor is broken not copying correctly stack use. I think we are fine with the default copy constructor as follows Does it fix your testcase? Index: ipa-fnsummary.h =================================================================== --- ipa-fnsummary.h (revision 279523) +++ ipa-fnsummary.h (working copy) @@ -99,11 +99,6 @@ public: : estimated_self_stack_size (0), self_size (0), size (0) { } - /* Copy constructor. */ - ipa_size_summary (const ipa_size_summary &s) - : estimated_self_stack_size (0), self_size (s.self_size), size (s.size) - { - } }; /* Function inlining information. */ @@ -226,18 +221,20 @@ extern GTY(()) fast_function_summary + public fast_function_summary { public: ipa_size_summary_t (symbol_table *symtab): - fast_function_summary (symtab) {} + fast_function_summary (symtab) + { + disable_insertion_hook (); + } - static ipa_size_summary_t *create_ggc (symbol_table *symtab) + virtual void duplicate (cgraph_node *, cgraph_node *, + ipa_size_summary *src_data, + ipa_size_summary *dst_data) { - class ipa_size_summary_t *summary = new (ggc_alloc ()) - ipa_size_summary_t (symtab); - summary->disable_insertion_hook (); - return summary; + *dst_data = *src_data; } }; extern fast_function_summary Index: ipa-fnsummary.c =================================================================== --- ipa-fnsummary.c (revision 279523) +++ ipa-fnsummary.c (working copy) @@ -672,8 +672,7 @@ static void ipa_fn_summary_alloc (void) { gcc_checking_assert (!ipa_fn_summaries); - ipa_size_summaries = new fast_function_summary - (symtab); + ipa_size_summaries = new ipa_size_summary_t (symtab); ipa_fn_summaries = ipa_fn_summary_t::create_ggc (symtab); ipa_call_summaries = new ipa_call_summary_t (symtab); }