diff mbox

Provide a hook for target to disable register renaming for some instructions

Message ID 4C2B84B7.2000506@codesourcery.com
State New
Headers show

Commit Message

Jie Zhang June 30, 2010, 5:53 p.m. UTC 
When compiler an application for ARM, the GAS issued a warning:

Warning: register range not in ascending order

for the instruction

push	{ip, r3, r4, lr}

Before regrename pass, this instruction looked like

push	{r0, r3, r4, lr}

Then GCC renamed r0 to ip in the regrename pass.  Such kind of register 
renaming is bad since the resulted register list is not in ascending 
order any more. And each register in the list was pushed at a wrong 
memory address.

This patch adds a target hook to provide a way for target to disable 
register renaming in such case when necessary.

Bootstrap and regression tested on x86_64. Regression tested for ARM 
NEON on arm-none-eabi target. Both are OK.

I have no small test case can be included. :-(

Is it OK?


Regards,

Comments

Richard Henderson June 30, 2010, 6:03 p.m. UTC | #1 
On 06/30/2010 10:53 AM, Jie Zhang wrote:
> When compiler an application for ARM, the GAS issued a warning:
> 
> Warning: register range not in ascending order
> 
> for the instruction
> 
> push    {ip, r3, r4, lr}
> 
> Before regrename pass, this instruction looked like
> 
> push    {r0, r3, r4, lr}

Doesn't it work just as well to simply remove the register
constraint from the push_multi instruction?  Without that
the regrename pass won't get a register class for the 
operand and will leave it alone.


r~
Jie Zhang June 30, 2010, 6:46 p.m. UTC | #2 
On 07/01/2010 02:03 AM, Richard Henderson wrote:
> On 06/30/2010 10:53 AM, Jie Zhang wrote:
>> When compiler an application for ARM, the GAS issued a warning:
>>
>> Warning: register range not in ascending order
>>
>> for the instruction
>>
>> push    {ip, r3, r4, lr}
>>
>> Before regrename pass, this instruction looked like
>>
>> push    {r0, r3, r4, lr}
>
> Doesn't it work just as well to simply remove the register
> constraint from the push_multi instruction?  Without that
> the regrename pass won't get a register class for the
> operand and will leave it alone.
>
Thanks! I never thought about it. It works. I will do a full testing.


Jie
diff mbox

Patch


	* regrename.c: Include "target.h".
	(regrename_optimize): Call targetm.regrename_ok.
	* targhooks.c (default_regrename_ok): New.
	* targhooks.h (default_regrename_ok): Declare.
	* target.def (regrename_ok): New.
	* doc/tm.texi.in: Document TARGET_REGRENAME_OK.
	* doc/tm.texi: Regenerate.
	* config/arm/arm.c (arm_regrename_ok): New.
	(TARGET_REGRENAME_OK): Define.
	* config/arm/arm.md (*push_multi): Rename to push_multi.

Index: regrename.c
===================================================================
--- regrename.c	(revision 161569)
+++ regrename.c	(working copy)
@@ -36,6 +36,7 @@ 
 #include "flags.h"
 #include "toplev.h"
 #include "obstack.h"
+#include "target.h"
 #include "timevar.h"
 #include "tree-pass.h"
 #include "df.h"
@@ -301,7 +302,8 @@  regrename_optimize (void)
 			&& ! (HARD_REGNO_CALL_PART_CLOBBERED
 			      (reg, GET_MODE (*tmp->loc)))
 			&& (HARD_REGNO_CALL_PART_CLOBBERED
-			    (new_reg, GET_MODE (*tmp->loc)))))
+			    (new_reg, GET_MODE (*tmp->loc))))
+		    || ! targetm.regrename_ok (tmp->insn, reg, new_reg))
 		  break;
 	      if (! tmp)
 		{
Index: doc/tm.texi.in
===================================================================
--- doc/tm.texi.in	(revision 161569)
+++ doc/tm.texi.in	(working copy)
@@ -2256,6 +2256,18 @@  is not saved by a prologue in an interru
 The default version of this hook always returns @code{true}.
 @end deftypefn
 
+@hook TARGET_REGRENAME_OK
+
+This target hook should return @code{true} if it is OK to rename a register
+@var{reg} to a new register @var{new_reg}.
+
+This macro can be used to prevent register renaming in some cases.  ARM port uses
+this macro to prevent register renaming generating non-ascending register list
+in push multiple instruction.
+
+The default version of this hook always returns @code{true}.
+@end deftypefn
+
 @defmac AVOID_CCMODE_COPIES
 Define this macro if the compiler should avoid copies to/from @code{CCmode}
 registers.  You should only define this macro if support for copying to/from
Index: targhooks.c
===================================================================
--- targhooks.c	(revision 161569)
+++ targhooks.c	(working copy)
@@ -1102,6 +1102,16 @@  default_mode_dependent_address_p (const_
 #endif
 }
 
+/* The default implementation of TARGET_REGRENAME_OK.  */
+
+bool
+default_regrename_ok (rtx insn ATTRIBUTE_UNUSED,
+		      int reg ATTRIBUTE_UNUSED,
+		      int new_reg ATTRIBUTE_UNUSED)
+{
+  return true;
+}
+
 bool
 default_target_option_valid_attribute_p (tree ARG_UNUSED (fndecl),
 					 tree ARG_UNUSED (name),
Index: targhooks.h
===================================================================
--- targhooks.h	(revision 161569)
+++ targhooks.h	(working copy)
@@ -130,6 +130,7 @@  extern tree default_mangle_decl_assemble
 extern tree default_emutls_var_fields (tree, tree *);
 extern tree default_emutls_var_init (tree, tree, tree);
 extern bool default_hard_regno_scratch_ok (unsigned int);
+extern bool default_regrename_ok (rtx, int, int);
 extern bool default_mode_dependent_address_p (const_rtx addr);
 extern bool default_target_option_valid_attribute_p (tree, tree, tree, int);
 extern bool default_target_option_pragma_parse (tree, tree);
Index: target.def
===================================================================
--- target.def	(revision 161569)
+++ target.def	(working copy)
@@ -1980,6 +1980,13 @@  DEFHOOK
  bool, (unsigned int regno),
  default_hard_regno_scratch_ok)
 
+/* Return true if is OK to rename REG to NEW_REG in INSN.  */
+DEFHOOK
+(regrename_ok,
+ "",
+ bool, (rtx insn, int reg, int new_reg),
+ default_regrename_ok)
+
 /* Return the smallest number of different values for which it is best to
    use a jump-table instead of a tree of conditional branches.  */
 DEFHOOK
Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c	(revision 161569)
+++ config/arm/arm.c	(working copy)
@@ -219,6 +219,7 @@  static const char *arm_invalid_parameter
 static const char *arm_invalid_return_type (const_tree t);
 static tree arm_promoted_type (const_tree t);
 static tree arm_convert_to_type (tree type, tree expr);
+static bool arm_regrename_ok (rtx, int, int);
 static bool arm_scalar_mode_supported_p (enum machine_mode);
 static bool arm_frame_pointer_required (void);
 static bool arm_can_eliminate (const int, const int);
@@ -507,6 +508,9 @@  static const struct attribute_spec arm_a
 #undef TARGET_CONVERT_TO_TYPE
 #define TARGET_CONVERT_TO_TYPE arm_convert_to_type
 
+#undef TARGET_REGRENAME_OK
+#define TARGET_REGRENAME_OK arm_regrename_ok
+
 #undef TARGET_SCALAR_MODE_SUPPORTED_P
 #define TARGET_SCALAR_MODE_SUPPORTED_P arm_scalar_mode_supported_p
 
@@ -18118,6 +18122,43 @@  arm_convert_to_type (tree type, tree exp
   return NULL_TREE;
 }
 
+/* Implement TARGET_REGRENAME_OK.
+   We cannot rename REG to NEW_REG in INSN if INSN is a push_multi instruction
+   and the registers aren't in ascending order any more after renaming.  */
+
+static bool
+arm_regrename_ok (rtx insn, int reg, int new_reg)
+{
+  if (recog_memoized (insn) == CODE_FOR_push_multi)
+    {
+      int regs[MAX_RECOG_OPERANDS + 2];
+      int num_regs, i;
+
+      extract_insn (insn);
+
+      num_regs = XVECLEN (recog_data.operand[2], 0);
+
+      if (num_regs == 1)
+	return true;
+
+      regs[0] = -1;
+      regs[1] = REGNO (recog_data.operand[1]);
+      regs[num_regs + 1] = FIRST_PSEUDO_REGISTER;
+      for (i = 1; i < num_regs; i++)
+	regs[i + 1] = REGNO (XEXP (XVECEXP (recog_data.operand[2], 0, i), 0));
+
+      for (i = 1; i <= num_regs; i++)
+	if (reg == regs[i])
+	  {
+	    if (new_reg > regs[i - 1] && new_reg < regs[i + 1])
+	      return true;
+	    else
+	      return false;
+	  }
+    }
+  return true;
+}
+
 /* Implement TARGET_SCALAR_MODE_SUPPORTED_P.
    This simply adds HFmode as a supported mode; even though we don't
    implement arithmetic on this type directly, it's supported by
Index: config/arm/arm.md
===================================================================
--- config/arm/arm.md	(revision 161569)
+++ config/arm/arm.md	(working copy)
@@ -10868,7 +10868,7 @@ 
 ;; Push multiple registers to the stack.  Registers are in parallel (use ...)
 ;; expressions.  For simplicity, the first register is also in the unspec
 ;; part.
-(define_insn "*push_multi"
+(define_insn "push_multi"
   [(match_parallel 2 "multi_register_push"
     [(set (match_operand:BLK 0 "memory_operand" "=m")
 	  (unspec:BLK [(match_operand:SI 1 "s_register_operand" "r")]