| Message ID | 1464942950-29272-1-git-send-email-james.greenhalgh@arm.com |
|---|---|
| State | Accepted |
| Commit | b2fb6b750b8de0131569bdb1b1939cf427db7ae2 |
| Headers | show |
On Fri, Jun 03, 2016 at 09:35:50AM +0100, James Greenhalgh wrote: > > Hi, > > This patch rebases the floating-point cost table for Cortex-A57 to be > relative to the cost of a floating-point move. This in response to this > feedback from Richard Sandiford [2] on Ramana's patch to calls.c [1] from > 2014: > > I think this is really a bug in the backend. The backend is assigning a > cost of COSTS_N_INSNS (3) to a floating-point constant not because the > constant itself is expensive -- it's actually as cheap as a register > in this context -- but because the backend considers floating-point > moves to be 3 times more expensive than cheap integer moves. > > The argument is that a move in mode X should be treated with cost > COSTS_N_INSNS (1), and other instructions should have a cost relative to > that move. For example, in this patch we say that instructions building a > floating-point constant are the same cost as a floating-point register to > register move. Fixing this fixes the issue Ramana was seeing, in a way > consistent with what other back-ends do. > > This patch gives a small improvement to Spec2000FP on a Cortex-A57 > platform. > > Bootstrapped on aarch64-none-linux-gnu with no issues. > > OK? *ping* Thanks, James > 2016-06-03 James Greenhalgh <james.greenhalgh@arm.com> > > * config/arm/aarch-cost-tables.h (cortexa57_extra_costs): Make FP > costs relative to the cost of a register move. >
On Fri, Jun 10, 2016 at 09:29:46AM +0100, James Greenhalgh wrote: > On Fri, Jun 03, 2016 at 09:35:50AM +0100, James Greenhalgh wrote: > > > > Hi, > > > > This patch rebases the floating-point cost table for Cortex-A57 to be > > relative to the cost of a floating-point move. This in response to this > > feedback from Richard Sandiford [2] on Ramana's patch to calls.c [1] from > > 2014: > > > > I think this is really a bug in the backend. The backend is assigning a > > cost of COSTS_N_INSNS (3) to a floating-point constant not because the > > constant itself is expensive -- it's actually as cheap as a register > > in this context -- but because the backend considers floating-point > > moves to be 3 times more expensive than cheap integer moves. > > > > The argument is that a move in mode X should be treated with cost > > COSTS_N_INSNS (1), and other instructions should have a cost relative to > > that move. For example, in this patch we say that instructions building a > > floating-point constant are the same cost as a floating-point register to > > register move. Fixing this fixes the issue Ramana was seeing, in a way > > consistent with what other back-ends do. > > > > This patch gives a small improvement to Spec2000FP on a Cortex-A57 > > platform. > > > > Bootstrapped on aarch64-none-linux-gnu with no issues. > > > > OK? > > *ping* *ping^* https://gcc.gnu.org/ml/gcc-patches/2016-06/msg00251.html Thanks, James > > 2016-06-03 James Greenhalgh <james.greenhalgh@arm.com> > > > > * config/arm/aarch-cost-tables.h (cortexa57_extra_costs): Make FP > > costs relative to the cost of a register move. > > >
On 03/06/16 09:35, James Greenhalgh wrote: > > Hi, > > This patch rebases the floating-point cost table for Cortex-A57 to be > relative to the cost of a floating-point move. This in response to this > feedback from Richard Sandiford [2] on Ramana's patch to calls.c [1] from > 2014: > > I think this is really a bug in the backend. The backend is assigning a > cost of COSTS_N_INSNS (3) to a floating-point constant not because the > constant itself is expensive -- it's actually as cheap as a register > in this context -- but because the backend considers floating-point > moves to be 3 times more expensive than cheap integer moves. > > The argument is that a move in mode X should be treated with cost > COSTS_N_INSNS (1), and other instructions should have a cost relative to > that move. For example, in this patch we say that instructions building a > floating-point constant are the same cost as a floating-point register to > register move. Fixing this fixes the issue Ramana was seeing, in a way > consistent with what other back-ends do. > > This patch gives a small improvement to Spec2000FP on a Cortex-A57 > platform. > > Bootstrapped on aarch64-none-linux-gnu with no issues. > > OK? > OK. R. > Thanks, > James > > --- > 2016-06-03 James Greenhalgh <james.greenhalgh@arm.com> > > * config/arm/aarch-cost-tables.h (cortexa57_extra_costs): Make FP > costs relative to the cost of a register move. > > [1] https://gcc.gnu.org/ml/gcc-patches/2014-10/msg00136.html > [2] https://gcc.gnu.org/ml/gcc-patches/2014-10/msg00391.html > > > 0001-AArch64-Give-some-new-costs-for-Cortex-A57-floating-.patch > > > diff --git a/gcc/config/arm/aarch-cost-tables.h b/gcc/config/arm/aarch-cost-tables.h > index c971b30..5f42253 100644 > --- a/gcc/config/arm/aarch-cost-tables.h > +++ b/gcc/config/arm/aarch-cost-tables.h > @@ -294,35 +294,35 @@ const struct cpu_cost_table cortexa57_extra_costs = > { > /* FP SFmode */ > { > - COSTS_N_INSNS (17), /* div. */ > - COSTS_N_INSNS (5), /* mult. */ > - COSTS_N_INSNS (9), /* mult_addsub. */ > - COSTS_N_INSNS (9), /* fma. */ > - COSTS_N_INSNS (4), /* addsub. */ > - COSTS_N_INSNS (2), /* fpconst. */ > - COSTS_N_INSNS (2), /* neg. */ > - COSTS_N_INSNS (2), /* compare. */ > - COSTS_N_INSNS (4), /* widen. */ > - COSTS_N_INSNS (4), /* narrow. */ > - COSTS_N_INSNS (4), /* toint. */ > - COSTS_N_INSNS (4), /* fromint. */ > - COSTS_N_INSNS (4) /* roundint. */ > + COSTS_N_INSNS (6), /* div. */ > + COSTS_N_INSNS (1), /* mult. */ > + COSTS_N_INSNS (2), /* mult_addsub. */ > + COSTS_N_INSNS (2), /* fma. */ > + COSTS_N_INSNS (1), /* addsub. */ > + 0, /* fpconst. */ > + 0, /* neg. */ > + 0, /* compare. */ > + COSTS_N_INSNS (1), /* widen. */ > + COSTS_N_INSNS (1), /* narrow. */ > + COSTS_N_INSNS (1), /* toint. */ > + COSTS_N_INSNS (1), /* fromint. */ > + COSTS_N_INSNS (1) /* roundint. */ > }, > /* FP DFmode */ > { > - COSTS_N_INSNS (31), /* div. */ > - COSTS_N_INSNS (5), /* mult. */ > - COSTS_N_INSNS (9), /* mult_addsub. */ > - COSTS_N_INSNS (9), /* fma. */ > - COSTS_N_INSNS (4), /* addsub. */ > - COSTS_N_INSNS (2), /* fpconst. */ > - COSTS_N_INSNS (2), /* neg. */ > - COSTS_N_INSNS (2), /* compare. */ > - COSTS_N_INSNS (4), /* widen. */ > - COSTS_N_INSNS (4), /* narrow. */ > - COSTS_N_INSNS (4), /* toint. */ > - COSTS_N_INSNS (4), /* fromint. */ > - COSTS_N_INSNS (4) /* roundint. */ > + COSTS_N_INSNS (11), /* div. */ > + COSTS_N_INSNS (1), /* mult. */ > + COSTS_N_INSNS (2), /* mult_addsub. */ > + COSTS_N_INSNS (2), /* fma. */ > + COSTS_N_INSNS (1), /* addsub. */ > + 0, /* fpconst. */ > + 0, /* neg. */ > + 0, /* compare. */ > + COSTS_N_INSNS (1), /* widen. */ > + COSTS_N_INSNS (1), /* narrow. */ > + COSTS_N_INSNS (1), /* toint. */ > + COSTS_N_INSNS (1), /* fromint. */ > + COSTS_N_INSNS (1) /* roundint. */ > } > }, > /* Vector */ >
diff --git a/gcc/config/arm/aarch-cost-tables.h b/gcc/config/arm/aarch-cost-tables.h index c971b30..5f42253 100644 --- a/gcc/config/arm/aarch-cost-tables.h +++ b/gcc/config/arm/aarch-cost-tables.h @@ -294,35 +294,35 @@ const struct cpu_cost_table cortexa57_extra_costs = { /* FP SFmode */ { - COSTS_N_INSNS (17), /* div. */ - COSTS_N_INSNS (5), /* mult. */ - COSTS_N_INSNS (9), /* mult_addsub. */ - COSTS_N_INSNS (9), /* fma. */ - COSTS_N_INSNS (4), /* addsub. */ - COSTS_N_INSNS (2), /* fpconst. */ - COSTS_N_INSNS (2), /* neg. */ - COSTS_N_INSNS (2), /* compare. */ - COSTS_N_INSNS (4), /* widen. */ - COSTS_N_INSNS (4), /* narrow. */ - COSTS_N_INSNS (4), /* toint. */ - COSTS_N_INSNS (4), /* fromint. */ - COSTS_N_INSNS (4) /* roundint. */ + COSTS_N_INSNS (6), /* div. */ + COSTS_N_INSNS (1), /* mult. */ + COSTS_N_INSNS (2), /* mult_addsub. */ + COSTS_N_INSNS (2), /* fma. */ + COSTS_N_INSNS (1), /* addsub. */ + 0, /* fpconst. */ + 0, /* neg. */ + 0, /* compare. */ + COSTS_N_INSNS (1), /* widen. */ + COSTS_N_INSNS (1), /* narrow. */ + COSTS_N_INSNS (1), /* toint. */ + COSTS_N_INSNS (1), /* fromint. */ + COSTS_N_INSNS (1) /* roundint. */ }, /* FP DFmode */ { - COSTS_N_INSNS (31), /* div. */ - COSTS_N_INSNS (5), /* mult. */ - COSTS_N_INSNS (9), /* mult_addsub. */ - COSTS_N_INSNS (9), /* fma. */ - COSTS_N_INSNS (4), /* addsub. */ - COSTS_N_INSNS (2), /* fpconst. */ - COSTS_N_INSNS (2), /* neg. */ - COSTS_N_INSNS (2), /* compare. */ - COSTS_N_INSNS (4), /* widen. */ - COSTS_N_INSNS (4), /* narrow. */ - COSTS_N_INSNS (4), /* toint. */ - COSTS_N_INSNS (4), /* fromint. */ - COSTS_N_INSNS (4) /* roundint. */ + COSTS_N_INSNS (11), /* div. */ + COSTS_N_INSNS (1), /* mult. */ + COSTS_N_INSNS (2), /* mult_addsub. */ + COSTS_N_INSNS (2), /* fma. */ + COSTS_N_INSNS (1), /* addsub. */ + 0, /* fpconst. */ + 0, /* neg. */ + 0, /* compare. */ + COSTS_N_INSNS (1), /* widen. */ + COSTS_N_INSNS (1), /* narrow. */ + COSTS_N_INSNS (1), /* toint. */ + COSTS_N_INSNS (1), /* fromint. */ + COSTS_N_INSNS (1) /* roundint. */ } }, /* Vector */