diff mbox

[RFC,4/9] Add TARGET_LEGITIMIZE_ADDRESS_DISPLACEMENT target macro

Message ID 20141218.100046.337578930.kkojima@rr.iij4u.or.jp
State New
Headers show

Commit Message

Kaz Kojima Dec. 18, 2014, 1 a.m. UTC
This patch adds a target macro discussed in PR55212

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55212#c76
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55212#c78

is to avoid bad codes on SH.
SH has very limited base+displacement addressing and it looks
that the old reload has some mechanism so to split
  reg := mem[base + bad_displacement]
to
  new_base := base + (bad_displacement - legitimate_displacement)
  reg := mem[new_base + legitimate_displacement]
The new target macro is to give same effect on LRA.
This helps SH especially for the address like stack_pointer + offset
where offset has 4-bit except for SH2A.
I've tried to define this target macro for ARM thumb and found
only one CSiBE test replaypc-0.4.0.preproc/build-ndx is improved
at 4 bytes with -Os.  I guess that 8-bit offset field for sp+offset
addressing on ARM thumb is very good for this issue.
Again it would be better if something can cover this case without
new target macros.
--
	* lra-constraints.c (process_address_1): Try if target can split
	displacement with targetm.legitimize_address_displacement.
	* target.def (legitimize_address_displacement): New hook.
	* targhooks.c (default_legitimize_address_displacement): New function.
	* targhooks.h (default_legitimize_address_displacement): Declare.
	* config/sh/sh.c (sh_legitimize_address_displacement): New function.
	(TARGET_LEGITIMIZE_ADDRESS_DISPLACEMENT): Define.
	* doc/tm.texi.in (TARGET_LEGITIMIZE_ADDRESS_DISPLACEMENT): New hook.
	* doc/tm.texi: Regenerate.

Comments

Vladimir Makarov Dec. 18, 2014, 9:18 p.m. UTC | #1
On 2014-12-17 8:00 PM, Kaz Kojima wrote:
> This patch adds a target macro discussed in PR55212
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55212#c76
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55212#c78
>
> is to avoid bad codes on SH.
> SH has very limited base+displacement addressing and it looks
> that the old reload has some mechanism so to split

Yes, that is probably last thing of reload missed in LRA.  As I remember 
Joern wrote this very long ago specifically for SH.  postreload pass 
also has some code to deal with different displacements and their costs 
(if there are no constraints on the constants).

>    reg := mem[base + bad_displacement]
> to
>    new_base := base + (bad_displacement - legitimate_displacement)
>    reg := mem[new_base + legitimate_displacement]
> The new target macro is to give same effect on LRA.
> This helps SH especially for the address like stack_pointer + offset
> where offset has 4-bit except for SH2A.
> I've tried to define this target macro for ARM thumb and found
> only one CSiBE test replaypc-0.4.0.preproc/build-ndx is improved
> at 4 bytes with -Os.  I guess that 8-bit offset field for sp+offset
> addressing on ARM thumb is very good for this issue.
> Again it would be better if something can cover this case without
> new target macros.
> --
> 	* lra-constraints.c (process_address_1): Try if target can split
> 	displacement with targetm.legitimize_address_displacement.
> 	* target.def (legitimize_address_displacement): New hook.
> 	* targhooks.c (default_legitimize_address_displacement): New function.
> 	* targhooks.h (default_legitimize_address_displacement): Declare.
> 	* config/sh/sh.c (sh_legitimize_address_displacement): New function.
> 	(TARGET_LEGITIMIZE_ADDRESS_DISPLACEMENT): Define.
> 	* doc/tm.texi.in (TARGET_LEGITIMIZE_ADDRESS_DISPLACEMENT): New hook.
> 	* doc/tm.texi: Regenerate.
>

It is an interesting solution.

The patch is ok for me.

