From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 53693 invoked by alias); 16 Sep 2016 07:25:37 -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 53677 invoked by uid 89); 16 Sep 2016 07:25:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=HTo:D*edu.cn, HTo:D*cn, H*f:sk:339617a, H*i:sk:339617a 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; Fri, 16 Sep 2016 07:25:34 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id BB76F543F38; Fri, 16 Sep 2016 09:25:31 +0200 (CEST) Date: Fri, 16 Sep 2016 08:09:00 -0000 From: Jan Hubicka To: "Yuan, Pengfei" Cc: Richard Biener , GCC Patches , Jan Hubicka Subject: Re: [PATCH, 5.x/6.x/7.x] Be more conservative in early inliner if FDO is enabled Message-ID: <20160916072531.GB69806@kam.mff.cuni.cz> References: <58f49a76.4c20.15712b20e40.Coremail.ypf@pku.edu.cn> <392727ce.b9c7.1572b385ece.Coremail.ypf@pku.edu.cn> <339617a6.8f06.1573166cbaf.Coremail.ypf@pku.edu.cn> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <339617a6.8f06.1573166cbaf.Coremail.ypf@pku.edu.cn> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2016-09/txt/msg01004.txt.bz2 > > Here are the results: > > Param Size (GCC5) Time (GCC5) Time (GCC7) > 0 44686265 (-8.26%) 58.772s 66.332s > 1 45692793 (-6.19%) 40.684s 39.220s > 2 45556185 (-6.47%) 35.292s 34.328s > 3 46251049 (-5.05%) 28.820s 27.136s > 4 47028873 (-3.45%) 24.616s 22.200s > 5 47495641 (-2.49%) 20.160s 17.800s > 6 47520153 (-2.44%) 16.444s 15.656s > 14 48708873 5.620s 5.556s Thanks for data! I meant to run the benchmark myself, but had little time to do it over past week becuase of traveling and was also wondering what to do given that spec is rather poor benchmark in this area. Tramp3d is biassed but we are in stage1 and can fine tune latter. I am debugging the libxul crashes in FDO binary now, so we can re-run talos. > > Param: value of PARAM_EARLY_INLINING_INSNS > Size: code size (.text) of optimized libxul.so > Time: execution time of instrumented tramp3d (-n 25) > > To balance between size reduction of optimized binary and speed penalty > of instrumented binary, I set param=6 as baseline and compare: > > Param Size score Time score Total > 0 3.39 -3.57 -0.18 > 1 2.54 -2.47 0.07 > 2 2.65 -2.15 0.50 > 3 2.07 -1.75 0.32 > 4 1.41 -1.50 -0.09 > 5 1.02 -1.23 -0.21 > 6 1.00 -1.00 0.00 > 14 0.00 -0.34 -0.34 > > Therefore, I think param=2 is the best choice. > > Is the attached patch OK? Setting param to 2 looks fine > gcc/ChangeLog > * opts.c (finish_options): Adjust PARAM_EARLY_INLINING_INSNS > when FDO is enabled. > > > diff --git a/gcc/opts.c b/gcc/opts.c > index 39c190d..b59c700 100644 > --- a/gcc/opts.c > +++ b/gcc/opts.c > @@ -826,8 +826,14 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, > maybe_set_param_value (PARAM_STACK_FRAME_GROWTH, 40, > opts->x_param_values, opts_set->x_param_values); > } > > + /* Adjust PARAM_EARLY_INLINING_INSNS when FDO is enabled. */ > + if ((opts->x_profile_arc_flag && !opts->x_flag_test_coverage) > + || (opts->x_flag_branch_probabilities && !opts->x_flag_auto_profile)) > + maybe_set_param_value (PARAM_EARLY_INLINING_INSNS, 2, > + opts->x_param_values, opts_set->x_param_values); > + I would actually preffer to have PARAM_EARLY_ININING_INSNS_FEEDBACK. We already have TRACER_DYNAMIC_COVERAGE_FEEDBACK and other params. The reason is that profile is not a global property of program. It may or may not be available for given function, while params are global. Even at compile time profile may be selectively missing for example for COMDATs that did not win in the linking process. There is also need to update the documentation. Thanks for the work! Honza > if (opts->x_flag_lto) > { > #ifdef ENABLE_LTO > opts->x_flag_generate_lto = 1;