From patchwork Fri Jul 12 01:58:56 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bin Cheng X-Patchwork-Id: 258677 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id B37172C0339 for ; Fri, 12 Jul 2013 12:05:02 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:references:in-reply-to:subject:date:message-id :mime-version:content-type; q=dns; s=default; b=uPR8lETUu3q0zSvQ a/HnKQ0xrPw1pcusA+KGhK4x5JlvINhIBK+FIzbC2WtPioqkTcVTarNQ2rOGiCty wIoILtUF8GMHWILy7xmdw/FkxxNq3xEewiCcqoR10WEGYKk3Jfwpe/MF2DPAJgzQ rbcrplQ8sxjHkzIozFQFhCCoAno= 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:from :to:cc:references:in-reply-to:subject:date:message-id :mime-version:content-type; s=default; bh=DYpA1uuwrdD3AaNmssQ9UZ 1zpTg=; b=iUihtrRLYuUqvTdQTFpS8Ofr4xYZE0q2PA1RZ+Tx4XcpoYwLsPElPl RArtdHaBqxqZGRu/A26++oGrlkr45so41IGWgcYRNDbv2DvIGziqDZYYb9QFsR7Z JBsMDCTtOTEN9SNhp8gfvbN+1D1uItf/VH3tFcOOYmZgqMj/IYhyU= Received: (qmail 26479 invoked by alias); 12 Jul 2013 02:04:55 -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 26467 invoked by uid 89); 12 Jul 2013 02:04:55 -0000 X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_20, KHOP_THREADED, MSGID_MULTIPLE_AT, RCVD_IN_DNSWL_LOW, SPF_PASS, TW_CF autolearn=ham version=3.3.1 Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 12 Jul 2013 02:04:53 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Fri, 12 Jul 2013 03:04:50 +0100 Received: from Binsh02 ([10.1.255.212]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Fri, 12 Jul 2013 03:04:48 +0100 From: "Bin Cheng" To: "'Eric Botcazou'" Cc: References: <1702451.CLWONJt66H@polaris> In-Reply-To: <1702451.CLWONJt66H@polaris> Subject: RE: FW: [PATCH GCC]Relax the probability condition in CE pass when optimizing for code size Date: Fri, 12 Jul 2013 09:58:56 +0800 Message-ID: <00f801ce7ea3$60ff34b0$22fd9e10$@cheng@arm.com> MIME-Version: 1.0 X-MC-Unique: 113071203045003201 X-Virus-Found: No > -----Original Message----- > From: Eric Botcazou [mailto:ebotcazou@adacore.com] > Sent: Wednesday, July 10, 2013 5:06 PM > To: Bin Cheng > Cc: gcc-patches@gcc.gnu.org > Subject: Re: FW: [PATCH GCC]Relax the probability condition in CE pass when > optimizing for code size > > > 2013-03-25 Bin Cheng > > > > * ifcvt.c (ifcvt_after_combine): New static variable. > > (cheap_bb_rtx_cost_p): Set scale to REG_BR_PROB_BASE when optimizing > > for size. > > (rest_of_handle_if_conversion, rest_of_handle_if_after_combine): > > Clear/set the variable ifcvt_after_combine. > > The idea looks sensible. Some remarks: > - add an after_combine parameter to if_convert and set the global from within > this function instead of the pass functions (True, not TRUE, in the comment). > - explain in the comment why you use optimize_function_for_speed_p instead of > the 'speed' variable defined just above in cheap_bb_rtx_cost_p, > - set the 'scale' variable only once in cheap_bb_rtx_cost_p (otherwise this > is gratuitously confusing) and explain in the comment the reasoning for > choosing REG_BR_PROB_BASE in the !speed case (I presume it's to void the > identical scaling applied to the insns of the block). > Thanks Eric, I updated the patch incorporating your comments. Also retested on x86/thumb2 with both normal and Os. Is it OK? Thanks. bin 2013-07-12 Bin Cheng * ifcvt.c (ifcvt_after_combine): New static variable. (cheap_bb_rtx_cost_p): Set scale to REG_BR_PROB_BASE when optimizing for size. (if_convert): New parameter after_combine. Set ifcvt_after_combine. (rest_of_handle_if_conversion, rest_of_handle_if_after_combine, rest_of_handle_if_after_reload): Pass new argument for if_convert. Index: gcc/ifcvt.c =================================================================== --- gcc/ifcvt.c (revision 200774) +++ gcc/ifcvt.c (working copy) @@ -67,6 +67,9 @@ #define NULL_BLOCK ((basic_block) NULL) +/* True if after combine pass. */ +static bool ifcvt_after_combine; + /* # of IF-THEN or IF-THEN-ELSE blocks we looked at */ static int num_possible_if_blocks; @@ -141,11 +144,24 @@ cheap_bb_rtx_cost_p (const_basic_block bb, int sca rtx insn = BB_HEAD (bb); bool speed = optimize_bb_for_speed_p (bb); + /* Set scale to REG_BR_PROB_BASE to void the identical scaling + applied to insn_rtx_cost when optimizing for size. Only do + this after combine because if-conversion might interfer with + passes before combine. + + Use optimize_function_for_speed_p instead of the pre-defined + variable speed to make sure it is set to same value for all + basic blocks in one if-conversion transformation. */ + if (!optimize_function_for_speed_p (cfun) && ifcvt_after_combine) + scale = REG_BR_PROB_BASE; /* Our branch probability/scaling factors are just estimates and don't account for cases where we can get speculation for free and other secondary benefits. So we fudge the scale factor to make speculating - appear a little more profitable. */ - scale += REG_BR_PROB_BASE / 8; + appear a little more profitable when optimizing for performance. */ + else + scale += REG_BR_PROB_BASE / 8; + + max_cost *= scale; while (1) @@ -4337,10 +4353,11 @@ dead_or_predicable (basic_block test_bb, basic_blo return FALSE; } -/* Main entry point for all if-conversion. */ +/* Main entry point for all if-conversion. AFTER_COMBINE is true if + we are after combine pass. */ static void -if_convert (void) +if_convert (bool after_combine) { basic_block bb; int pass; @@ -4351,6 +4368,8 @@ static void df_live_set_all_dirty (); } + /* Record whether we are after combine pass. */ + ifcvt_after_combine = after_combine; num_possible_if_blocks = 0; num_updated_if_blocks = 0; num_true_changes = 0; @@ -4454,7 +4473,7 @@ rest_of_handle_if_conversion (void) dump_flow_info (dump_file, dump_flags); } cleanup_cfg (CLEANUP_EXPENSIVE); - if_convert (); + if_convert (false); } cleanup_cfg (0); @@ -4495,7 +4514,7 @@ gate_handle_if_after_combine (void) static unsigned int rest_of_handle_if_after_combine (void) { - if_convert (); + if_convert (true); return 0; } @@ -4530,7 +4549,7 @@ gate_handle_if_after_reload (void) static unsigned int rest_of_handle_if_after_reload (void) { - if_convert (); + if_convert (true); return 0; }