diff mbox

[AVR] : Fix PR46779

Message ID 4DFA0C13.9060102@redhat.com
State New
Headers show

Commit Message

Richard Henderson June 16, 2011, 1:58 p.m. UTC
On 06/16/2011 04:46 AM, Denis Chertykov wrote:
> I forgot to said that suggestion from Bernd Schmidt about
> HARD_FRAME_POINTER_REGNUM seems very useful:
>> Maybe it would help for your port to define a separate
>> FRAME_POINTER_REGNUM, able to hold an HImode value, which then gets
>> eliminated to HARD_FRAME_POINTER_REGNUM? This mechanism is used on many
>> other ports if you need examples.
> 
> It's not related to addressing modes but it's related to frame pointer bugs.

Yes, that was the first thing I did while investigating G-J's
spill failure.  It didn't help that, of course, but I kept it
around in my tree anyway.

This probably doesn't apply by itself, as is, but for the record...


r~
commit 0ca9df0a8c2722e8a555c2c37d7d7b40467bc5d4
Author: Richard Henderson <rth@redhat.com>
Date:   Wed Jun 15 14:03:19 2011 -0700

    avr: Add soft frame pointer register.

	* config/avr/avr.c (avr_can_eliminate): Simplify.
	(avr_initial_elimination_offset): Likewise.
	(expand_prologue): Use hard_frame_pointer_rtx.
	(expand_epilogue): Likewise.
	(avr_legitimize_address): Gut.
	(avr_hard_regno_nregs): New.
	(avr_hard_regno_ok): Allow only Pmode for arg and frame_pointers.
	(avr_regno_mode_code_ok_for_base_b): Handle arg and frame pointers.
	* config/avr/avr.h (FIXED_REGISTERS): Adjust arg pointer,
	add soft frame pointer.
	(CALL_USED_REGISTERS): Likewise.
	(REG_CLASS_CONTENTS): Likewise.
	(REGISTER_NAMES): Likewise.
	(HARD_REGNO_NREGS): Use avr_hard_regno_nregs.
	(HARD_FRAME_POINTER_REGNUM): New.
	(FRAME_POINTER_REGNUM): Use soft frame pointer.
	(ELIMINABLE_REGS): Eliminate from the soft frame pointer,
	remove the HARD_FRAME_POINTER self-elimination.

Comments

Denis Chertykov June 16, 2011, 6:09 p.m. UTC | #1
2011/6/16 Richard Henderson <rth@redhat.com>:
> On 06/16/2011 04:46 AM, Denis Chertykov wrote:
>> I forgot to said that suggestion from Bernd Schmidt about
>> HARD_FRAME_POINTER_REGNUM seems very useful:
>>> Maybe it would help for your port to define a separate
>>> FRAME_POINTER_REGNUM, able to hold an HImode value, which then gets
>>> eliminated to HARD_FRAME_POINTER_REGNUM? This mechanism is used on many
>>> other ports if you need examples.
>>
>> It's not related to addressing modes but it's related to frame pointer bugs.
>
> Yes, that was the first thing I did while investigating G-J's
> spill failure.  It didn't help that, of course, but I kept it
> around in my tree anyway.
>
> This probably doesn't apply by itself, as is, but for the record...

Only one question why you removed avr_legitimize_address ?

Denis.
diff mbox

Patch

diff --git a/gcc/config/avr/avr-protos.h b/gcc/config/avr/avr-protos.h
index 2af123c..5c60e2e 100644
--- a/gcc/config/avr/avr-protos.h
+++ b/gcc/config/avr/avr-protos.h
@@ -98,6 +98,7 @@  extern int byte_immediate_operand (rtx op, enum machine_mode mode);
 extern int test_hard_reg_class (enum reg_class rclass, rtx x);
 extern int jump_over_one_insn_p (rtx insn, rtx dest);
 
