public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch] Add new hook to diagnose address space usage
@ 2016-07-14 15:12 Georg-Johann Lay
  2016-07-15 15:27 ` [patch,avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE Georg-Johann Lay
  2016-07-18 14:13 ` [patch] Add new hook to diagnose address space usage Bernd Schmidt
  0 siblings, 2 replies; 7+ messages in thread
From: Georg-Johann Lay @ 2016-07-14 15:12 UTC (permalink / raw)
  To: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1545 bytes --]

This adds a new hook that allows to emit better diagnostics if an address space 
is used that's not available.

One solution would be no not register the address space with 
c_register_addr_space but that gives ugly report like

error: expected '=', ',', ';', 'asm' or '__attribute__' before 'f321'

The hook allows better diagnostics:  The address spaces are registered with 
c_register_addr_space and if the parser comes across an address space it 
provides the hook with the needed information, in particular the location of 
the token so that the message would be something like

ptiny.c:12:11: error: address space '__flash2' not supported for devices with 
flash size up to 128 KiB
  const int __flash2 f321[] = { 321, 654 };
            ^~~~~~~~

This only works if address spaces are part of the language dialect.

Default implementation is void.

The intended user is the avr backend which currently emits diagnostics in some 
hook like TARGET_INSERT_ATTRIBUTER, but the locations are out of sync there.

Ok for trunk if bootstrap passes?

Johann


gcc/
	* target.def (addr_space): Add new diagnose_usage to hook vector.
	* targhooks.c (default_addr_space_diagnose_usage): Add default
	implementation and...
	* targhooks.h (default_addr_space_diagnose_usage): ... its prototype.
	* c/c-parser.c (c_lex_one_token) [CPP_NAME]: If the token
	is some address space, call targetm.addr_space.diagnose_usage.
	* doc/tm.texi.in (Named Address Spaces): Add anchor for
	TARGET_ADDR_SPACE_DIAGNOSE_USAGE documentation.
	* doc/tm.texi: Regenerate.

[-- Attachment #2: addr-space-usage.diff --]
[-- Type: text/x-patch, Size: 4304 bytes --]

Index: target.def
===================================================================
--- target.def	(revision 237587)
+++ target.def	(working copy)
@@ -3241,6 +3241,21 @@ The result is the value to be used with
  int, (addr_space_t as),
  default_addr_space_debug)
 
+/* Function to emit custom diagnostic if an address space is used.  */
+DEFHOOK
+(diagnose_usage,
+ "Define this hook if the availability of an address space depends on\n\
+command line options and some diagnostics shall be printed when the\n\
+address space is used.  This hook is called during parsing and allows\n\
+to emit a better diagnostic compared to the case where the address space\n\
+was not registered with @code{c_register_addr_space}.  @var{as} is\n\
+the address space as registered with @code{c_register_addr_space}.\n\
+@var{loc} is the location of the address space qualifier token.\n\
+Return true if the hook issued an error and false, otherwise.\n\
+The default implementation does nothing.",
+ bool, (addr_space_t as, location_t loc),
+ default_addr_space_diagnose_usage)
+
 HOOK_VECTOR_END (addr_space)
 
 #undef HOOK_PREFIX
Index: targhooks.c
===================================================================
--- targhooks.c	(revision 237587)
+++ targhooks.c	(working copy)
@@ -1291,6 +1291,14 @@ default_addr_space_debug (addr_space_t a
   return as;
 }
 
+bool
+default_addr_space_diagnose_usage (addr_space_t ARG_UNUSED (as),
+				   location_t ARG_UNUSED (loc))
+{
+  return false;
+}
+	 
+
 /* The default hook for TARGET_ADDR_SPACE_CONVERT. This hook should never be
    called for targets with only a generic address space.  */
 
