diff mbox

[avr,RFC] Add var attribute "absdata" to support LDS / STS on AVR_TINY.

Message ID 9680f386-48a4-9080-77db-e8dc93cdaa63@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay Aug. 3, 2016, 4:17 p.m. UTC
This is a proposal to support LDS / STS instructions on AVR_TINY.

Currently the fact that the compile won't generate LDS / STS instructions is a 
major source of code bloat.  The patch adds a new variable attribute so that 
the user can assert that address range of respective static-storage data will 
be in the range of LDS / STS (0x40...0xbf).

There is currently no support in Binutils, and IMO support it in Binutils would 
be kind of overkill...  Such support could implement new sections like .zdata, 
.zrodata, .zbss in the linker script which would be located before .data, 
.rodata, .bss.  The compiler would put absdata objects into one of the 
z-sections, but we would have to supply extended startup-code because bss data 
is no more in one contiguous chunk of memory; same for [ro]data.

More thoughts on how to get better support for LDS / STS?

Johann



gcc/
	* doc/extend.texi (AVR Variable Attributes) [absdata]: Document it.
	* config/avr/avr.c (AVR_SYMBOL_FLAG_TINY_ABSDATA): New macro.
	(avr_address_tiny_absdata_p): New static function.
	(avr_legitimate_address_p, avr_legitimize_address) [AVR_TINY]: Use
	it to determine validity of constant addresses.
	(avr_attribute_table) [absdata]: New variable attribute...
	(avr_handle_absdata_attribute): ...and handler.
	(avr_decl_absdata_p): New static function.
	(avr_encode_section_info) [AVR_TINY]: Use it to add flag
	AVR_SYMBOL_FLAG_TINY_ABSDATA to respective symbols_refs.

Comments

Sandra Loosemore Aug. 8, 2016, 5:10 a.m. UTC | #1
On 08/03/2016 10:17 AM, Georg-Johann Lay wrote:

> Index: doc/extend.texi
> ===================================================================
> --- doc/extend.texi	(revision 238983)
> +++ doc/extend.texi	(working copy)
> @@ -5957,6 +5957,25 @@ memory-mapped peripherals that may lie o
>  volatile int porta __attribute__((address (0x600)));
>  @end smallexample
>
> +@item absdata
> +@cindex @code{absdata} variable attribute, AVR
> +Variables in static storage and with the @code{absdata} attribute can
> +be accessed by the @code{LDS} and @code{STS} instructions which take
> +absolute addresses.
> +
> +@itemize @bullet
> +@item
> +This attribute is only supported for the reduced AVR Tiny core
> +like @code{ATtiny40}.
> +
> +@item
> +There is currently no Binutils support for this attribute and the user has
> +to make sure that respective data is located into an address range that
> +can actually be handled by @code{LDS} and @code{STS}.
> +This applies to addresses in the range @code{0x40}@dots{}@code{0xbf}.

The wording here is a little awkward -- in particular, "the user" is 
really "you", the reader of the manual.  How about something like this 
instead?

@item
You must use an appropriate linker script to locate the data in the 
address range accessible by @code{LDS} and @code{STS} 
(@code{0x40}@dots{}@code{0xbf}).

-Sandra
diff mbox

Patch

Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 238983)
+++ doc/extend.texi	(working copy)
@@ -5957,6 +5957,25 @@  memory-mapped peripherals that may lie o
 volatile int porta __attribute__((address (0x600)));
 @end smallexample
 
+@item absdata
+@cindex @code{absdata} variable attribute, AVR
+Variables in static storage and with the @code{absdata} attribute can
+be accessed by the @code{LDS} and @code{STS} instructions which take
+absolute addresses.
+
+@itemize @bullet
+@item
+This attribute is only supported for the reduced AVR Tiny core
+like @code{ATtiny40}.
+
+@item
+There is currently no Binutils support for this attribute and the user has
+to make sure that respective data is located into an address range that
+can actually be handled by @code{LDS} and @code{STS}.
+This applies to addresses in the range @code{0x40}@dots{}@code{0xbf}.
+
+@end itemize
+
 @end table
 
 @node Blackfin Variable Attributes
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 238983)
+++ config/avr/avr.c	(working copy)
@@ -81,9 +81,13 @@ 
    / SYMBOL_FLAG_MACH_DEP)
 
 /* (AVR_TINY only): Symbol has attribute progmem */
-#define AVR_SYMBOL_FLAG_TINY_PM \
+#define AVR_SYMBOL_FLAG_TINY_PM                 \
   (SYMBOL_FLAG_MACH_DEP << 7)
 
+/* (AVR_TINY only): Symbol has attribute absdata */
+#define AVR_SYMBOL_FLAG_TINY_ABSDATA            \
+  (SYMBOL_FLAG_MACH_DEP << 8)
+
 #define TINY_ADIW(REG1, REG2, I)                                \
     "subi " #REG1 ",lo8(-(" #I "))" CR_TAB                      \
     "sbci " #REG2 ",hi8(-(" #I "))"
