RE: [PATCH][AArch64] Enable -frename-registers at -O2 and higher
> 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
> 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
> 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
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
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
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 } };