+extern int avr_hard_regno_nregs (int regno, enum machine_mode mode);
 extern int avr_hard_regno_mode_ok (int regno, enum machine_mode mode);
 extern void final_prescan_insn (rtx insn, rtx *operand, int num_operands);
 extern int avr_simplify_comparison_p (enum machine_mode mode,
diff --git a/gcc/config/avr/avr.c b/gcc/config/avr/avr.c
index c6087fe..e1d6b50 100644
--- a/gcc/config/avr/avr.c
+++ b/gcc/config/avr/avr.c
@@ -493,29 +493,28 @@  avr_regs_to_save (HARD_REG_SET *set)
 /* Return true if register FROM can be eliminated via register TO.  */
 
 bool
-avr_can_eliminate (const int from, const int to)
+avr_can_eliminate (int from ATTRIBUTE_UNUSED, int to)
 {
-  return ((from == ARG_POINTER_REGNUM && to == FRAME_POINTER_REGNUM)
-	  || ((from == FRAME_POINTER_REGNUM 
-	       || from == FRAME_POINTER_REGNUM + 1)
-	      && !frame_pointer_needed));
+  return to == HARD_FRAME_POINTER_REGNUM;
 }
 
 /* Compute offset between arg_pointer and frame_pointer.  */
 
 int
-avr_initial_elimination_offset (int from, int to)
+avr_initial_elimination_offset (int from, int to ATTRIBUTE_UNUSED)
 {
-  if (from == FRAME_POINTER_REGNUM && to == STACK_POINTER_REGNUM)
-    return 0;
-  else
-    {
-      int offset = frame_pointer_needed ? 2 : 0;
-      int avr_pc_size = AVR_HAVE_EIJMP_EICALL ? 3 : 2;
+  int offset = 0;
 
+  if (from == ARG_POINTER_REGNUM)
+    {
+      offset += AVR_HAVE_EIJMP_EICALL ? 3 : 2;
+      offset += frame_pointer_needed ? 2 : 0;
       offset += avr_regs_to_save (NULL);
-      return get_frame_size () + (avr_pc_size) + 1 + offset;
+      offset += get_frame_size ();
+      offset += 1; /* post-dec stack space */
     }
+
+  return offset;
 }
 
 /* Actual start of frame is virtual_stack_vars_rtx this is offset from 
@@ -747,12 +746,12 @@  expand_prologue (void)
 	 notes to the front.  Thus we build them in the reverse order of
 	 how we want dwarf2out to process them.  */
 
-      /* The function does always set frame_pointer_rtx, but whether that
+      /* The function does always set hard_frame_pointer_rtx, but whether that
 	 is going to be permanent in the function is frame_pointer_needed.  */
       add_reg_note (insn, REG_CFA_ADJUST_CFA,
 		    gen_rtx_SET (VOIDmode,
 				 (frame_pointer_needed
-				  ? frame_pointer_rtx : stack_pointer_rtx),
+				  ? hard_frame_pointer_rtx : stack_pointer_rtx),
 				 plus_constant (stack_pointer_rtx,
 						-(size + live_seq))));
 
@@ -793,7 +792,7 @@  expand_prologue (void)
 
           if (!size)
             {
-              insn = emit_move_insn (frame_pointer_rtx, stack_pointer_rtx);
+              insn = emit_move_insn (hard_frame_pointer_rtx, stack_pointer_rtx);
               RTX_FRAME_RELATED_P (insn) = 1;
             }
           else
@@ -816,12 +815,12 @@  expand_prologue (void)
                 {
                   /* The high byte (r29) doesn't change.  Prefer 'subi'
 		     (1 cycle) over 'sbiw' (2 cycles, same size).  */
-                  myfp = gen_rtx_REG (QImode, FRAME_POINTER_REGNUM);
+                  myfp = gen_rtx_REG (QImode, HARD_FRAME_POINTER_REGNUM);
                 }
               else 
                 {
                   /*  Normal sized addition.  */
-                  myfp = frame_pointer_rtx;
+                  myfp = hard_frame_pointer_rtx;
                 }
 
 	      /* Method 1-Adjust frame pointer.  */
@@ -833,12 +832,12 @@  expand_prologue (void)
 		 instead indicate that the entire operation is complete after
 		 the frame pointer subtraction is done.  */
 
-              emit_move_insn (frame_pointer_rtx, stack_pointer_rtx);
+              emit_move_insn (hard_frame_pointer_rtx, stack_pointer_rtx);
 
               insn = emit_move_insn (myfp, plus_constant (myfp, -size));
               RTX_FRAME_RELATED_P (insn) = 1;
 	      add_reg_note (insn, REG_CFA_ADJUST_CFA,
-			    gen_rtx_SET (VOIDmode, frame_pointer_rtx,
+			    gen_rtx_SET (VOIDmode, hard_frame_pointer_rtx,
 					 plus_constant (stack_pointer_rtx,
 							-size)));
 
@@ -847,23 +846,23 @@  expand_prologue (void)
 		 need not be annotated at all.  */
 	      if (AVR_HAVE_8BIT_SP)
 		{
-		  emit_move_insn (stack_pointer_rtx, frame_pointer_rtx);
+		  emit_move_insn (stack_pointer_rtx, hard_frame_pointer_rtx);
 		}
 	      else if (TARGET_NO_INTERRUPTS 
 		       || cfun->machine->is_signal
 		       || cfun->machine->is_OS_main)
 		{
 		  emit_insn (gen_movhi_sp_r_irq_off (stack_pointer_rtx, 
-						     frame_pointer_rtx));
+						     hard_frame_pointer_rtx));
 		}
 	      else if (cfun->machine->is_interrupt)
 		{
 		  emit_insn (gen_movhi_sp_r_irq_on (stack_pointer_rtx, 
-						    frame_pointer_rtx));
+						    hard_frame_pointer_rtx));
 		}
 	      else
 		{
-		  emit_move_insn (stack_pointer_rtx, frame_pointer_rtx);
+		  emit_move_insn (stack_pointer_rtx, hard_frame_pointer_rtx);
 		}
 
 	      fp_plus_insns = get_insns ();