One more thing though.  The patches you propose affect very sensitive 
parts of LRA.  They might break other ports currently using LRA.  If it 
happens, please, revert a breaking patch as I can not be a help next 2 
weeks.
Kaz Kojima Dec. 19, 2014, 1:04 a.m. UTC | #2
Vladimir Makarov <vmakarov@redhat.com> wrote:
>> 	* lra-constraints.c (process_address_1): Try if target can split
>> 	displacement with targetm.legitimize_address_displacement.
>> 	* target.def (legitimize_address_displacement): New hook.
>> 	* targhooks.c (default_legitimize_address_displacement): New function.
>> 	* targhooks.h (default_legitimize_address_displacement): Declare.
>> 	* config/sh/sh.c (sh_legitimize_address_displacement): New function.
>> 	(TARGET_LEGITIMIZE_ADDRESS_DISPLACEMENT): Define.
>> 	* doc/tm.texi.in (TARGET_LEGITIMIZE_ADDRESS_DISPLACEMENT): New hook.
>> 	* doc/tm.texi: Regenerate.
>>
> 
> It is an interesting solution.
> 
> The patch is ok for me.
> 
> One more thing though.  The patches you propose affect very sensitive
> parts of LRA.  They might break other ports currently using LRA.  If
> it happens, please, revert a breaking patch as I can not be a help
> next 2 weeks.

I'll revert them ASAP if it causes something on trunk.
Thanks for your review!

Regards,
	kaz
diff mbox

Patch

diff --git a/lra-constraints.c b/lra-constraints.c
index 1666380..7bdb288 100644
--- a/lra-constraints.c
+++ b/lra-constraints.c
@@ -3009,6 +3009,32 @@  process_address_1 (int nop, bool check_only_p,
 	      delete_insns_since (PREV_INSN (last_insn));
 	    }
 	}
+      /* Try if target can split displacement into legitimite new disp
+	 and offset.  If it's the case, we replace the last insn with
+	 insns for base + offset => new_reg and set new_reg + new disp
+	 to *ad.inner.  */
+      last_insn = get_last_insn ();
+      if ((set = single_set (last_insn)) != NULL_RTX
+	  && GET_CODE (SET_SRC (set)) == PLUS
+	  && REG_P (XEXP (SET_SRC (set), 0))
+	  && REGNO (XEXP (SET_SRC (set), 0)) < FIRST_PSEUDO_REGISTER
+	  && CONST_INT_P (XEXP (SET_SRC (set), 1)))
+	{
+	  rtx addend, disp = XEXP (SET_SRC (set), 1);
+	  if (targetm.legitimize_address_displacement (&disp, &addend,
+						       ad.mode))
+	    {
+	      rtx_insn *new_insns;
+	      start_sequence ();
+	      lra_emit_add (new_reg, XEXP (SET_SRC (set), 0), addend);
+	      new_insns = get_insns ();
+	      end_sequence ();
+	      new_reg = gen_rtx_PLUS (Pmode, new_reg, disp);
+	      delete_insns_since (PREV_INSN (last_insn));
+	      add_insn (new_insns);
+	      insns = get_insns ();
+	    }
+	}
       end_sequence ();
       emit_insn (insns);
       *ad.inner = new_reg;
diff --git a/target.def b/target.def
index b0c86b8..c21c9e3 100644
--- a/target.def
+++ b/target.def
@@ -5051,6 +5051,19 @@  can be used to avoid excessive spilling.",
  bool, (rtx subst),
  hook_bool_rtx_false)
 
+/* This target hook allows the backend to legitimize base plus
+   displacement addressing.  */
+DEFHOOK
+(legitimize_address_displacement,
+ "A target hook which returns @code{true} if *@var{disp} is\n\
+legitimezed to valid address displacement with subtracting *@var{offset}\n\
+at memory mode @var{mode}.\n\
+The default version of this target hook returns @code{false}.\n\
+This hook will benefit machines with limited base plus displacement\n\
+addressing.",
+ bool, (rtx *disp, rtx *offset, machine_mode mode),
+ default_legitimize_address_displacement)
+
 /* This target hook allows the backend to perform additional
    processing while initializing for variable expansion.  */
 DEFHOOK
