diff mbox

[AVR] : Clean up hard-coded SFR addresses

Message ID 4F2C39AE.7030708@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay Feb. 3, 2012, 7:46 p.m. UTC 
This patch removes the define_constants from avr.md:
SREG_ADDR, SP_ADDR, RAMPZ_ADDR.

The constants were not used in md directly and didn't take care of afr_offset
between RAM and I/O address.

The replacement is a new structure avr_addr that holds RAM addresses of
respective SFRs and takes into account avr_current_arch->sfr_offset.

sfr_offset is the same for all architectures, but that may change in the future.

Tested without regression.

Ok for trunk?

Johann

	* config/avr/avr.md (SREG_ADDR): Remove constant definition.
	(SP_ADDR): Ditto.
	(RAMPZ_ADDR): Ditto.
	* config/avr/avr.c (avr_addr_t): New typedef.
	(avr_addr): New struct to hold RAM address of SP, RAMPZ, SREG.
	(avr_init_expanders): Initialize it.
	(expand_prologue): Use avr_addr instead of RAMPZ_ADDR, SP_ADDR,
	SREG_ADDR.
	(expand_epilogue): Ditto.
	(avr_print_operand): Ditto.
	(avr_file_start): Ditto.
	(avr_emit_movmemhi): Ditto.

Comments

Weddington, Eric Feb. 3, 2012, 9:14 p.m. UTC | #1 
> -----Original Message-----
> From: Georg-Johann Lay [mailto:avr@gjlay.de]
> Sent: Friday, February 03, 2012 12:47 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Denis Chertykov; Weddington, Eric
> Subject: [Patch,AVR]: Clean up hard-coded SFR addresses
> 
> This patch removes the define_constants from avr.md:
> SREG_ADDR, SP_ADDR, RAMPZ_ADDR.
> 
> The constants were not used in md directly and didn't take care of
afr_offset
> between RAM and I/O address.
> 
> The replacement is a new structure avr_addr that holds RAM addresses
of
> respective SFRs and takes into account avr_current_arch->sfr_offset.
> 
> sfr_offset is the same for all architectures, but that may change in
the
> future.
> 
> Tested without regression.
> 
> Ok for trunk?

In the struct avr_addr_t is there any reason why you didn't want to have
the low and high bytes of the stack pointer in a union with the full
stack pointer?

Eric
Georg-Johann Lay Feb. 4, 2012, 1:29 a.m. UTC | #2 
>>This patch removes the define_constants from avr.md:
>>SREG_ADDR, SP_ADDR, RAMPZ_ADDR.
>>
>>The constants were not used in md directly and didn't take care of
>>afr_offset between RAM and I/O address.
>>
>>The replacement is a new structure avr_addr that holds RAM addresses
>>of respective SFRs and takes into account avr_current_arch->sfr_offset.
>>sfr_offset is the same for all architectures, but that may change in
>>the future.
>>
>>Tested without regression.
>>
>>Ok for trunk?
> 
> In the struct avr_addr_t is there any reason why you didn't want to have
> the low and high bytes of the stack pointer in a union with the full
> stack pointer?

What would be the purpose of such a union?
Removing the .sp = .sp_l sugar? If so, then removing one of them 
altogether is the clearest approach, imo.

Johann
diff mbox

Patch

Index: config/avr/avr.md
===================================================================
--- config/avr/avr.md	(revision 183874)
+++ config/avr/avr.md	(working copy)
@@ -57,12 +57,6 @@  (define_constants
    (LPM_REGNO	0)	; implicit target register of LPM
    (TMP_REGNO	0)	; temporary register r0
    (ZERO_REGNO	1)	; zero register r1
-
-   ;; RAM addresses of some SFRs common to all Devices.
-
-   (SREG_ADDR   0x5F)   ; Status Register
-   (SP_ADDR     0x5D)   ; Stack Pointer
-   (RAMPZ_ADDR  0x5B)   ; Address' high part when loading via ELPM
    ])
 
 (define_c_enum "unspec"
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 183874)
+++ config/avr/avr.c	(working copy)
@@ -104,6 +104,23 @@  static const char* const progmem_section
     ".progmem5.data"
   };
 
+/* Holding RAM addresses of some SFRs used by the compiler and that
+   are unique over all devices in an architecture like 'avr4'.  */
+  
+typedef struct
+{
+  /* SREG: The pocessor status */
+  int sreg;
+
+  /* RAMPZ: The high byte of 24-bit address used with ELPM */ 
+  int rampz;
+
+  /* SP: The stack pointer and its low and high byte */
+  int sp, sp_l, sp_h;
+} avr_addr_t;
+
+static avr_addr_t avr_addr;
+
 
 /* Prototypes for local helper functions.  */
 
@@ -394,6 +411,19 @@  avr_option_override (void)
   avr_current_device = &avr_mcu_types[avr_mcu_index];
   avr_current_arch = &avr_arch_types[avr_current_device->arch];
   avr_extra_arch_macro = avr_current_device->macro;