@@ -880,7 +879,7 @@  expand_prologue (void)
 		  insn = emit_move_insn (stack_pointer_rtx, insn);
 		  RTX_FRAME_RELATED_P (insn) = 1;
 		  
-		  insn = emit_move_insn (frame_pointer_rtx, stack_pointer_rtx);
+		  insn = emit_move_insn (hard_frame_pointer_rtx, stack_pointer_rtx);
 		  RTX_FRAME_RELATED_P (insn) = 1;
 
 		  sp_plus_insns = get_insns ();
@@ -997,13 +996,13 @@  expand_epilogue (bool sibcall_p)
       if (frame_pointer_needed)
 	{
           /*  Get rid of frame.  */
-	  emit_move_insn(frame_pointer_rtx,
-                         gen_rtx_PLUS (HImode, frame_pointer_rtx,
-                                       gen_int_mode (size, HImode)));
+	  emit_move_insn (hard_frame_pointer_rtx,
+                          gen_rtx_PLUS (HImode, hard_frame_pointer_rtx,
+                                        gen_int_mode (size, HImode)));
 	}
       else
 	{
-          emit_move_insn (frame_pointer_rtx, stack_pointer_rtx);
+          emit_move_insn (hard_frame_pointer_rtx, stack_pointer_rtx);
 	}
 	
       emit_insn (gen_epilogue_restores (gen_int_mode (live_seq, HImode)));
@@ -1022,12 +1021,12 @@  expand_epilogue (bool sibcall_p)
                 {
                   /* The high byte (r29) doesn't change - prefer 'subi' 
                      (1 cycle) over 'sbiw' (2 cycles, same size).  */
-                  myfp = gen_rtx_REG (QImode, FRAME_POINTER_REGNUM);
+                  myfp = gen_rtx_REG (QImode, HARD_FRAME_POINTER_REGNUM);
                 }
               else 
                 {
                   /* Normal sized addition.  */
-                  myfp = frame_pointer_rtx;
+                  myfp = hard_frame_pointer_rtx;
                 }
 	      
               /* Method 1-Adjust frame pointer.  */