@@ -1802,6 +1806,28 @@  avr_mode_dependent_address_p (const_rtx
 }
 
 
+/* Return true if rtx X is a CONST_INT, CONST or SYMBOL_REF
+   address with the `absdata' variable attribute, i.e. respective
+   data can be read / written by LDS / STS instruction.
+   This is used only for AVR_TINY.  */
+
+static bool
+avr_address_tiny_absdata_p (rtx x, machine_mode mode)
+{
+  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_ABSDATA;
+
+  if (CONST_INT_P (x)
+      && IN_RANGE (INTVAL (x), 0, 0xc0 - GET_MODE_SIZE (mode)))
+    return true;
+
+  return false;
+}
+
+
 /* Helper function for `avr_legitimate_address_p'.  */
 
 static inline bool
@@ -1886,8 +1912,7 @@  avr_legitimate_address_p (machine_mode m
       /* avrtiny's load / store instructions only cover addresses 0..0xbf:
          IN / OUT range is 0..0x3f and LDS / STS can access 0x40..0xbf.  */
 
-      ok = (CONST_INT_P (x)
-            && IN_RANGE (INTVAL (x), 0, 0xc0 - GET_MODE_SIZE (mode)));
+      ok = avr_address_tiny_absdata_p (x, mode);
     }
 
   if (avr_log.legitimate_address_p)
@@ -1929,8 +1954,7 @@  avr_legitimize_address (rtx x, rtx oldx,
   if (AVR_TINY)
     {
       if (CONSTANT_ADDRESS_P (x)
-          && !(CONST_INT_P (x)
-               && IN_RANGE (INTVAL (x), 0, 0xc0 - GET_MODE_SIZE (mode))))
+          && ! avr_address_tiny_absdata_p (x, mode))
         {
           x = force_reg (Pmode, x);
         }
@@ -9124,6 +9148,32 @@  avr_handle_fntype_attribute (tree *node,
 }
 
 static tree
+avr_handle_absdata_attribute (tree *node, tree name, tree /* args */,
+                              int /* flags */, bool *no_add)
+{
+  location_t loc = DECL_SOURCE_LOCATION (*node);
+
+  if (AVR_TINY)
+    {
+      if (TREE_CODE (*node) != VAR_DECL
+          || (!TREE_STATIC (*node) && !DECL_EXTERNAL (*node)))
+        {
+          warning_at (loc, OPT_Wattributes, "%qE attribute only applies to"
+                      " variables in static storage", name);
+          *no_add = true;
+        }
+    }
+  else
+    {
+      warning_at (loc, OPT_Wattributes, "%qE attribute only supported"
+                  " for reduced Tiny cores", name);
+      *no_add = true;
+    }
+
+  return NULL_TREE;
+}
+
+static tree
 avr_handle_addr_attribute (tree *node, tree name, tree args,
 			   int flags ATTRIBUTE_UNUSED, bool *no_add)
 {
@@ -9230,6 +9280,8 @@  avr_attribute_table[] =
     false },
   { "address",   1, 1, false, false, false,  avr_handle_addr_attribute,
     false },
+  { "absdata",   0, 0, true, false, false,  avr_handle_absdata_attribute,
+    false },
   { NULL,        0, 0, false, false, false, NULL, false }
 };
 
@@ -9314,6 +9366,17 @@  avr_progmem_p (tree decl, tree attribute
 }
 
 
+/* Return true if DECL has attribute `absdata' set.  This function should
+   only be used for AVR_TINY.  */
+
+static bool
+avr_decl_absdata_p (tree decl, tree attributes)
+{
+  return (TREE_CODE (decl) == VAR_DECL
+          && NULL_TREE != lookup_attribute ("absdata", attributes));
+}
+
+
 /* Scan type TYP for pointer references to address space ASn.
    Return ADDR_SPACE_GENERIC (i.e. 0) if all pointers targeting
    the AS are also declared to be CONST.
@@ -9738,14 +9801,29 @@  avr_encode_section_info (tree decl, rtx
   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;
+
+      if (-1 == avr_progmem_p (decl, DECL_ATTRIBUTES (decl)))
+        {
+          // Tag symbols for later addition of 0x4000 (AVR_TINY_PM_OFFSET).
+          SYMBOL_REF_FLAGS (sym) |= AVR_SYMBOL_FLAG_TINY_PM;
+        }
+
+      if (avr_decl_absdata_p (decl, DECL_ATTRIBUTES (decl)))
+        {
+          // May be accessed by LDS / STS.
+          SYMBOL_REF_FLAGS (sym) |= AVR_SYMBOL_FLAG_TINY_ABSDATA;
+        }
+
+      if (-1 == avr_progmem_p (decl, DECL_ATTRIBUTES (decl))
+          && avr_decl_absdata_p (decl, DECL_ATTRIBUTES (decl)))
+        {
+          error ("%q+D has incompatible attributes %qs and %qs",
+                 decl, "progmem", "absdata");
+        }
     }
 }