Index: targhooks.h
===================================================================
--- targhooks.h	(revision 237587)
+++ targhooks.h	(working copy)
@@ -181,6 +181,7 @@ extern rtx default_addr_space_legitimize
 extern bool default_addr_space_subset_p (addr_space_t, addr_space_t);
 extern bool default_addr_space_zero_address_valid (addr_space_t);
 extern int default_addr_space_debug (addr_space_t);
+extern bool default_addr_space_diagnose_usage (addr_space_t, location_t);
 extern rtx default_addr_space_convert (rtx, tree, tree);
 extern unsigned int default_case_values_threshold (void);
 extern bool default_have_conditional_execution (void);
Index: c/c-parser.c
===================================================================
--- c/c-parser.c	(revision 237587)
+++ c/c-parser.c	(working copy)
@@ -300,6 +300,9 @@ c_lex_one_token (c_parser *parser, c_tok
 	    else if (rid_code >= RID_FIRST_ADDR_SPACE
 		     && rid_code <= RID_LAST_ADDR_SPACE)
 	      {
+		addr_space_t as;
+		as = (addr_space_t) (rid_code - RID_FIRST_ADDR_SPACE);
+		targetm.addr_space.diagnose_usage (as, token->location);
 		token->id_kind = C_ID_ADDRSPACE;
 		token->keyword = rid_code;
 		break;
Index: doc/tm.texi.in
===================================================================
--- doc/tm.texi.in	(revision 237587)
+++ doc/tm.texi.in	(working copy)
@@ -7486,6 +7486,8 @@ c_register_addr_space ("__ea", ADDR_SPAC
 
 @hook TARGET_ADDR_SPACE_DEBUG
 
+@hook TARGET_ADDR_SPACE_DIAGNOSE_USAGE
+
 @node Misc
 @section Miscellaneous Parameters
 @cindex parameters, miscellaneous
Index: doc/tm.texi
===================================================================
--- doc/tm.texi	(revision 237587)
+++ doc/tm.texi	(working copy)
@@ -10431,6 +10431,18 @@ Define this to define how the address sp
 The result is the value to be used with @code{DW_AT_address_class}.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_ADDR_SPACE_DIAGNOSE_USAGE (addr_space_t @var{as}, location_t @var{loc})
+Define this hook if the availability of an address space depends on
+command line options and some diagnostics shall be printed when the
+address space is used.  This hook is called during parsing and allows
+to emit a better diagnostic compared to the case where the address space
+was not registered with @code{c_register_addr_space}.  @var{as} is
+the address space as registered with @code{c_register_addr_space}.
+@var{loc} is the location of the address space qualifier token.
+Return true if the hook issued an error and false, otherwise.
+The default implementation does nothing.
+@end deftypefn
+
 @node Misc
 @section Miscellaneous Parameters
 @cindex parameters, miscellaneous

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [patch,avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE
  2016-07-14 15:12 [patch] Add new hook to diagnose address space usage Georg-Johann Lay
@ 2016-07-15 15:27 ` Georg-Johann Lay
  2016-07-18  6:58   ` Denis Chertykov
  2016-07-18 14:13 ` [patch] Add new hook to diagnose address space usage Bernd Schmidt
  1 sibling, 1 reply; 7+ messages in thread
From: Georg-Johann Lay @ 2016-07-15 15:27 UTC (permalink / raw)
  To: gcc-patches; +Cc: Senthil Kumar Selvaraj, Denis Chertykov

[-- Attachment #1: Type: text/plain, Size: 2355 bytes --]

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.


[-- Attachment #2: tiny-progmem-2.diff --]
[-- Type: text/x-patch, Size: 15401 bytes --]

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;
+}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch,avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE
  2016-07-15 15:27 ` [patch,avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE Georg-Johann Lay
@ 2016-07-18  6:58   ` Denis Chertykov
  2016-07-20 14:27     ` Georg-Johann Lay
  0 siblings, 1 reply; 7+ messages in thread
From: Denis Chertykov @ 2016-07-18  6:58 UTC (permalink / raw)
  To: Georg-Johann Lay; +Cc: gcc-patches, Senthil Kumar Selvaraj

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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch] Add new hook to diagnose address space usage
  2016-07-14 15:12 [patch] Add new hook to diagnose address space usage Georg-Johann Lay
  2016-07-15 15:27 ` [patch,avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE Georg-Johann Lay
@ 2016-07-18 14:13 ` Bernd Schmidt
  2016-07-19  8:16   ` [patch] Add new hook to diagnose address space usage (take #2) Georg-Johann Lay
  1 sibling, 1 reply; 7+ messages in thread
From: Bernd Schmidt @ 2016-07-18 14:13 UTC (permalink / raw)
  To: Georg-Johann Lay, GCC Patches

On 07/14/2016 05:11 PM, Georg-Johann Lay wrote:
> The hook allows better diagnostics:  The address spaces are registered
> with c_register_addr_space and if the parser comes across an address
> space it provides the hook with the needed information, in particular
> the location of the token so that the message would be something like

Looks reasonable, except...

> +(diagnose_usage,
> + "Define this hook if the availability of an address space depends on\n\
> +command line options and some diagnostics shall be printed when the\n\

"should", not "shall", I think.

>
> +bool
> +default_addr_space_diagnose_usage (addr_space_t ARG_UNUSED (as),
> +				   location_t ARG_UNUSED (loc))
> +{
> +  return false;
> +}

The return value is not used, so it should return void. That would also 
match the documentation you added (which says "does nothing" rather than 
"returns false").

Remove unused arg names in default hook implementations, I think.


Bernd

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch] Add new hook to diagnose address space usage (take #2)
  2016-07-18 14:13 ` [patch] Add new hook to diagnose address space usage Bernd Schmidt
@ 2016-07-19  8:16   ` Georg-Johann Lay
  2016-07-20 11:10     ` Bernd Schmidt
  0 siblings, 1 reply; 7+ messages in thread
From: Georg-Johann Lay @ 2016-07-19  8:16 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1795 bytes --]

On 18.07.2016 16:13, Bernd Schmidt wrote:
> On 07/14/2016 05:11 PM, Georg-Johann Lay wrote:
>> The hook allows better diagnostics:  The address spaces are registered
>> with c_register_addr_space and if the parser comes across an address
>> space it provides the hook with the needed information, in particular
>> the location of the token so that the message would be something like
>
> Looks reasonable, except...
>
>> +(diagnose_usage,
>> + "Define this hook if the availability of an address space depends on\n\
>> +command line options and some diagnostics shall be printed when the\n\
>
> "should", not "shall", I think.

Fixed.

>> +bool
>> +default_addr_space_diagnose_usage (addr_space_t ARG_UNUSED (as),
>> +                   location_t ARG_UNUSED (loc))
>> +{
>> +  return false;
>> +}
>
> The return value is not used, so it should return void. That would also match
> the documentation you added (which says "does nothing" rather than "returns
> false").

Fixed, the hook returns void now.

The idea was that in a future version the c-parser might take decision 
depending on whether an error has been issued.

> Remove unused arg names in default hook implementations, I think.
>
>
> Bernd
>


Done.  Attached is the updated version of the change, log entry is the same as 
before.

Johann


gcc/
	* target.def (addr_space): Add new diagnose_usage to hook vector.
	* targhooks.c (default_addr_space_diagnose_usage): Add default
	implementation and...
	* targhooks.h (default_addr_space_diagnose_usage): ... its prototype.
	* c/c-parser.c (c_lex_one_token) [CPP_NAME]: If the token
	is some address space, call targetm.addr_space.diagnose_usage.
	* doc/tm.texi.in (Named Address Spaces): Add anchor for
	TARGET_ADDR_SPACE_DIAGNOSE_USAGE documentation.
	* doc/tm.texi: Regenerate.


[-- Attachment #2: addr-space-usage-2.diff --]
[-- Type: text/x-patch, Size: 4243 bytes --]

Index: c/c-parser.c
===================================================================
--- c/c-parser.c	(revision 238425)
+++ c/c-parser.c	(working copy)
@@ -301,6 +301,9 @@ c_lex_one_token (c_parser *parser, c_tok
 	    else if (rid_code >= RID_FIRST_ADDR_SPACE
 		     && rid_code <= RID_LAST_ADDR_SPACE)
 	      {
+		addr_space_t as;
+		as = (addr_space_t) (rid_code - RID_FIRST_ADDR_SPACE);
+		targetm.addr_space.diagnose_usage (as, token->location);
 		token->id_kind = C_ID_ADDRSPACE;
 		token->keyword = rid_code;
 		break;
Index: doc/tm.texi
===================================================================
--- doc/tm.texi	(revision 238425)
+++ doc/tm.texi	(working copy)
@@ -10431,6 +10431,17 @@ Define this to define how the address sp
 The result is the value to be used with @code{DW_AT_address_class}.
 @end deftypefn
 
+@deftypefn {Target Hook} void TARGET_ADDR_SPACE_DIAGNOSE_USAGE (addr_space_t @var{as}, location_t @var{loc})
+Define this hook if the availability of an address space depends on
+command line options and some diagnostics should be printed when the
+address space is used.  This hook is called during parsing and allows
+to emit a better diagnostic compared to the case where the address space
+was not registered with @code{c_register_addr_space}.  @var{as} is
+the address space as registered with @code{c_register_addr_space}.
+@var{loc} is the location of the address space qualifier token.
+The default implementation does nothing.
+@end deftypefn
+
 @node Misc
 @section Miscellaneous Parameters
 @cindex parameters, miscellaneous
Index: doc/tm.texi.in
===================================================================
--- doc/tm.texi.in	(revision 238425)
+++ doc/tm.texi.in	(working copy)
@@ -7486,6 +7486,8 @@ c_register_addr_space ("__ea", ADDR_SPAC
 
 @hook TARGET_ADDR_SPACE_DEBUG
 
+@hook TARGET_ADDR_SPACE_DIAGNOSE_USAGE
+
 @node Misc
 @section Miscellaneous Parameters
 @cindex parameters, miscellaneous
Index: target.def
===================================================================
--- target.def	(revision 238425)
+++ target.def	(working copy)
@@ -3241,6 +3241,20 @@ The result is the value to be used with
  int, (addr_space_t as),
  default_addr_space_debug)
 
+/* Function to emit custom diagnostic if an address space is used.  */
+DEFHOOK
+(diagnose_usage,
+ "Define this hook if the availability of an address space depends on\n\
+command line options and some diagnostics should be printed when the\n\
+address space is used.  This hook is called during parsing and allows\n\
+to emit a better diagnostic compared to the case where the address space\n\
+was not registered with @code{c_register_addr_space}.  @var{as} is\n\
+the address space as registered with @code{c_register_addr_space}.\n\
+@var{loc} is the location of the address space qualifier token.\n\
+The default implementation does nothing.",
+ void, (addr_space_t as, location_t loc),
+ default_addr_space_diagnose_usage)
+
 HOOK_VECTOR_END (addr_space)
 
 #undef HOOK_PREFIX
Index: targhooks.c
===================================================================
--- targhooks.c	(revision 238425)
+++ targhooks.c	(working copy)
@@ -1291,6 +1291,15 @@ default_addr_space_debug (addr_space_t a
   return as;
 }
 
+/* The default hook implementation for TARGET_ADDR_SPACE_DIAGNOSE_USAGE.
+   Don't complain about any address space.  */
+
+void
+default_addr_space_diagnose_usage (addr_space_t, location_t)
+{
+}
+	 
+
 /* The default hook for TARGET_ADDR_SPACE_CONVERT. This hook should never be
    called for targets with only a generic address space.  */
 
Index: targhooks.h
===================================================================
--- targhooks.h	(revision 238425)
+++ targhooks.h	(working copy)
@@ -181,6 +181,7 @@ extern rtx default_addr_space_legitimize
 extern bool default_addr_space_subset_p (addr_space_t, addr_space_t);
 extern bool default_addr_space_zero_address_valid (addr_space_t);
 extern int default_addr_space_debug (addr_space_t);
+extern void default_addr_space_diagnose_usage (addr_space_t, location_t);
 extern rtx default_addr_space_convert (rtx, tree, tree);
 extern unsigned int default_case_values_threshold (void);
 extern bool default_have_conditional_execution (void);

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch] Add new hook to diagnose address space usage (take #2)
  2016-07-19  8:16   ` [patch] Add new hook to diagnose address space usage (take #2) Georg-Johann Lay
@ 2016-07-20 11:10     ` Bernd Schmidt
  0 siblings, 0 replies; 7+ messages in thread
From: Bernd Schmidt @ 2016-07-20 11:10 UTC (permalink / raw)
  To: Georg-Johann Lay; +Cc: GCC Patches



On 07/19/2016 10:16 AM, Georg-Johann Lay wrote:

> Done.  Attached is the updated version of the change, log entry is the
> same as before.
>
> Johann

LGTM.


Bernd

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [patch,avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE
  2016-07-18  6:58   ` Denis Chertykov
@ 2016-07-20 14:27     ` Georg-Johann Lay
  0 siblings, 0 replies; 7+ messages in thread
From: Georg-Johann Lay @ 2016-07-20 14:27 UTC (permalink / raw)
  To: Denis Chertykov; +Cc: gcc-patches, Senthil Kumar Selvaraj

[-- Attachment #1: Type: text/plain, Size: 4597 bytes --]

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.


[-- Attachment #2: pm-part1.diff --]
[-- Type: text/x-patch, Size: 6868 bytes --]

Index: config/avr/avr-c.c
===================================================================
--- config/avr/avr-c.c	(revision 238518)
+++ config/avr/avr-c.c	(revision 238519)
@@ -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++)
     {
@@ -391,10 +394,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 238518)
+++ config/avr/avr-protos.h	(revision 238519)
@@ -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 238518)
+++ config/avr/avr.c	(revision 238519)
@@ -9148,6 +9148,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 void
+avr_addr_space_diagnose_usage (addr_space_t as, location_t loc)
+{
+  (void) 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
@@ -9218,16 +9254,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;
         }
@@ -9291,25 +9324,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);
-        }
+      if (TYPE_P (node))
+        error ("pointer targeting address space %qs must be const in %qT",
+               avr_addrspace[as].name, node);
       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);
-        }
+        error ("pointer targeting address space %qs must be const"
+               " in %s %q+D",
+               avr_addrspace[as].name, reason, node);
     }
 
   return reason == NULL;
@@ -9342,18 +9363,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))
         {
@@ -13728,6 +13737,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
 

[-- Attachment #3: pm-part2.diff --]
[-- Type: text/x-patch, Size: 8884 bytes --]

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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-07-20 14:27 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-14 15:12 [patch] Add new hook to diagnose address space usage Georg-Johann Lay
2016-07-15 15:27 ` [patch,avr] make progmem work on AVR_TINY, use TARGET_ADDR_SPACE_DIAGNOSE_USAGE Georg-Johann Lay
2016-07-18  6:58   ` Denis Chertykov
2016-07-20 14:27     ` Georg-Johann Lay
2016-07-18 14:13 ` [patch] Add new hook to diagnose address space usage Bernd Schmidt
2016-07-19  8:16   ` [patch] Add new hook to diagnose address space usage (take #2) Georg-Johann Lay
2016-07-20 11:10     ` Bernd Schmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).