@@ -1038,22 +1037,22 @@  expand_epilogue (bool sibcall_p)
 	      /* Copy to stack pointer.  */
 	      if (AVR_HAVE_8BIT_SP)
 		{
-		  emit_move_insn (stack_pointer_rtx, frame_pointer_rtx);
+		  emit_move_insn (stack_pointer_rtx, hard_frame_pointer_rtx);
 		}
 	      else if (TARGET_NO_INTERRUPTS 
 		       || cfun->machine->is_signal)
 		{
 		  emit_insn (gen_movhi_sp_r_irq_off (stack_pointer_rtx, 
-						     frame_pointer_rtx));
+						     hard_frame_pointer_rtx));
 		}
 	      else if (cfun->machine->is_interrupt)
 		{
 		  emit_insn (gen_movhi_sp_r_irq_on (stack_pointer_rtx, 
-						    frame_pointer_rtx));
+						    hard_frame_pointer_rtx));
 		}
 	      else
 		{
-		  emit_move_insn (stack_pointer_rtx, frame_pointer_rtx);
+		  emit_move_insn (stack_pointer_rtx, hard_frame_pointer_rtx);
 		}
 
 	      fp_plus_insns = get_insns ();
@@ -1228,32 +1227,8 @@  avr_legitimate_address_p (enum machine_mode mode, rtx x, bool strict)
    memory address for an operand of mode MODE  */
 
 rtx
-avr_legitimize_address (rtx x, rtx oldx, enum machine_mode mode)
+avr_legitimize_address (rtx x, rtx oldx ATTRIBUTE_UNUSED, enum machine_mode mode ATTRIBUTE_UNUSED)
 {
-  x = oldx;
-  if (TARGET_ALL_DEBUG)
-    {
-      fprintf (stderr, "legitimize_address mode: %s", GET_MODE_NAME(mode));
-      debug_rtx (oldx);
-    }
-  
-  if (GET_CODE (oldx) == PLUS
-      && REG_P (XEXP (oldx,0)))
-    {
-      if (REG_P (XEXP (oldx,1)))
-	x = force_reg (GET_MODE (oldx), oldx);
-      else if (GET_CODE (XEXP (oldx, 1)) == CONST_INT)
-	{
-	  int offs = INTVAL (XEXP (oldx,1));
-	  if (frame_pointer_rtx != XEXP (oldx,0))
-	    if (offs > MAX_LD_OFFSET (mode))
-	      {
-		if (TARGET_ALL_DEBUG)
-		  fprintf (stderr, "force_reg (big offset)\n");
-		x = force_reg (GET_MODE (oldx), oldx);
-	      }
-	}
-    }
   return x;
 }
 
@@ -6096,8 +6071,7 @@  extra_constraint_Q (rtx x)
 	return 1;		/* allocate pseudos */
       else if (regno == REG_Z || regno == REG_Y)
 	return 1;		/* strictly check */
-      else if (xx == frame_pointer_rtx
-	       || xx == arg_pointer_rtx)
+      else if (xx == frame_pointer_rtx || xx == arg_pointer_rtx)
 	return 1;		/* XXX frame & arg pointer checks */
     }
   return 0;
@@ -6280,6 +6254,18 @@  jump_over_one_insn_p (rtx insn, rtx dest)
   return dest_addr - jump_addr == get_attr_length (insn) + 1;
 }
 
