[patch,avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE

[Georg-Johann Lay <avr at gjlay dot 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.

Index: config/avr/avr-c.c
===================================================================
--- config/avr/avr-c.c	(revision 237643)
+++ config/avr/avr-c.c	(working copy)
@@ -26,7 +26,7 @@
 #include "c-family/c-common.h"
 #include "stor-layout.h"
 #include "langhooks.h"
-
+#include "tm_p.h"
 
 /* IDs for all the AVR builtins.  */
 
@@ -253,7 +253,10 @@ avr_register_target_pragmas (void)
   gcc_assert (ADDR_SPACE_GENERIC == ADDR_SPACE_RAM);
 
   /* Register address spaces.  The order must be the same as in the respective
-     enum from avr.h (or designated initializers must be used in avr.c).  */
+     enum from avr.h (or designated initializers must be used in avr.c).
+     We always register all address spaces even if some of them make no
+     sense for some targets.  Diagnose for non-supported spaces will be
+     emit by TARGET_ADDR_SPACE_DIAGNOSE_USAGE.  */
 
   for (i = 0; i < ADDR_SPACE_COUNT; i++)
     {
@@ -293,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);
@@ -334,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)
@@ -391,10 +395,7 @@ /* Define builtin macros so that the use
             /* Only supply __FLASH<n> macro if the address space is reasonable
                for this target.  The address space qualifier itself is still
                supported, but using it will throw an error.  */
-            && avr_addrspace[i].segment < avr_n_flash
-	    /* Only support __MEMX macro if we have LPM.  */
-	    && (AVR_HAVE_LPM || avr_addrspace[i].pointer_size <= 2))
-
+            && avr_addr_space_supported_p ((addr_space_t) i))
           {
             const char *name = avr_addrspace[i].name;
             char *Name = (char*) alloca (1 + strlen (name));
Index: config/avr/avr-protos.h
===================================================================
--- config/avr/avr-protos.h	(revision 237643)
+++ config/avr/avr-protos.h	(working copy)
@@ -37,6 +37,7 @@ extern void avr_asm_output_aligned_decl_
 extern void avr_asm_asm_output_aligned_bss (FILE *, tree, const char *, unsigned HOST_WIDE_INT, int, void (*) (FILE *, tree, const char *, unsigned HOST_WIDE_INT, int));
 extern void asm_output_external (FILE *file, tree decl, char *name);
 extern int avr_progmem_p (tree decl, tree attributes);
+extern bool avr_addr_space_supported_p (addr_space_t, location_t loc = UNKNOWN_LOCATION);
 
 #ifdef RTX_CODE /* inside TREE_CODE */
 extern void avr_init_cumulative_args (CUMULATIVE_ARGS*, tree, rtx, tree);
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 237643)
+++ config/avr/avr.c	(working copy)
@@ -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:
@@ -8909,6 +8936,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);
 }
 
@@ -9120,6 +9153,42 @@ avr_attribute_table[] =
 };
 
 
+/* Return true if we support address space AS for the architecture in effect
+   and false, otherwise.  If LOC is not UNKNOWN_LOCATION then also issue
+   a respective error.  */
+   
+bool
+avr_addr_space_supported_p (addr_space_t as, location_t loc)
+{
+  if (AVR_TINY)
+    {
+      if (loc != UNKNOWN_LOCATION)
+        error_at (loc, "address spaces are not supported for reduced "
+                  "Tiny devices");
+      return false;
+    }
+  else if (avr_addrspace[as].segment >= avr_n_flash)
+    {
+      if (loc != UNKNOWN_LOCATION)
+        error_at (loc, "address space %qs not supported for devices with "
+                  "flash size up to %d KiB", avr_addrspace[as].name,
+                  64 * avr_n_flash);
+      return false;
+    }
+
+  return true;
+}
+
+
+/* Implement `TARGET_ADDR_SPACE_DIAGNOSE_USAGE'.  */
+
+static bool
+avr_addr_space_diagnose_usage (addr_space_t as, location_t loc)
+{
+  return !avr_addr_space_supported_p (as, loc);
+}
+
+
 /* Look if DECL shall be placed in program memory space by
    means of attribute `progmem' or some address-space qualifier.
    Return non-zero if DECL is data that must end up in Flash and
@@ -9190,16 +9259,13 @@ avr_nonconst_pointer_addrspace (tree typ
       while (TREE_CODE (target) == ARRAY_TYPE)
         target = TREE_TYPE (target);
 
-      /* Pointers to non-generic address space must be const.
-         Refuse address spaces outside the device's flash.  */
+      /* Pointers to non-generic address space must be const.  */
 
       as = TYPE_ADDR_SPACE (target);
 
       if (!ADDR_SPACE_GENERIC_P (as)
-          && (!TYPE_READONLY (target)
-              || avr_addrspace[as].segment >= avr_n_flash
-	      /* Also refuse __memx address space if we can't support it.  */
-	      || (!AVR_HAVE_LPM && avr_addrspace[as].pointer_size > 2)))
+          && !TYPE_READONLY (target)
+          && avr_addr_space_supported_p (as))
         {
           return as;
         }
