RE: [PATCH][AArch64] Enable -frename-registers at -O2 and higher

2016-06-16 Thread Evandro Menezes
> Though there's a slight (<1%) overall improvement on Exynos M1, there just
were
> too many significant (<-3%) regressions for a few significant improvements for
me
> to be comfortable with -frename-registers being a generic default for AArch64.
> 
> I'll run some larger benchmarks tonight, but I'm leaning towards having it as
a
> target specific extra tuning option.

The results are in and -frename-registers is not a good idea for Exynos M1.

Thank you,

-- 
Evandro Menezes  Austin, TX






RE: [PATCH][AArch64] Enable -frename-registers at -O2 and higher

2016-06-15 Thread Evandro Menezes
> On Fri, May 27, 2016 at 02:50:15PM +0100, Kyrill Tkachov wrote:
> >
> > As mentioned in
> > https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00297.html,
> > frename-registers registers can be beneficial for aarch64 and the
> > patch at https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01618.html
> > resolves the AESE/AESMC fusion issue that it exposed in the aarch64
> > backend. So this patch enables the pass for aarch64 at -O2 and above.
> >
> > Ok for trunk?
> 
> As you're proposing to have this on by default, I'd like to give a chance
to
> hear whether there is consensus as to this being the right choice for the
> thunderx, xgene1, exynos-m1 and qdf24xx subtargets.

Though there's a slight (<1%) overall improvement on Exynos M1, there just
were too many significant (<-3%) regressions for a few significant
improvements for me to be comfortable with -frename-registers being a
generic default for AArch64.

I'll run some larger benchmarks tonight, but I'm leaning towards having it
as a target specific extra tuning option.

Thank you,

-- 
Evandro Menezes  Austin, TX

PS: I'm fine with refactoring aarch_option_optimization_table to
aarch64_option_optimization_table.



Re: [PATCH][AArch64] Enable -frename-registers at -O2 and higher

2016-06-15 Thread Dr. Philipp Tomsich

> On 10 Jun 2016, at 01:28, Jim Wilson  wrote:
> 
> On Tue, May 31, 2016 at 2:56 AM, James Greenhalgh
>  wrote:
>> As you're proposing to have this on by default, I'd like to give a chance
>> to hear whether there is consensus as to this being the right choice for
>> the thunderx, xgene1, exynos-m1 and qdf24xx subtargets.

Testing on XGene-1 and XGene-2 shows a small improvement (0.8% overall)
on SPEC2006 and a negligible improvement for CoreMark, just in line with 
what we’d expect. 

So from our end, it’s a vote for “on by default”.

Regards,
Phil.

Re: [PATCH][AArch64] Enable -frename-registers at -O2 and higher

2016-06-09 Thread Jim Wilson
On Tue, May 31, 2016 at 2:56 AM, James Greenhalgh
 wrote:
> As you're proposing to have this on by default, I'd like to give a chance
> to hear whether there is consensus as to this being the right choice for
> the thunderx, xgene1, exynos-m1 and qdf24xx subtargets.

I tested this on qdf24xx using SPEC 2006.  I'm seeing a very small
performance decrease on the int benchmarks which may just be noise,
and an even smaller decrease on the FP side which is definitely in the
noise.  This is an out-of-order part with hardware register renaming,
so this optimization probably doesn't do much for this target.  I may
also be seeing secondary issues, e.g. the pipeline description isn't
as good as I would like yet.

Jim


Re: [PATCH][AArch64] Enable -frename-registers at -O2 and higher

2016-05-31 Thread James Greenhalgh
On Fri, May 27, 2016 at 02:50:15PM +0100, Kyrill Tkachov wrote:
> Hi all,
> 
> As mentioned in https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00297.html,
> frename-registers registers can be beneficial for aarch64 and the patch at
> https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01618.html resolves the
> AESE/AESMC fusion issue that it exposed in the aarch64 backend. So this patch
> enables the pass for aarch64 at -O2 and above.
> 
> Bootstrapped and tested on aarch64-none-linux-gnu.
> 
> Ok for trunk?

