diff mbox

Ping^2: A problem simplifying subregs of the hard frame pointer

Message ID 4C58AD79.3030909@codesourcery.com
State New
Headers show

Commit Message

Bernd Schmidt Aug. 3, 2010, 11:59 p.m. UTC 
On 07/31/2010 01:45 PM, Richard Earnshaw wrote:
> This is part of the reload_{in,out}hi sequences.  These have always
> blown my mind away and I've never fully understood when and why these
> are really needed.

Essentially, whenever reload may have to move something from A to B,
but can't do it without a scratch reg.

> In thumb1 you potentially need some sort of recovery sequence if the
> compiler needs to spill a register held in r8-r14 (since these registers
> can't be stored directly to memory), but why it really needs a secondary
> reload escapes me -- the compiler should just copy the register to a
> lo-reg and then store that (or vice-versa for a load).

And I think that happens in all normal cases.  For example, arithmetic
operations use "l" constraints which means reload regs are LO_REGS and
no scratch reg is required to move anything into or out of it.

> I suggest we get rid of the pattern entirely, and the code chain that
> can end up calling it (ie thumb_reload_out_hi.

As below?  I've had this in my last test run (before checking in the
other patches) and that was successful.  It has a slight change in that
it uses TARGET_32BIT instead of TARGET_ARM now.  It may be worthwhile as
an experiment trying to delete the arm branch as well to see if that
causes problems.


Bernd
* config/arm/arm.c (thumb_reload_out_hi, thumb_reload_in_hi):
	Delete.
	* config/arm/arm.md (thumb_movhi_clobber): Delete.
	(reload_outhi): Enabled only for TARGET_32BIT.  Remove Thumb
	branch.
	(reload_inhi): Likewise.
	* config/arm/arm-protos.h (thumb_reload_out_hi, thumb_reload_in_hi):
	Don't declare.

Comments

Bernd Schmidt Aug. 4, 2010, 1:10 a.m. UTC | #1 
On 08/04/2010 01:59 AM, Bernd Schmidt wrote:
> On 07/31/2010 01:45 PM, Richard Earnshaw wrote:
>> This is part of the reload_{in,out}hi sequences.  These have always
>> blown my mind away and I've never fully understood when and why these
>> are really needed.
> 
> Essentially, whenever reload may have to move something from A to B,
> but can't do it without a scratch reg.

Sorry, I was somewhat inaccurate there.  Trying to explain it at 3am
probably won't lead to much better results than at 1am, but anyway...

Normally, you just need to tell reload you need a scratch.  The insn
pattern comes into play when you need to do something more interesting
than to move the value from A to scratch to B.  I think on ARM, that's
primarily for ((MODE) == HImode && ! arm_arch4) where we don't have the
necessary memory access insns IIRC.  Thumb should be OK as far as I can
tell.


Bernd
diff mbox

Patch

Index: gcc/config/arm/arm.c
===================================================================
--- gcc.orig/config/arm/arm.c
+++ gcc/config/arm/arm.c
@@ -20883,19 +20883,6 @@  thumb_expand_movmemqi (rtx *operands)
     }
 }
 
-void
-thumb_reload_out_hi (rtx *operands)
-{
-  emit_insn (gen_thumb_movhi_clobber (operands[0], operands[1], operands[2]));
-}
-
-/* Handle reading a half-word from memory during reload.  */
-void
-thumb_reload_in_hi (rtx *operands ATTRIBUTE_UNUSED)
-{
-  gcc_unreachable ();
-}
-
 /* Return the length of a function name prefix
     that starts with the character 'c'.  */
 static int
Index: gcc/config/arm/arm.md
===================================================================
--- gcc.orig/config/arm/arm.md
+++ gcc/config/arm/arm.md
@@ -5734,23 +5734,6 @@ 
   [(set_attr "predicable" "yes")]
 )
 
-(define_expand "thumb_movhi_clobber"
-  [(set (match_operand:HI     0 "memory_operand"   "")
-	(match_operand:HI     1 "register_operand" ""))
-   (clobber (match_operand:DI 2 "register_operand" ""))]
-  "TARGET_THUMB1"
-  "
-  if (strict_memory_address_p (HImode, XEXP (operands[0], 0))
-      && REGNO (operands[1]) <= LAST_LO_REGNUM)
-    {
-      emit_insn (gen_movhi (operands[0], operands[1]));
-      DONE;
-    }
-  /* XXX Fixme, need to handle other cases here as well.  */
-  gcc_unreachable ();
-  "
-)
-	
 ;; We use a DImode scratch because we may occasionally need an additional
 ;; temporary if the address isn't offsettable -- push_reload doesn't seem
 ;; to take any notice of the "o" constraints on reload_memory_operand operand.
@@ -5758,27 +5741,21 @@ 
   [(parallel [(match_operand:HI 0 "arm_reload_memory_operand" "=o")
 	      (match_operand:HI 1 "s_register_operand"        "r")
 	      (match_operand:DI 2 "s_register_operand"        "=&l")])]
-  "TARGET_EITHER"
-  "if (TARGET_ARM)
-     arm_reload_out_hi (operands);
-   else
-     thumb_reload_out_hi (operands);
+  "TARGET_32BIT"
+{
+  arm_reload_out_hi (operands);
   DONE;
-  "
-)
+})
 
 (define_expand "reload_inhi"
   [(parallel [(match_operand:HI 0 "s_register_operand" "=r")
 	      (match_operand:HI 1 "arm_reload_memory_operand" "o")
 	      (match_operand:DI 2 "s_register_operand" "=&r")])]
-  "TARGET_EITHER"
-  "
-  if (TARGET_ARM)
-    arm_reload_in_hi (operands);
-  else
-    thumb_reload_out_hi (operands);
+  "TARGET_32BIT"
+{
+  arm_reload_in_hi (operands);
   DONE;
-")
+})
 
 (define_expand "movqi"
   [(set (match_operand:QI 0 "general_operand" "")
Index: gcc/config/arm/arm-protos.h
===================================================================
--- gcc.orig/config/arm/arm-protos.h
+++ gcc/config/arm/arm-protos.h
@@ -180,8 +180,6 @@  extern const char *thumb_output_move_mem
 extern const char *thumb_call_via_reg (rtx);
 extern void thumb_expand_movmemqi (rtx *);
 extern rtx arm_return_addr (int, rtx);
-extern void thumb_reload_out_hi (rtx *);
-extern void thumb_reload_in_hi (rtx *);
 extern void thumb_set_return_address (rtx, rtx);
 extern const char *thumb1_output_casesi (rtx *);
 extern const char *thumb2_output_casesi (rtx *);