+/* Returns the number of registers required to hold a value of MODE.  */
+
+int
+avr_hard_regno_nregs (int regno, enum machine_mode mode)
+{
+  /* The fake registers are designed to hold exactly a pointer.  */
+  if (regno == ARG_POINTER_REGNUM || regno == FRAME_POINTER_REGNUM)
+    return 1;
+
+  return (GET_MODE_SIZE (mode) + UNITS_PER_WORD - 1) / UNITS_PER_WORD;
+}
+
 /* Returns 1 if a value of mode MODE can be stored starting with hard
    register number REGNO.  On the enhanced core, anything larger than
    1 byte must start in even numbered register for "movw" to work
@@ -6288,6 +6274,10 @@  jump_over_one_insn_p (rtx insn, rtx dest)
 int
 avr_hard_regno_mode_ok (int regno, enum machine_mode mode)
 {
+  /* The fake registers are designed to hold exactly a pointer.  */
+  if (regno == ARG_POINTER_REGNUM || regno == FRAME_POINTER_REGNUM)
+    return mode == Pmode;
+
   /* Any GENERAL_REGS register can hold 8-bit values.  */
   /* FIXME:  8-bit values must not be disallowed for R28 or R29.
      Disallowing QI et al. in these registers might lead to code like
@@ -6299,8 +6289,7 @@  avr_hard_regno_mode_ok (int regno, enum machine_mode mode)
   if (GET_MODE_SIZE (mode) == 1)
      return 1;
    
-   /* All modes larger than 8 bits should start in an even register.  */
-   
+  /* All modes larger than 8 bits should start in an even register.  */
   return regno % 2 == 0;
 }
 
@@ -6332,27 +6321,35 @@  avr_mode_code_base_reg_class (enum machine_mode mode ATTRIBUTE_UNUSED,
 /* Worker function for `REGNO_MODE_CODE_OK_FOR_BASE_P'.  */
 
 bool
-avr_regno_mode_code_ok_for_base_p (int regno, enum machine_mode mode ATTRIBUTE_UNUSED,
-                                   RTX_CODE outer_code, RTX_CODE index_code ATTRIBUTE_UNUSED)
+avr_regno_mode_code_ok_for_base_p (int regno,
+				   enum machine_mode mode ATTRIBUTE_UNUSED,
+                                   RTX_CODE outer_code,
+				   RTX_CODE index_code ATTRIBUTE_UNUSED)
 {
   bool ok;
   
+  ok = (regno == REG_Z
+        || regno == REG_Y
+        || regno == ARG_POINTER_REGNUM
+        || regno == FRAME_POINTER_REGNUM);
+
   switch (outer_code)
     {
     case PLUS:
-      ok = regno == REG_Z || regno == REG_Y;
+      /* Computed above */
       break;
       
     case MEM: /* plain reg */
     case POST_INC:
     case PRE_DEC:
-      ok = regno == REG_Z || regno == REG_Y || regno == REG_X;
+      /* As above, but also X.  */
+      if (regno == REG_X)
+	ok = true;
       break;
       
     default:
       ok = false;
       break;
-      
     }
 
   return ok;
diff --git a/gcc/config/avr/avr.h b/gcc/config/avr/avr.h
index 9c64f96..426ddec 100644
--- a/gcc/config/avr/avr.h
+++ b/gcc/config/avr/avr.h
@@ -191,7 +191,8 @@  extern GTY(()) section *progmem_section;
   0,0,/* r28 r29 */\
   0,0,/* r30 r31 */\
   1,1,/*  STACK */\
-  1,1 /* arg pointer */  }
+  1,  /* arg pointer */\
+  1   /* frame pointer */  }
 
 #define CALL_USED_REGISTERS {			\
   1,1,/* r0 r1 */				\
@@ -211,7 +212,8 @@  extern GTY(()) section *progmem_section;
     0,0,/* r28 r29 */				\
     1,1,/* r30 r31 */				\
     1,1,/*  STACK */				\
-    1,1 /* arg pointer */  }
+    1,  /* arg pointer */			\
+    1   /* frame pointer */ }
 
 #define REG_ALLOC_ORDER {			\
     24,25,					\
@@ -229,7 +231,7 @@  extern GTY(()) section *progmem_section;
 #define ADJUST_REG_ALLOC_ORDER order_regs_for_local_alloc ()
 
 
-#define HARD_REGNO_NREGS(REGNO, MODE) ((GET_MODE_SIZE (MODE) + UNITS_PER_WORD - 1) / UNITS_PER_WORD)
+#define HARD_REGNO_NREGS(REGNO, MODE) avr_hard_regno_nregs(REGNO, MODE)
 
 #define HARD_REGNO_MODE_OK(REGNO, MODE) avr_hard_regno_mode_ok(REGNO, MODE)
 