Not without some more detail on the situations in which this is a likely
gain for AArch64 and whether this expected improvement is generic enough
to enable it for all AArch64 targets and worth the compile time trade-off.

As you're proposing to have this on by default, I'd like to give a chance
to hear whether there is consensus as to this being the right choice for
the thunderx, xgene1, exynos-m1 and qdf24xx subtargets.

My expectation is that the slight code size decrease is likely to be a good
thing everywhere, and the breaking of register dependency chains may give
better scheduling for our cores with detailed scheduling models. So we're
unlikely to cause any performance worries, and may get a slight uplift in
a few places. Any data confirming that would be useful. I'd also like to see
some estimates for the impact on compilation time (we are already running
one regrename pass when targeting Cortex-A57).

If we can't reach consensus, then the next best approach is to add this as
a new option to AARCH64_EXTRA_TUNING_OPTION and enable it as appropriate
for subtargets - you might find that this is not worthwhile for the small
uplift Ramana and Wilco measured.

I've widened the CC list for you, let's wait a week for comments and make
a decision then.

Thanks,
James

> P.S. Why is the table holding this information called 
> aarch_option_optimization_table rather than
> aarch64_option_optimization_table ?
> 
> 2016-05-27  Kyrylo Tkachov  
> 
> * common/config/aarch64/aarch64-common.c
> (aarch64_option_optimization_table): Enable -frename-registers at
> -O2 and higher.

> diff --git a/gcc/common/config/aarch64/aarch64-common.c 
> b/gcc/common/config/aarch64/aarch64-common.c
> index 
> 08e795934207d015d9fa22c3822930af4a21c93a..91801df731471f1842802370497e498fda62098a
>  100644
> --- a/gcc/common/config/aarch64/aarch64-common.c
> +++ b/gcc/common/config/aarch64/aarch64-common.c
> @@ -50,6 +50,8 @@ static const struct default_options 
> aarch_option_optimization_table[] =
>  { OPT_LEVELS_1_PLUS, OPT_fsched_pressure, NULL, 1 },
>  /* Enable redundant extension instructions removal at -O2 and higher.  */
>  { OPT_LEVELS_2_PLUS, OPT_free, NULL, 1 },
> +/* Enable the register renaming pass at -O2 and higher.  */
> +{ OPT_LEVELS_2_PLUS, OPT_frename_registers, NULL, 1 },
>  { OPT_LEVELS_NONE, 0, NULL, 0 }
>};
>  



[PATCH][AArch64] Enable -frename-registers at -O2 and higher

2016-05-27 Thread Kyrill Tkachov

Hi all,

As mentioned in https://gcc.gnu.org/ml/gcc-patches/2016-05/msg00297.html, 
frename-registers registers can be beneficial for aarch64
and the patch at https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01618.html 
resolves the AESE/AESMC fusion issue that it exposed
in the aarch64 backend. So this patch enables the pass for aarch64 at -O2 and 
above.

Bootstrapped and tested on aarch64-none-linux-gnu.

Ok for trunk?
Thanks,
Kyrill

P.S. Why is the table holding this information called 
aarch_option_optimization_table rather than
aarch64_option_optimization_table ?

2016-05-27  Kyrylo Tkachov  

* common/config/aarch64/aarch64-common.c
(aarch64_option_optimization_table): Enable -frename-registers at
-O2 and higher.
diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c
index 08e795934207d015d9fa22c3822930af4a21c93a..91801df731471f1842802370497e498fda62098a 100644
--- a/gcc/common/config/aarch64/aarch64-common.c
+++ b/gcc/common/config/aarch64/aarch64-common.c
@@ -50,6 +50,8 @@ static const struct default_options aarch_option_optimization_table[] =
 { OPT_LEVELS_1_PLUS, OPT_fsched_pressure, NULL, 1 },
 /* Enable redundant extension instructions removal at -O2 and higher.  */
 { OPT_LEVELS_2_PLUS, OPT_free, NULL, 1 },
+/* Enable the register renaming pass at -O2 and higher.  */
+{ OPT_LEVELS_2_PLUS, OPT_frename_registers, NULL, 1 },
 { OPT_LEVELS_NONE, 0, NULL, 0 }
   };