diff mbox

[avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE

Message ID a6b89297-adf7-a1c4-171a-ecf96006c855@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay July 20, 2016, 2:27 p.m. UTC
On 18.07.2016 08:58, Denis Chertykov wrote:
> 2016-07-15 18:26 GMT+03:00 Georg-Johann Lay <avr@gjlay.de>:
>> This patch needs new hook TARGET_ADDR_SPACE_DIAGNOSE_USAGE:
>> https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00839.html
>>
>> This patch turns attribute progmem into a working feature for AVR_TINY
>> cores.
>>
>> It boils down to adding 0x4000 to all symbols with progmem:  Flash memory
>> can be seen in the RAM address space starting at 0x4000, i.e. data in flash
>> can be read by means of LD instruction if we add offsets of 0x4000.  There
>> is no need for special access macros like pgm_read_* or special address
>> spaces as there is nothing like a LPM instruction.
>>
>> This is simply achieved by setting a respective symbol_ref_flag, and when
>> such a symbol has to be printed, then plus_constant with 0x4000 is used.
>>
>> Diagnosing of unsupported address spaces is now performed by
>> TARGET_ADDR_SPACE_DIAGNOSE_USAGE which has exact location information.
>> Hence there is no need to scan all decls for invalid address spaces.
>>
>> For AVR_TINY, alls address spaces have been disabled.  They are of no use.
>> Supporting __flash would just make the backend more complicated without any
>> gains.
>>
>>
>> Ok for trunk?
>>
>> Johann
>>
>>
>> gcc/
>>         * doc/extend.texi (AVR Variable Attributes) [progmem]: Add
>>         documentation how it works on reduced Tiny cores.
>>         (AVR Named Address Spaces): No support for reduced Tiny.
>>         * avr-protos.h (avr_addr_space_supported_p): New prototype.
>>         * avr.c (AVR_SYMBOL_FLAG_TINY_PM): New macro.
>>         (avr_address_tiny_pm_p): New static function.
>>         (avr_print_operand_address) [AVR_TINY]: Add AVR_TINY_PM_OFFSET
>>         if the address is in progmem.
>>         (avr_assemble_integer): Same.
>>         (avr_encode_section_info) [AVR_TINY]: Set AVR_SYMBOL_FLAG_TINY_PM
>>         for symbol_ref in progmem.
>>         (TARGET_ADDR_SPACE_DIAGNOSE_USAGE): New hook define...
>>         (avr_addr_space_diagnose_usage): ...and implementation.
>>         (avr_addr_space_supported_p): New function.
>>         (avr_nonconst_pointer_addrspace, avr_pgm_check_var_decl): Only
>>         report bad address space usage if that space is supported.
>>         (avr_insert_attributes): Same.  No more complain about unsupported
>>         address spaces.
>>         * avr.h (AVR_TINY_PM_OFFSET): New macro.
>>         * avr-c.c (tm_p.h): Include it.
>>         (avr_cpu_cpp_builtins) [__AVR_TINY_PM_BASE_ADDRESS__]: Use
>>         AVR_TINY_PM_OFFSET instead of magic 0x4000 when built-in def'ing.
>>         Only define addr-space related built-in macro if
>>         avr_addr_space_supported_p.
>> gcc/testsuite/
>>         * gcc.target/avr/torture/tiny-progmem.c: New test.
>>
>
> Approved.

Committed, but I split it into 2 change-sets.  The only effective change is 
that the hook has a different prototype (returns void instead of bool).


Part1: Implement new target hook TARGET_ADDR_SPACE_DIAGNOSE_USAGE.

https://gcc.gnu.org/r238519

gcc/
	* avr-protos.h (avr_addr_space_supported_p): New prototype.
	* avr.c (TARGET_ADDR_SPACE_DIAGNOSE_USAGE): New hook define...
	(avr_addr_space_diagnose_usage): ...and implementation.
	(avr_addr_space_supported_p): New function.
	(avr_nonconst_pointer_addrspace, avr_pgm_check_var_decl): Only
	report bad address space usage if that space is supported.
	(avr_insert_attributes): Same.  No more complain about unsupported
	address spaces.
	* avr-c.c (tm_p.h): Include it.
	(avr_cpu_cpp_builtins):	Only define addr-space related built-in
	macro if avr_addr_space_supported_p.

Part2: Make progmem work for reduced Tiny cores

https://gcc.gnu.org/r238525

gcc/
	Implement attribute progmem on reduced Tiny cores by adding
	flash offset 0x4000 to respective symbols.

	PR target/71948
	* doc/extend.texi (AVR Variable Attributes) [progmem]: Add
	documentation how it works on reduced Tiny cores.
	(AVR Named Address Spaces): No support for reduced Tiny.
	* config/avr/avr.c (AVR_SYMBOL_FLAG_TINY_PM): New macro.
	(avr_address_tiny_pm_p): New static function.
	(avr_print_operand_address) [AVR_TINY]: Add AVR_TINY_PM_OFFSET
	if the address is in progmem.
	(avr_assemble_integer): Same.
	(avr_encode_section_info) [AVR_TINY]: Set AVR_SYMBOL_FLAG_TINY_PM
	for symbol_ref in progmem.
	* config/avr/avr.h (AVR_TINY_PM_OFFSET): New macro.
	* config/avr/avr-c.c (avr_cpu_cpp_builtins): Use it instead of
	magic 0x4000 when built-in def'ing __AVR_TINY_PM_BASE_ADDRESS__.
gcc/testsuite/
	PR target/71948
	* gcc.target/avr/torture/tiny-progmem.c: New test.
diff mbox

Patch

Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 238524)
+++ doc/extend.texi	(revision 238525)
@@ -1422,6 +1422,11 @@  const __memx void *pfoo = &foo;
 Such code requires at least binutils 2.23, see
 @w{@uref{http://sourceware.org/PR13503,PR13503}}.
 
+@item
+On the reduced Tiny devices like ATtiny40, no address spaces are supported.
+Data can be put into and read from flash memory by means of
+attribute @code{progmem}, see @ref{AVR Variable Attributes}.
+
 @end itemize
 
 @subsection M32C Named Address Spaces
@@ -5847,10 +5852,12 @@  attribute accomplishes this by putting r
 section whose name starts with @code{.progmem}.
 
 This attribute works similar to the @code{section} attribute
-but adds additional checking. Notice that just like the
-@code{section} attribute, @code{progmem} affects the location
-of the data but not how this data is accessed.
+but adds additional checking.
 
+@table @asis
+@item @bullet{}@tie{} Ordinary AVR cores with 32 general purpose registers:
+@code{progmem} affects the location
+of the data but not how this data is accessed.
 In order to read data located with the @code{progmem} attribute
 (inline) assembler must be used.
 @smallexample
@@ -5873,6 +5880,28 @@  normally resides in the data memory (RAM
 See also the @ref{AVR Named Address Spaces} section for
 an alternate way to locate and access data in flash memory.
 
+@item @bullet{}@tie{}Reduced AVR Tiny cores like ATtiny40:
+The compiler adds @code{0x4000}
+to the addresses of objects and declarations in @code{progmem} and locates
+the objects in flash memory, namely in section @code{.progmem.data}.
+The offset is needed because the flash memory is visible in the RAM
+address space starting at address @code{0x4000}.
+
+Data in @code{progmem} can be accessed by means of ordinary C@tie{}code,
+no special functions or macros are needed.
+
+@smallexample
+/* var is located in flash memory */
+extern const int var[2] __attribute__((progmem));
+
+int read_var (int i)
+@{
+    return var[i];
+@}
+@end smallexample
+
+@end table
+
 @item io
 @itemx io (@var{addr})
 @cindex @code{io} variable attribute, AVR
Index: testsuite/gcc.target/avr/torture/tiny-progmem.c
===================================================================
--- testsuite/gcc.target/avr/torture/tiny-progmem.c	(nonexistent)
+++ testsuite/gcc.target/avr/torture/tiny-progmem.c	(revision 238525)
@@ -0,0 +1,109 @@ 
+/* { dg-do run } */
+/* { dg-options "-Wl,--defsym,test6_xdata=0" } */
+
+#ifdef __AVR_TINY__
+#define PM __attribute__((__progmem__))
+#else
+/* On general core, just resort to vanilla C. */
+#define PM /* Empty */
+#endif
+
+#define PSTR(s) (__extension__({ static const char __c[] PM = (s); &__c[0];}))
+
+#define NI __attribute__((noinline,noclone))
+
+const volatile int data[] PM = { 1234, 5678 };
+const volatile int * volatile pdata = &data[1];
+
+int ram[2];
+
+const int myvar PM = 42;
+extern const int xvar __asm ("myvar") PM;
+
+NI int const volatile* get_addr_1 (void)
+{
+  return &data[1];
+}
+
+NI int const volatile* get_addr_x (int x)
+{
+  return &data[x];
+}
+
+void test_1 (void)
+{
+  if (data[0] != 1234)
+    __builtin_abort();
+
+  if (data[1] != 5678)
+    __builtin_abort();
+}
+
+void test_2 (void)
+{
+  if (data[1] != 5678)
+    __builtin_abort();
+}
+
+void test_3 (void)
+{
+  if (&data[1] != pdata)
+    __builtin_abort();
+}
+
+void test_4 (void)
+{
+  if (5678 != *get_addr_1())
+    __builtin_abort();
+  if (5678 != *get_addr_x(1))
+    __builtin_abort();
+}
+
+void test_5 (void)
+{
+  __builtin_memcpy (&ram, (void*) &data, 4);
+  if (ram[0] - ram[1] != 1234 - 5678)
+    __builtin_abort();
+}
+
+const char pmSTR[] PM = "01234";
+
+NI const char* get_pmSTR (int i)
+{
+  return pmSTR + 2 + i;
+}
+
+void test_6 (void)
+{
+#ifdef __AVR_TINY__
+  extern const int test6_xdata PM;
+  const char* str = PSTR ("Hallo");
+  if (0 == (__AVR_TINY_PM_BASE_ADDRESS__ & (__UINTPTR_TYPE__) str))
+    __builtin_abort();
+  if (0 == (__AVR_TINY_PM_BASE_ADDRESS__ & (__UINTPTR_TYPE__) test6_xdata))
+    __builtin_abort();
+#endif
+  
+  if (get_pmSTR (0)[0] != '0' + 2)
+    __builtin_abort();
+  if (get_pmSTR (1)[0] != '1' + 2)
+    __builtin_abort();
+}
+
+void test_7 (void)
+{
+  if (xvar != 42)
+    __builtin_abort();
+}
+
+int main()
+{
+  test_1();
+  test_2();
+  test_3();
+  test_4();
+  test_5();
+  test_6();
+  test_7();
+  return 0;
+}
Index: config/avr/avr-c.c
===================================================================
--- config/avr/avr-c.c	(revision 238524)
+++ config/avr/avr-c.c	(revision 238525)
@@ -296,7 +296,7 @@  avr_cpu_cpp_builtins (struct cpp_reader
   builtin_define_std ("AVR");
 
   /* __AVR_DEVICE_NAME__ and  avr_mcu_types[].macro like __AVR_ATmega8__
-	 are defined by -D command option, see device-specs file.  */
+     are defined by -D command option, see device-specs file.  */
 
   if (avr_arch->macro)
     cpp_define_formatted (pfile, "__AVR_ARCH__=%s", avr_arch->macro);
@@ -337,7 +337,8 @@  start address.  This macro shall be used
          it has been mapped to the data memory.  For AVR_TINY devices
          (ATtiny4/5/9/10/20 and 40) mapped program memory starts at 0x4000. */
 
-      cpp_define (pfile, "__AVR_TINY_PM_BASE_ADDRESS__=0x4000");
+      cpp_define_formatted (pfile, "__AVR_TINY_PM_BASE_ADDRESS__=0x%x",
+                            AVR_TINY_PM_OFFSET);
     }
 
   if (AVR_HAVE_EIJMP_EICALL)
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 238524)
+++ config/avr/avr.c	(revision 238525)
@@ -80,6 +80,10 @@ 
   ((SYMBOL_REF_FLAGS (sym) & AVR_SYMBOL_FLAG_PROGMEM)           \
    / SYMBOL_FLAG_MACH_DEP)
 
+/* (AVR_TINY only): Symbol has attribute progmem */
+#define AVR_SYMBOL_FLAG_TINY_PM \
+  (SYMBOL_FLAG_MACH_DEP << 4)
+
 #define TINY_ADIW(REG1, REG2, I)                                \
     "subi " #REG1 ",lo8(-(" #I "))" CR_TAB                      \
     "sbci " #REG2 ",hi8(-(" #I "))"
@@ -2161,12 +2165,35 @@  cond_string (enum rtx_code code)
 }
 
 
+/* Return true if rtx X is a CONST or SYMBOL_REF with progmem.
+   This must be used for AVR_TINY only because on other cores
+   the flash memory is not visible in the RAM address range and
+   cannot be read by, say,  LD instruction.  */
+
+static bool
+avr_address_tiny_pm_p (rtx x)
+{
+  if (CONST == GET_CODE (x))
+    x = XEXP (XEXP (x, 0), 0);
+
+  if (SYMBOL_REF_P (x))
+    return SYMBOL_REF_FLAGS (x) & AVR_SYMBOL_FLAG_TINY_PM;
+
+  return false;
+}
+
 /* Implement `TARGET_PRINT_OPERAND_ADDRESS'.  */
 /* Output ADDR to FILE as address.  */
 
 static void
 avr_print_operand_address (FILE *file, machine_mode /*mode*/, rtx addr)
 {
+  if (AVR_TINY
+      && avr_address_tiny_pm_p (addr))
+    {
+      addr = plus_constant (Pmode, addr, AVR_TINY_PM_OFFSET);
+    }
+
   switch (GET_CODE (addr))
     {
     case REG:
@@ -8937,6 +8964,12 @@  avr_assemble_integer (rtx x, unsigned in
       return true;
     }
 
+  if (AVR_TINY
+      && avr_address_tiny_pm_p (x))
+    {
+      x = plus_constant (Pmode, x, AVR_TINY_PM_OFFSET);
+    }
+
   return default_assemble_integer (x, size, aligned_p);
 }
 
@@ -9603,7 +9636,7 @@  avr_encode_section_info (tree decl, rtx
   if (decl && DECL_P (decl)
       && TREE_CODE (decl) != FUNCTION_DECL
       && MEM_P (rtl)
-      && SYMBOL_REF == GET_CODE (XEXP (rtl, 0)))
+      && SYMBOL_REF_P (XEXP (rtl, 0)))
    {
       rtx sym = XEXP (rtl, 0);
       tree type = TREE_TYPE (decl);
@@ -9616,7 +9649,8 @@  avr_encode_section_info (tree decl, rtx
       /* PSTR strings are in generic space but located in flash:
          patch address space.  */
 
-      if (-1 == avr_progmem_p (decl, attr))
+      if (!AVR_TINY
+          && -1 == avr_progmem_p (decl, attr))
         as = ADDR_SPACE_FLASH;
 
       AVR_SYMBOL_SET_ADDR_SPACE (sym, as);
@@ -9647,6 +9681,19 @@  avr_encode_section_info (tree decl, rtx
       if (addr_attr && !DECL_EXTERNAL (decl))
 	SYMBOL_REF_FLAGS (sym) |= SYMBOL_FLAG_ADDRESS;
     }
+
+  if (AVR_TINY
+      && decl
+      && VAR_DECL == TREE_CODE (decl)
+      && -1 == avr_progmem_p (decl, DECL_ATTRIBUTES (decl))
+      && MEM_P (rtl)
+      && SYMBOL_REF_P (XEXP (rtl, 0)))
+    {
+      /* Tag symbols for later addition of 0x4000 (AVR_TINY_PM_OFFSET).  */
+
+      rtx sym = XEXP (rtl, 0);
+      SYMBOL_REF_FLAGS (sym) |= AVR_SYMBOL_FLAG_TINY_PM;
+    }
 }
 
 
Index: config/avr/avr.h
===================================================================
--- config/avr/avr.h	(revision 238524)
+++ config/avr/avr.h	(revision 238525)
@@ -74,6 +74,8 @@  enum
                         || avr_arch->have_rampd)
 #define AVR_HAVE_EIJMP_EICALL (avr_arch->have_eijmp_eicall)
 
+#define AVR_TINY_PM_OFFSET (0x4000)
+
 /* Handling of 8-bit SP versus 16-bit SP is as follows:
 
 FIXME: DRIVER_SELF_SPECS has changed.