@@ -279,17 +281,17 @@  enum reg_class {
   {3 << REG_Z,0x00000000},      /* POINTER_Z_REGS, r30 - r31 */		\
   {0x00000000,0x00000003},	/* STACK_REG, STACK */			\
   {(3 << REG_Y) | (3 << REG_Z),						\
-     0x00000000},		/* BASE_POINTER_REGS, r28 - r31 */	\
+     0x0000000c},		/* BASE_POINTER_REGS, r28 - r31,ap,fp */\
   {(3 << REG_X) | (3 << REG_Y) | (3 << REG_Z),				\
-     0x00000000},		/* POINTER_REGS, r26 - r31 */		\
+     0x0000000c},		/* POINTER_REGS, r26 - r31,ap,fp */	\
   {(3 << REG_X) | (3 << REG_Y) | (3 << REG_Z) | (3 << REG_W),		\
      0x00000000},		/* ADDW_REGS, r24 - r31 */		\
   {0x00ff0000,0x00000000},	/* SIMPLE_LD_REGS r16 - r23 */          \
   {(3 << REG_X)|(3 << REG_Y)|(3 << REG_Z)|(3 << REG_W)|(0xff << 16),	\
-     0x00000000},	/* LD_REGS, r16 - r31 */			\
+     0x0000000c},	/* LD_REGS, r16 - r31 */			\
   {0x0000ffff,0x00000000},	/* NO_LD_REGS  r0 - r15 */              \
-  {0xffffffff,0x00000000},	/* GENERAL_REGS, r0 - r31 */		\
-  {0xffffffff,0x00000003}	/* ALL_REGS */				\
+  {0xffffffff,0x0000000c},	/* GENERAL_REGS, r0 - r31,ap,fp */	\
+  {0xffffffff,0x0000000f}	/* ALL_REGS */				\
 }
 
 #define REGNO_REG_CLASS(R) avr_regno_reg_class(R)
@@ -322,16 +324,18 @@  enum reg_class {
 
 #define STACK_POINTER_REGNUM 32
 
-#define FRAME_POINTER_REGNUM REG_Y
+#define HARD_FRAME_POINTER_REGNUM REG_Y
 
 #define ARG_POINTER_REGNUM 34
+#define FRAME_POINTER_REGNUM 35
 
 #define STATIC_CHAIN_REGNUM 2
 
 #define ELIMINABLE_REGS {					\
-      {ARG_POINTER_REGNUM, FRAME_POINTER_REGNUM},		\
-	{FRAME_POINTER_REGNUM, STACK_POINTER_REGNUM}		\
-       ,{FRAME_POINTER_REGNUM+1,STACK_POINTER_REGNUM+1}}
+      { ARG_POINTER_REGNUM, STACK_POINTER_REGNUM },		\
+      { ARG_POINTER_REGNUM, HARD_FRAME_POINTER_REGNUM },	\
+      { FRAME_POINTER_REGNUM, STACK_POINTER_REGNUM },		\
+      { FRAME_POINTER_REGNUM, HARD_FRAME_POINTER_REGNUM }}
 
 #define INITIAL_ELIMINATION_OFFSET(FROM, TO, OFFSET)			\
   OFFSET = avr_initial_elimination_offset (FROM, TO)
@@ -582,7 +586,7 @@  sprintf (STRING, "*.%s%lu", PREFIX, (unsigned long)(NUM))
     "r8","r9","r10","r11","r12","r13","r14","r15",	\
     "r16","r17","r18","r19","r20","r21","r22","r23",	\
     "r24","r25","r26","r27","r28","r29","r30","r31",	\
-    "__SP_L__","__SP_H__","argL","argH"}
+    "__SP_L__","__SP_H__","ap","fp"}
 
 #define FINAL_PRESCAN_INSN(insn, operand, nop) final_prescan_insn (insn, operand,nop)