| Message ID | 20141218.100012.59175136.kkojima@rr.iij4u.or.jp |
|---|---|
| State | New |
| Headers | show |
On 2014-12-17 8:00 PM, Kaz Kojima wrote: > This was discussed in PR55212 > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55212#c25 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55212#c58 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55212#c69 > > and is to fix another ICEs in assign_by_spills. > This patch introduces a new target hook to disable replacing with > memory equivalence if the macro returns true. > The problem happens for r0 again. SH ISA has many instructions with > "r0-parity". The index register is r0 only, only r0 can be used as > QI/HImode load/store target/source, and so on. > It seems that replacing mem equiv makes register pressure high > especially on r0 and fails in spilling. Also replacing mem equiv > doesn't win CSiBE in average on this target. Please see the above > trail #c69 for CSiBE numbers. > > I think the one design goal of LRA is to avoid target macros if possible. > There might be some way to handle the above situation without target > macros, though it's over my head. > I don't think it will be easy to solve this problem by following reg pressure. I guess R0 will be in the same pressure reg class. I believe some hook is an adequate solution here. > -- > * lra-constraints.c (get_equiv): Don't return memory equivalence > when targetm.cannot_substitute_mem_equiv_p is true. > * target.def (cannot_substitute_mem_equiv_p): New hook. > * config/sh/sh.c (sh_cannot_substitute_mem_equiv_p): New function. > (TARGET_CANNOT_SUBSTITUTE_MEM_EQUIV_P): Define. > * doc/tm.texi.in (TARGET_CANNOT_SUBSTITUTE_MEM_EQUIV_P): New hook. > * doc/tm.texi: Regenerate. > LRA part of the patch is ok. I found one typo in "and the speed on avarage working sets." (average). But I'd recommend to try different hook implementation checking r0 in the address as still equiv memory substitution might be profitable for SH in some cases.
Vladimir Makarov <vmakarov@redhat.com> wrote: > I don't think it will be easy to solve this problem by following reg > pressure. I guess R0 will be in the same pressure reg class. I > believe some hook is an adequate solution here. > >> -- >> * lra-constraints.c (get_equiv): Don't return memory equivalence >> when targetm.cannot_substitute_mem_equiv_p is true. >> * target.def (cannot_substitute_mem_equiv_p): New hook. >> * config/sh/sh.c (sh_cannot_substitute_mem_equiv_p): New function. >> (TARGET_CANNOT_SUBSTITUTE_MEM_EQUIV_P): Define. >> * doc/tm.texi.in (TARGET_CANNOT_SUBSTITUTE_MEM_EQUIV_P): New hook. >> * doc/tm.texi: Regenerate. >> > > LRA part of the patch is ok. I found one typo in "and the speed on > avarage working sets." (average). Thanks for looking at the patch. I'll commit it with fixing that typo. > But I'd recommend to try different hook implementation checking r0 in > the address as still equiv memory substitution might be profitable for > SH in some cases. I've seen that the equiv substitution improves the code much on other targets. First I've tried hooks which disable only some addressing related r0 usages but they didn't win CSiBE. I'll try another hook implementaion as you suggested. Regards, kaz
diff --git a/lra-constraints.c b/lra-constraints.c index 47cbbec..1666380 100644 --- a/lra-constraints.c +++ b/lra-constraints.c @@ -488,7 +488,11 @@ get_equiv (rtx x) || lra_get_regno_hard_regno (regno) >= 0) return x; if ((res = ira_reg_equiv[regno].memory) != NULL_RTX) - return res; + { + if (targetm.cannot_substitute_mem_equiv_p (res)) + return x; + return res; + } if ((res = ira_reg_equiv[regno].constant) != NULL_RTX) return res; if ((res = ira_reg_equiv[regno].invariant) != NULL_RTX) diff --git a/target.def b/target.def index 7c0296d..b0c86b8 100644 --- a/target.def +++ b/target.def @@ -5037,6 +5037,20 @@ DEFHOOK reg_class_t, (reg_class_t rclass), default_preferred_rename_class) +/* This target hook allows the backend to avoid unsafe substitution + during register allocation. */ +DEFHOOK +(cannot_substitute_mem_equiv_p, + "A target hook which returns @code{true} if @var{subst} can't\n\ +substitute safely pseudos with equivalent memory values during\n\ +register allocation.\n\ +The default version of this target hook returns @code{false}.\n\ +On most machines, this default should be used. For generally\n\ +machines with non orthogonal register usage for addressing, such\n\ +as SH, this hook can be used to avoid excessive spilling.", + bool, (rtx subst), + hook_bool_rtx_false) + /* This target hook allows the backend to perform additional processing while initializing for variable expansion. */ DEFHOOK diff --git a/config/sh/sh.c b/config/sh/sh.c index f578b43..c445199 100644 --- a/config/sh/sh.c +++ b/config/sh/sh.c @@ -291,6 +291,7 @@ static reg_class_t sh_secondary_reload (bool, rtx, reg_class_t, static bool sh_legitimate_address_p (machine_mode, rtx, bool); static rtx sh_legitimize_address (rtx, rtx, machine_mode); static rtx sh_delegitimize_address (rtx); +static bool sh_cannot_substitute_mem_equiv_p (rtx); static int shmedia_target_regs_stack_space (HARD_REG_SET *); static int shmedia_reserve_space_for_target_registers_p (int, HARD_REG_SET *); static int shmedia_target_regs_stack_adjust (HARD_REG_SET *); @@ -630,6 +631,9 @@ static const struct attribute_spec sh_attribute_table[] = #undef TARGET_LEGITIMATE_ADDRESS_P #define TARGET_LEGITIMATE_ADDRESS_P sh_legitimate_address_p +#undef TARGET_CANNOT_SUBSTITUTE_MEM_EQUIV_P +#define TARGET_CANNOT_SUBSTITUTE_MEM_EQUIV_P sh_cannot_substitute_mem_equiv_p + #undef TARGET_TRAMPOLINE_INIT #define TARGET_TRAMPOLINE_INIT sh_trampoline_init #undef TARGET_TRAMPOLINE_ADJUST_ADDRESS @@ -13214,6 +13218,24 @@ sh_secondary_reload (bool in_p, rtx x, reg_class_t rclass_i, return NO_REGS; } +/* Return true if SUBST can't safely replace its equivalent during RA. */ +static bool +sh_cannot_substitute_mem_equiv_p (rtx) +{ + if (TARGET_SHMEDIA) + return false; + + /* If SUBST is mem[base+index] or QI/HImode mem[base+disp], the insn + uses R0 and may cause spill failure when R0 is already used. + We have to return true for that case at least. + Moreover SH has strong R0 parity and also have not enough numbers of + the hard registers to make the equiv substitution win in the size + and the speed on avarage working sets. The pseudos produced to + hold the equiv values can't get good hard registers for bad cases + and end up memory save/restore insns which make the code worse. */ + return true; +} + static void sh_conditional_register_usage (void) { diff --git a/doc/tm.texi.in b/doc/tm.texi.in index 8f86538..a99cc72 100644 --- a/doc/tm.texi.in +++ b/doc/tm.texi.in @@ -2481,6 +2481,8 @@ as below: @hook TARGET_DIFFERENT_ADDR_DISPLACEMENT_P +@hook TARGET_CANNOT_SUBSTITUTE_MEM_EQUIV_P + @hook TARGET_SPILL_CLASS @hook TARGET_CSTORE_MODE