Message ID | 1572025151-22783-3-git-send-email-craig.blackmore@embecosm.com |
---|---|
State | New |
Headers | show |
Series | RISC-V: Allow more load/stores to be compressed | expand |
On Fri, Oct 25, 2019 at 10:40 AM Craig Blackmore <craig.blackmore@embecosm.com> wrote: > The sched2 pass undoes some of the addresses generated by the RISC-V > shorten_memrefs code size optimization (patch 1/2) and consequently increases > code size. This patch prevents sched-deps.c from changing an address if it is > expected to increase address cost. This should be rewritten as a target hook, and then we can define the hook to do what we want for RISC-V. It isn't OK to make this change for other targets without testing them. If the default hook does nothing, then other targets won't be affected. Jim
Hi Jim, On 31/10/2019 00:03, Jim Wilson wrote: > On Fri, Oct 25, 2019 at 10:40 AM Craig Blackmore > <craig.blackmore@embecosm.com> wrote: >> The sched2 pass undoes some of the addresses generated by the RISC-V >> shorten_memrefs code size optimization (patch 1/2) and consequently increases >> code size. This patch prevents sched-deps.c from changing an address if it is >> expected to increase address cost. > > This should be rewritten as a target hook, and then we can define the > hook to do what we want for RISC-V. It isn't OK to make this change > for other targets without testing them. If the default hook does > nothing, then other targets won't be affected. > Thanks for the review, here is an updated patch rewriting this as a target hook. Thanks, Craig --- gcc/ChangeLog: * config/riscv/riscv.c (riscv_new_address_profitable_p): New function. (TARGET_NEW_ADDRESS_PROFITABLE_P): Define. * doc/tm.texi: Regenerate. * doc/tm.texi.in (TARGET_NEW_ADDRESS_PROFITABLE_P): New hook. * sched-deps.c (attempt_change): Use old address if it is cheaper than new address. * target.def (new_address_profitable_p): New hook. * targhooks.c (default_new_address_profitable_p): New function. * targhooks.h (default_new_address_profitable_p): Declare. --- gcc/config/riscv/riscv.c | 16 ++++++++++++++++ gcc/doc/tm.texi | 7 +++++++ gcc/doc/tm.texi.in | 2 ++ gcc/sched-deps.c | 3 +++ gcc/target.def | 11 +++++++++++ gcc/targhooks.c | 13 +++++++++++++ gcc/targhooks.h | 1 + 7 files changed, 53 insertions(+) diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c index 1d6d2f89f7d..a508894d16e 100644 --- a/gcc/config/riscv/riscv.c +++ b/gcc/config/riscv/riscv.c @@ -5095,6 +5095,19 @@ riscv_reorg (void) riscv_remove_unneeded_save_restore_calls (); } +/* Implement TARGET_NEW_ADDRESS_PROFITABLE_P. */ + +bool +riscv_new_address_profitable_p (rtx memref, rtx_insn *insn, rtx new_addr) +{ + /* Prefer old address if it is less expensive. */ + addr_space_t as = MEM_ADDR_SPACE (memref); + bool speed = optimize_bb_for_speed_p (BLOCK_FOR_INSN (insn)); + int old_cost = address_cost (XEXP (memref, 0), GET_MODE (memref), as, speed); + int new_cost = address_cost (new_addr, GET_MODE (memref), as, speed); + return new_cost <= old_cost; +} + /* Initialize the GCC target structure. */ #undef TARGET_ASM_ALIGNED_HI_OP #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t" @@ -5272,6 +5285,9 @@ riscv_reorg (void) #undef TARGET_MACHINE_DEPENDENT_REORG #define TARGET_MACHINE_DEPENDENT_REORG riscv_reorg +#undef TARGET_NEW_ADDRESS_PROFITABLE_P +#define TARGET_NEW_ADDRESS_PROFITABLE_P riscv_new_address_profitable_p + struct gcc_target targetm = TARGET_INITIALIZER; #include "gt-riscv.h" diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 5b8b68bd710..85470c8afa0 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -6944,6 +6944,13 @@ candidate as a replacement for the if-convertible sequence described in @code{if_info}. @end deftypefn +@deftypefn {Target Hook} bool TARGET_NEW_ADDRESS_PROFITABLE_P (rtx @var{memref}, rtx_insn * @var{insn}, rtx @var{new_addr}) +Return @code{true} if it is profitable to replace the address in +@var{memref} with @var{new_addr}. This allows targets to prevent the +scheduler from undoing address optimizations. The instruction containing the +memref is @var{insn}. The default implementation returns @code{true}. +@end deftypefn + @deftypefn {Target Hook} bool TARGET_NO_SPECULATION_IN_DELAY_SLOTS_P (void) This predicate controls the use of the eager delay slot filler to disallow speculatively executed instructions being placed in delay slots. Targets diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 1b061d70127..b5c80ce2fdc 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -4695,6 +4695,8 @@ Define this macro if a non-short-circuit operation produced by @hook TARGET_NOCE_CONVERSION_PROFITABLE_P +@hook TARGET_NEW_ADDRESS_PROFITABLE_P + @hook TARGET_NO_SPECULATION_IN_DELAY_SLOTS_P @hook TARGET_ESTIMATED_POLY_VALUE diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c index a9e73fc6044..8f56594b745 100644 --- a/gcc/sched-deps.c +++ b/gcc/sched-deps.c @@ -4693,6 +4693,9 @@ attempt_change (struct mem_inc_info *mii, rtx new_addr) rtx mem = *mii->mem_loc; rtx new_mem; + if (!targetm.new_address_profitable_p (mem, mii->mem_insn, new_addr)) + return NULL_RTX; + /* Jump through a lot of hoops to keep the attributes up to date. We do not want to call one of the change address variants that take an offset even though we know the offset in many cases. These diff --git a/gcc/target.def b/gcc/target.def index e0e856979a9..81c78d8ffea 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -3824,6 +3824,17 @@ candidate as a replacement for the if-convertible sequence described in\n\ bool, (rtx_insn *seq, struct noce_if_info *if_info), default_noce_conversion_profitable_p) +/* Return true if new_addr should be preferred over the existing address used by + memref in insn. */ +DEFHOOK +(new_address_profitable_p, + "Return @code{true} if it is profitable to replace the address in\n\ +@var{memref} with @var{new_addr}. This allows targets to prevent the\n\ +scheduler from undoing address optimizations. The instruction containing the\n\ +memref is @var{insn}. The default implementation returns @code{true}.", +bool, (rtx memref, rtx_insn * insn, rtx new_addr), +default_new_address_profitable_p) + DEFHOOK (estimated_poly_value, "Return an estimate of the runtime value of @var{val}, for use in\n\ diff --git a/gcc/targhooks.c b/gcc/targhooks.c index 2d3bcbf3e89..8e3ad63f1c1 100644 --- a/gcc/targhooks.c +++ b/gcc/targhooks.c @@ -1564,6 +1564,19 @@ default_mode_dependent_address_p (const_rtx addr ATTRIBUTE_UNUSED, return false; } +extern bool default_new_address_profitable_p (rtx, rtx); + + +/* The default implementation of TARGET_NEW_ADDRESS_PROFITABLE_P. */ + +bool +default_new_address_profitable_p (rtx memref ATTRIBUTE_UNUSED, + rtx_insn *insn ATTRIBUTE_UNUSED, + rtx new_addr ATTRIBUTE_UNUSED) +{ + return true; +} + bool default_target_option_valid_attribute_p (tree ARG_UNUSED (fndecl), tree ARG_UNUSED (name), diff --git a/gcc/targhooks.h b/gcc/targhooks.h index 34987bbcdb3..76a55428851 100644 --- a/gcc/targhooks.h +++ b/gcc/targhooks.h @@ -185,6 +185,7 @@ extern tree default_emutls_var_init (tree, tree, tree); extern unsigned int default_hard_regno_nregs (unsigned int, machine_mode); extern bool default_hard_regno_scratch_ok (unsigned int); extern bool default_mode_dependent_address_p (const_rtx, addr_space_t); +extern bool default_new_address_profitable_p (rtx, rtx_insn *, rtx); extern bool default_target_option_valid_attribute_p (tree, tree, tree, int); extern bool default_target_option_pragma_parse (tree, tree); extern bool default_target_can_inline_p (tree, tree);
diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c index 308db4e3ca0..c7d0ca550df 100644 --- a/gcc/sched-deps.c +++ b/gcc/sched-deps.c @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see #include "params.h" #include "cselib.h" #include "function-abi.h" +#include "predict.h" #ifdef INSN_SCHEDULING @@ -4694,6 +4695,14 @@ attempt_change (struct mem_inc_info *mii, rtx new_addr) rtx mem = *mii->mem_loc; rtx new_mem; + /* Prefer old address if it is less expensive. */ + addr_space_t as = MEM_ADDR_SPACE (mem); + bool speed = optimize_bb_for_speed_p (BLOCK_FOR_INSN (mii->mem_insn)); + int old_cost = address_cost (XEXP (mem, 0), GET_MODE (mem), as, speed); + int new_cost = address_cost (new_addr, GET_MODE (mem), as, speed); + if (new_cost > old_cost) + return NULL_RTX; + /* Jump through a lot of hoops to keep the attributes up to date. We do not want to call one of the change address variants that take an offset even though we know the offset in many cases. These