+  
+  /* RAM addresses of some SFRs common to all Devices in respective Arch. */
+
+  /* SREG: Status Register containing flags like I (global IRQ) */
+  avr_addr.sreg = 0x3F + avr_current_arch->sfr_offset;
+
+  /* RAMPZ: Address' high part when loading via ELPM */
+  avr_addr.rampz = 0x3B + avr_current_arch->sfr_offset;
+
+  /* SP: Stack Pointer (SP_H:SP_L) */
+  avr_addr.sp = 0x3D + avr_current_arch->sfr_offset;
+  avr_addr.sp_l = avr_addr.sp;
+  avr_addr.sp_h = avr_addr.sp + 1;
 
   init_machine_status = avr_init_machine_status;
 
@@ -433,7 +463,7 @@  avr_init_expanders (void)
 
   lpm_addr_reg_rtx = gen_rtx_REG (HImode, REG_Z);
 
-  rampz_rtx = gen_rtx_MEM (QImode, GEN_INT (RAMPZ_ADDR));
+  rampz_rtx = gen_rtx_MEM (QImode, GEN_INT (avr_addr.rampz));
 
   xstring_empty = gen_rtx_CONST_STRING (VOIDmode, "");
   xstring_e = gen_rtx_CONST_STRING (VOIDmode, "e");
@@ -1133,7 +1163,8 @@  expand_prologue (void)
 
       /* Push SREG.  */
       /* ??? There's no dwarf2 column reserved for SREG.  */
-      emit_move_insn (tmp_reg_rtx, gen_rtx_MEM (QImode, GEN_INT (SREG_ADDR)));
+      emit_move_insn (tmp_reg_rtx,
+                      gen_rtx_MEM (QImode, GEN_INT (avr_addr.sreg)));
       emit_push_byte (TMP_REGNO, false);
 
       /* Push RAMPZ.  */
@@ -1386,7 +1417,7 @@  expand_epilogue (bool sibcall_p)
       /* Restore SREG using tmp reg as scratch.  */
       
       emit_pop_byte (TMP_REGNO);
-      emit_move_insn (gen_rtx_MEM (QImode, GEN_INT (SREG_ADDR)), 
+      emit_move_insn (gen_rtx_MEM (QImode, GEN_INT (avr_addr.sreg)), 
                       tmp_reg_rtx);
 
       /* Restore tmp REG.  */
@@ -1869,17 +1900,14 @@  avr_print_operand (FILE *file, rtx x, in
       else if (low_io_address_operand (x, VOIDmode)
                || high_io_address_operand (x, VOIDmode))
         {
-          switch (ival)
+          if (ival == avr_addr.rampz)       fprintf (file, "__RAMPZ__");
+          else if (ival == avr_addr.sreg)   fprintf (file, "__SREG__");
+          else if (ival == avr_addr.sp_l)   fprintf (file, "__SP_L__");
+          else if (ival == avr_addr.sp_h)   fprintf (file, "__SP_H__");
+          else
             {
-            case RAMPZ_ADDR: fprintf (file, "__RAMPZ__"); break;
-            case SREG_ADDR: fprintf (file, "__SREG__"); break;
-            case SP_ADDR:   fprintf (file, "__SP_L__"); break;
-            case SP_ADDR+1: fprintf (file, "__SP_H__"); break;
-              
-            default:
               fprintf (file, HOST_WIDE_INT_PRINT_HEX,
                        ival - avr_current_arch->sfr_offset);
-              break;
             }
         }
       else
@@ -7294,22 +7322,16 @@  avr_file_start (void)
 
   default_file_start ();
 
+  /* Print I/O addresses of some SFRs used with IN and OUT.  */
+
   if (!AVR_HAVE_8BIT_SP)
-    fprintf (asm_out_file,
-             "__SP_H__ = 0x%02x\n",
-             -sfr_offset + SP_ADDR + 1);
-
-  fprintf (asm_out_file,
-           "__SP_L__ = 0x%02x\n"
-           "__SREG__ = 0x%02x\n"
-           "__RAMPZ__ = 0x%02x\n"
-           "__tmp_reg__ = %d\n" 
-           "__zero_reg__ = %d\n",
-           -sfr_offset + SP_ADDR,
-           -sfr_offset + SREG_ADDR,
-           -sfr_offset + RAMPZ_ADDR,
-           TMP_REGNO,
-           ZERO_REGNO);
+    fprintf (asm_out_file, "__SP_H__ = 0x%02x\n", avr_addr.sp_h - sfr_offset);
+
+  fprintf (asm_out_file, "__SP_L__ = 0x%02x\n", avr_addr.sp_l - sfr_offset);
+  fprintf (asm_out_file, "__SREG__ = 0x%02x\n", avr_addr.sreg - sfr_offset);
+  fprintf (asm_out_file, "__RAMPZ__ = 0x%02x\n", avr_addr.rampz - sfr_offset);
+  fprintf (asm_out_file, "__tmp_reg__ = %d\n", TMP_REGNO);
+  fprintf (asm_out_file, "__zero_reg__ = %d\n", ZERO_REGNO);
 }
 
 
@@ -9736,7 +9758,7 @@  avr_emit_movmemhi (rtx *xop)
       emit_move_insn (r23, a_hi8);
       
       insn = fun (addr0, addr1, xas, loop_reg, addr0, addr1,
-                  lpm_reg_rtx, loop_reg16, r23, r23, GEN_INT (RAMPZ_ADDR));
+                  lpm_reg_rtx, loop_reg16, r23, r23, GEN_INT (avr_addr.rampz));
     }
 
   set_mem_addr_space (SET_SRC (XVECEXP (insn, 0, 0)), as);