@@ -9263,25 +9329,13 @@ avr_pgm_check_var_decl (tree node)
 
   if (reason)
     {
-      if (avr_addrspace[as].segment >= avr_n_flash)
-        {
-          if (TYPE_P (node))
-            error ("%qT uses address space %qs beyond flash of %d KiB",
-                   node, avr_addrspace[as].name, 64 * avr_n_flash);
-          else
-            error ("%s %q+D uses address space %qs beyond flash of %d KiB",
-                   reason, node, avr_addrspace[as].name, 64 * avr_n_flash);
-        }
-      else
-        {
-          if (TYPE_P (node))
-            error ("pointer targeting address space %qs must be const in %qT",
-                   avr_addrspace[as].name, node);
-          else
-            error ("pointer targeting address space %qs must be const"
-                   " in %s %q+D",
-                   avr_addrspace[as].name, reason, node);
-        }
+      if (TYPE_P (node))
+        error ("pointer targeting address space %qs must be const in %qT",
+               avr_addrspace[as].name, node);
+      else
+        error ("pointer targeting address space %qs must be const"
+               " in %s %q+D",
+               avr_addrspace[as].name, reason, node);
     }
 
   return reason == NULL;
@@ -9314,18 +9368,6 @@ avr_insert_attributes (tree node, tree *
 
       as = TYPE_ADDR_SPACE (TREE_TYPE (node));
 
-      if (avr_addrspace[as].segment >= avr_n_flash)
-        {
-          error ("variable %q+D located in address space %qs beyond flash "
-                 "of %d KiB", node, avr_addrspace[as].name, 64 * avr_n_flash);
-        }
-      else if (!AVR_HAVE_LPM && avr_addrspace[as].pointer_size > 2)
-	{
-          error ("variable %q+D located in address space %qs"
-                 " which is not supported for architecture %qs",
-                 node, avr_addrspace[as].name, avr_arch->name);
-	}
-
       if (!TYPE_READONLY (node0)
           && !TREE_READONLY (node))
         {
@@ -9566,7 +9608,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);
@@ -9579,7 +9621,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);
@@ -9610,6 +9653,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;
+    }
 }
 
 
@@ -13698,6 +13754,9 @@ avr_fold_builtin (tree fndecl, int n_arg
 #undef  TARGET_ADDR_SPACE_LEGITIMIZE_ADDRESS
 #define TARGET_ADDR_SPACE_LEGITIMIZE_ADDRESS avr_addr_space_legitimize_address
 
+#undef  TARGET_ADDR_SPACE_DIAGNOSE_USAGE
+#define TARGET_ADDR_SPACE_DIAGNOSE_USAGE avr_addr_space_diagnose_usage
+
 #undef  TARGET_MODE_DEPENDENT_ADDRESS_P
 #define TARGET_MODE_DEPENDENT_ADDRESS_P avr_mode_dependent_address_p
 
Index: config/avr/avr.h
===================================================================
--- config/avr/avr.h	(revision 237643)
+++ config/avr/avr.h	(working copy)
@@ -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.
Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 237587)
+++ doc/extend.texi	(working copy)
@@ -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	(working copy)
@@ -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;
+}