Do not imply -fweb with -fpeel-loops

[Richard Biener]

On Tue, May 31, 2016 at 12:49 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> enabling -fpeel-loops with -O3 and -Ofast had unexpected effect of enabling
> -fweb and -frename-registers.  This patch drop -fweb from list of passes that
> are implied by -fpeel-loops because it is no longer needed since we do peeling
> at tree instead of RTL and the out-of-ssa pass cares about this.
>
> I will look into rename registers separately. My understanding is that rename
> registers is not really that specific for -funroll-loops or -fpeel-loops.
> Loop unrolling may make it bit more useful because it increases expected basic
> block size and thus enables scheduler to do more.
>
> Rather it depends on target whether register renaming is win. On out of order targets
> where scheduling is not that imprtant it does not seem to be worth the compile time
> and code size effects, while on targets that depends on scheduling it does.
> Tonight tests on x86 shows improvmeents in botan which I think are related to renaming
> (because they also improved when Bern temporarily enabled it few days back),
> measurable code size regression (which is probably fixable if we make rename-registers
> to not increase instruction encoding size) and noticeable compile time slowdown
> (not sure if it is from regrename itself that is quite easy pass or from the fact
> that scheduler has more freedom and thus work harder).
>
> On the other hand both SPECint and SPECfp improved by about 2% on Itanium and
> code size reduced.  I think similar effect should happen on superscalar inorder
> tarets with constant size of instruction encoding.
>
> I would say we should handle -frename-registers same way as we do -fschedule-insns.
> I.e. enable/disable it on target basis rather than budnle it to loop unrolling.
> But that is for future patch.  I will commit this and propose patch making -fpeel-loops
> independent of rename-registers next

Ok.  Does -fweb still do what its documentation says (enable register
allocation to
work on pseudos directly)?  Given its downside and strong GIMPLE
optimizations maybe
it is time to remove it?

Thanks,
Richard.

> Honza
>
> Bootstrapped/regtested x86_64-linux, will commit it shortly.
>
>         * loop-init.c (gate): Do not enale RTL loop unroller with -fpeel-loops.
>         It no longer does that.
>         * toplev.c (process_options): Do not enable flag_web with -fpeel-loops.
> Index: loop-init.c
> ===================================================================
> --- loop-init.c (revision 236894)
> +++ loop-init.c (working copy)
> @@ -560,7 +560,7 @@ public:
>    /* opt_pass methods: */
>    virtual bool gate (function *)
>      {
> -      return (flag_peel_loops || flag_unroll_loops || flag_unroll_all_loops);
> +      return (flag_unroll_loops || flag_unroll_all_loops);
>      }
>
>    virtual unsigned int execute (function *);
> Index: toplev.c
> ===================================================================
> --- toplev.c    (revision 236894)
> +++ toplev.c    (working copy)
> @@ -1296,7 +1296,7 @@ process_options (void)
>
>    /* web and rename-registers help when run after loop unrolling.  */
>    if (flag_web == AUTODETECT_VALUE)
> -    flag_web = flag_unroll_loops || flag_peel_loops;
> +    flag_web = flag_unroll_loops;
>
>    if (flag_rename_registers == AUTODETECT_VALUE)
>      flag_rename_registers = flag_unroll_loops || flag_peel_loops;