From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20539 invoked by alias); 3 Jul 2012 09:32:29 -0000 Received: (qmail 20526 invoked by uid 22791); 3 Jul 2012 09:32:28 -0000 X-SWARE-Spam-Status: No, hits=-4.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-ob0-f175.google.com (HELO mail-ob0-f175.google.com) (209.85.214.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 03 Jul 2012 09:32:15 +0000 Received: by obcva7 with SMTP id va7so10431302obc.20 for ; Tue, 03 Jul 2012 02:32:13 -0700 (PDT) MIME-Version: 1.0 Received: by 10.182.116.2 with SMTP id js2mr12062331obb.38.1341307933513; Tue, 03 Jul 2012 02:32:13 -0700 (PDT) Received: by 10.76.82.4 with HTTP; Tue, 3 Jul 2012 02:32:13 -0700 (PDT) In-Reply-To: <4ff2ad57.8aa0d80a.5910.ffffb0d6SMTPIN_ADDED@mx.google.com> References: <4feac6f5.4abd440a.074d.ffffcfbaSMTPIN_ADDED@mx.google.com> <4fec169c.e908b40a.26d1.ffffc3f1SMTPIN_ADDED@mx.google.com> <4ff2ad57.8aa0d80a.5910.ffffb0d6SMTPIN_ADDED@mx.google.com> Date: Tue, 03 Jul 2012 09:32:00 -0000 Message-ID: Subject: Re: [PATCH] Disable loop2_invariant for -Os From: Richard Guenther To: Zhenqiang Chen Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes 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 X-SW-Source: 2012-07/txt/msg00077.txt.bz2 On Tue, Jul 3, 2012 at 10:29 AM, Zhenqiang Chen wr= ote: >>-----Original Message----- >>From: Richard Guenther [mailto:richard.guenther@gmail.com] >>Sent: 2012=E5=B9=B46=E6=9C=8828=E6=97=A5 17:24 >>To: Zhenqiang Chen >>Cc: gcc-patches@gcc.gnu.org >>Subject: Re: [PATCH] Disable loop2_invariant for -Os >> >>On Thu, Jun 28, 2012 at 10:33 AM, Zhenqiang Chen >>wrote: >>>>> diff --git a/gcc/loop-init.c b/gcc/loop-init.c index >>>>> 03f8f61..5d8cf73 >>>>> 100644 >>>>> --- a/gcc/loop-init.c >>>>> +++ b/gcc/loop-init.c >>>>> @@ -273,6 +273,12 @@ struct rtl_opt_pass pass_rtl_loop_done =3D >>>>> static bool >>>>> gate_rtl_move_loop_invariants (void) >>>>> { >>>>> + /* In general, invariant motion can not reduce code size. But it >>>>> + will >>>>> + change the liverange of the invariant, which increases the >>>>> + register >>>>> + pressure and might lead to more spilling. */ >>>>> + if (optimize_function_for_size_p (cfun)) >>>>> + return false; >>>>> + >>>> >>>>Can you do this per loop instead? Using optimize_loop_nest_for_size_p? >>> >>> Update it according to the comments. >>> >>> Thanks! >>> -Zhenqiang >>> >>> diff --git a/gcc/loop-invariant.c b/gcc/loop-invariant.c index >>> f8405dd..b0e84a7 100644 >>> --- a/gcc/loop-invariant.c >>> +++ b/gcc/loop-invariant.c >>> @@ -1931,7 +1931,8 @@ move_loop_invariants (void) >>> curr_loop =3D loop; >>> /* move_single_loop_invariants for very large loops >>> is time consuming and might need a lot of memory. */ >>> - if (loop->num_nodes <=3D (unsigned) >>> LOOP_INVARIANT_MAX_BBS_IN_LOOP) >>> + if (loop->num_nodes <=3D (unsigned) >>> + LOOP_INVARIANT_MAX_BBS_IN_LOOP >>> + && ! optimize_loop_nest_for_size_p (loop)) >>> move_single_loop_invariants (loop); >> >>Wait - move_single_loop_invariants itself already uses >>optimize_loop_for_speed_p. >>And looking down it seems to have support for tracking spill cost (eventu= ally only >>with -fira-loop-pressure) - please work out why this support is not worki= ng for >>you. > > 1) If -fira_loop_pressure is enabled, it reduces ~24% invariant motions i= n my tests. But it does not help on total code size. Seams there is issue t= o update the "regs_needed" after moving an invariant out of the loop (My be= nchmark logs show ~73% cases have more than one invariants moved). > > During tracing, I found that move an integer constant out of the loop doe= s not increase regs_needed. Function "get_pressure_class_and_nregs (rtx ins= n, int *nregs)" computes the "regs_needed". > > *nregs > =3D ira_reg_class_max_nregs[pressure_class][GET_MODE (SET_SRC (set)= )]; > > In ARM, the insn to set an integer is like > (set (reg:SI 183) > (const_int 32 [0x20])) inv1.c:64 182 {*thumb1_movsi_insn} > (nil)) > GET_MODE (SET_SRC (set)) is VOIDMode and ira_reg_class_max_nregs[pressure= _class][VOIDMode] is 0. In one of my test cases, it moves 4 integer constan= ts out of the loop, which leads to spilling. > > According to the algorithm in "calculate_loop_reg_pressure", moving an in= variant out of the loop should impact on the register pressure. So I try to= add the following code > > if (! (*nregs)) > *nregs =3D ira_reg_class_max_nregs[pressure_class][GET_MODE (reg)]; > > Logs show it reduces another 32% invariant motions. But the code size is = still far from disabling the pass. Logs show -fira_loop_pressure impact oth= er passes in addition to loop2_invariant (The result of "-fira_loop_pressur= e -fno-move-loop-invariants" is different from the result of "-fno-move-loo= p-invariants"). > > 2) By default -fira_loop_pressure is not enabled for -Os, the logic to co= mpute "regs_used" seams not sound. The following codes is from function "fi= nd_invariants_to_move" > { > unsigned int n_regs =3D DF_REG_SIZE (df); > > regs_used =3D 2; > > for (i =3D 0; i < n_regs; i++) > { > if (!DF_REGNO_FIRST_DEF (i) && DF_REGNO_LAST_USE (i)) > { > /* This is a value that is used but not changed inside loop= . */ > regs_used++; > } > } > } > * There is no loop related inform in the code. > * Benchmark logs show the condition (!DF_REGNO_FIRST_DEF (i) && DF_REGNO_= LAST_USE (i)) is never true. Still there is code that tries to deal with -Os. Simply disabling the pass makes that logic pointless. Thus, please try to fix the code that is there to deal with -Os (a target m= ay opt to enable -fira-loop-pressure by default for -Os). Thanks, Richard. > Thanks! > -Zhenqiang > > >