On Tue, Jul 3, 2012 at 10:29 AM, Zhenqiang Chen <zhenqiang.c...@arm.com> wrote: >>-----Original Message----- >>From: Richard Guenther [mailto:richard.guent...@gmail.com] >>Sent: 2012年6月28日 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 <zhenqiang.c...@arm.com> >>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 = >>>>> 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 = loop; >>> /* move_single_loop_invariants for very large loops >>> is time consuming and might need a lot of memory. */ >>> - if (loop->num_nodes <= (unsigned) >>> LOOP_INVARIANT_MAX_BBS_IN_LOOP) >>> + if (loop->num_nodes <= (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 (eventually >>only >>with -fira-loop-pressure) - please work out why this support is not working >>for >>you. > > 1) If -fira_loop_pressure is enabled, it reduces ~24% invariant motions in my > tests. But it does not help on total code size. Seams there is issue to > update the "regs_needed" after moving an invariant out of the loop (My > benchmark logs show ~73% cases have more than one invariants moved). > > During tracing, I found that move an integer constant out of the loop does > not increase regs_needed. Function "get_pressure_class_and_nregs (rtx insn, > int *nregs)" computes the "regs_needed". > > *nregs > = 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 constants out of the loop, which leads to spilling. > > According to the algorithm in "calculate_loop_reg_pressure", moving an > invariant out of the loop should impact on the register pressure. So I try to > add the following code > > if (! (*nregs)) > *nregs = 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 other > passes in addition to loop2_invariant (The result of "-fira_loop_pressure > -fno-move-loop-invariants" is different from the result of > "-fno-move-loop-invariants"). > > 2) By default -fira_loop_pressure is not enabled for -Os, the logic to > compute "regs_used" seams not sound. The following codes is from function > "find_invariants_to_move" > { > unsigned int n_regs = DF_REG_SIZE (df); > > regs_used = 2; > > for (i = 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 may opt to enable -fira-loop-pressure by default for -Os). Thanks, Richard. > Thanks! > -Zhenqiang > > >