diff --git a/targhooks.c b/targhooks.c
index 42fd82e..5e723b4 100644
--- a/targhooks.c
+++ b/targhooks.c
@@ -167,6 +167,14 @@  default_legitimize_address (rtx x, rtx orig_x ATTRIBUTE_UNUSED,
   return x;
 }
 
+bool
+default_legitimize_address_displacement (rtx *disp ATTRIBUTE_UNUSED,
+					 rtx *offset ATTRIBUTE_UNUSED,
+					 machine_mode mode ATTRIBUTE_UNUSED)
+{
+  return false;
+}
+
 rtx
 default_expand_builtin_saveregs (void)
 {
diff --git a/targhooks.h b/targhooks.h
index 9734220..26e4f5f 100644
--- a/targhooks.h
+++ b/targhooks.h
@@ -24,6 +24,8 @@  extern bool default_legitimate_address_p (machine_mode, rtx, bool);
 
 extern void default_external_libcall (rtx);
 extern rtx default_legitimize_address (rtx, rtx, machine_mode);
+extern bool default_legitimize_address_displacement (rtx *, rtx *,
+						     machine_mode);
 
 extern int default_unspec_may_trap_p (const_rtx, unsigned);
 extern machine_mode default_promote_function_mode (const_tree, machine_mode,
diff --git a/config/sh/sh.c b/config/sh/sh.c
index c445199..6f34b33 100644
--- a/config/sh/sh.c
+++ b/config/sh/sh.c
@@ -292,6 +292,7 @@  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 bool sh_legitimize_address_displacement (rtx *, rtx *, machine_mode);
 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 *);
@@ -634,6 +636,10 @@  static const struct attribute_spec sh_attribute_table[] =
 #undef TARGET_CANNOT_SUBSTITUTE_MEM_EQUIV_P
 #define TARGET_CANNOT_SUBSTITUTE_MEM_EQUIV_P sh_cannot_substitute_mem_equiv_p
 
+#undef TARGET_LEGITIMIZE_ADDRESS_DISPLACEMENT
+#define TARGET_LEGITIMIZE_ADDRESS_DISPLACEMENT \
+  sh_legitimize_address_displacement
+
 #undef TARGET_TRAMPOLINE_INIT
 #define TARGET_TRAMPOLINE_INIT		sh_trampoline_init
 #undef TARGET_TRAMPOLINE_ADJUST_ADDRESS
@@ -13236,6 +13242,29 @@  sh_cannot_substitute_mem_equiv_p (rtx)
   return true;
 }
 
+/* Return true if DISP can be legitimized.  */
+static bool
+sh_legitimize_address_displacement (rtx *disp, rtx *offs,
+				    machine_mode mode)
+{
+  if (TARGET_SHMEDIA)
+    return false;
+
+  if (((TARGET_SH4 || TARGET_SH2A_DOUBLE) && mode == DFmode)
+      || (TARGET_SH2E && mode == SFmode))
+    return false;
+
+  struct disp_adjust adj = sh_find_mov_disp_adjust (mode, INTVAL (*disp));
+  if (adj.offset_adjust != NULL_RTX && adj.mov_disp != NULL_RTX)
+    {
+      *disp = adj.mov_disp;
+      *offs = adj.offset_adjust;
+      return true;
+    }
+ 
+  return false;
+}
+
 static void
 sh_conditional_register_usage (void)
 {
diff --git a/doc/tm.texi.in b/doc/tm.texi.in
index a99cc72..20c0129 100644
--- a/doc/tm.texi.in
+++ b/doc/tm.texi.in
@@ -2483,6 +2483,8 @@  as below:
 
 @hook TARGET_CANNOT_SUBSTITUTE_MEM_EQUIV_P
 
+@hook TARGET_LEGITIMIZE_ADDRESS_DISPLACEMENT
+
 @hook TARGET_SPILL_CLASS
 
 @hook TARGET_CSTORE_MODE