Patchwork [avr] Add predicate symbols for the linker script to bump sections containing addr-space data

login
register
mail settings
Submitter Georg-Johann Lay
Date Jan. 7, 2013, 7:59 p.m.
Message ID <50EB2907.7050004@gjlay.de>
Download mbox | patch
Permalink /patch/210083/
State New
Headers show

Comments

Georg-Johann Lay - Jan. 7, 2013, 7:59 p.m.
This is a tentative patch that adds symbols that can be used as predicates in
the linker script.

Background is binutils PR14406: Data in address space __flash1 must be located
in such a way that the high byte (hh8) of the address is 0x1.  This is needed
because avr-gcc sets RAMPZ to 0x1 before reading data from __flash1.

The input section for __flash1 is .progmem1.data

Similar for other __flash<N>, n = 2..5

The problems to be solved are:

1) .progmem<N>.data must be located in [0x<N>0000, 0x<N>ffff].

2) If there is too much data in .progmem<N>.data the linker should
   complain.

3) Moving the location counter to 0x<N>0000 is only needed if
   .progmem<N> is non-empty.

4) We must be careful not to move to pointer backwards.

5) User-level build-scripts and Makefiles should be the same if
   the user does not use the new features like __flash<N>.
   Even if __flash<N> is used, it's appreciated if everything
   works out of the box and without changing calls of objcopy,
   for example.

6) If .text extends to, say, 0x20010, it's still fine if .progmem2.data
   starts at 0x20011.  There is no need to throw an error because text
   overlaps 0x20000...0x2ffff.

The very problem is that the linker script language is too limited.  Neither
does it support memory holes, nor can a section float around others, and not
even detecting if a section does not contain any data works as promised by
binutils, cf. [1].

The patch addresses the latter problem: It defines symbols that can be used
like predicates by the linker script like so, cf. [2]:

  .text   :
  {
    ...

    . = MAX (DEFINED(__need_.progmem1.data) ? 1 << 16 : 0, ABSOLUTE(.)) ;
    __progmem1_start = . ;
    *(.progmem1)
    *(.progmem1.*)
    __progmem1_end = . ;

    ...
  } > text


There was some discussion in [3] about how PR14406 could be approached and
there were basically two proposals:

- One from Erik that does not solve 3), 6) or 5) i.e. requires a change in
  the user Makefiles like  -j .hightext  instead of  -j .text  etc.
  .text is after .progmem<N>

- One from me with the predicate-like-symbols kludge that needs a
  GCC change and has less informative error messages than Erik's
  approach in the case anything overflows.
  .text is before .progmem<N>

Maybe everything can be solved much easier and I am lacking the respective
binutils knowledge...

Thoughts?


Johann


[1] http://sourceware.org/ml/binutils/2012-12/msg00151.html
[2] http://lists.gnu.org/archive/html/avr-gcc-list/2012-12/msg00046.html
[3] http://lists.gnu.org/archive/html/avr-gcc-list/2012-12/msg00029.html



gcc/
	* config/avr/avr.c (avr_need_addrspace_p): New static variable.
	(avr_asm_named_section): Record avr_need_addrspace_p.
	(avr_asm_select_section): Ditto.
	(avr_asm_function_rodata_section): Ditto.
	(avr_file_end): Reference __need_.progmem<N>.data according to
	avr_need_addrspace_p[N].

libgcc/
	* config/avr/t-avr (LIB1ASMFUNCS): Add: _progmem1, _progmem2,
	_progmem3, _progmem4, _progmem5.
	* config/avr/lib1funcs.S (__need_.progmem1.data)
	(__need_.progmem2.data, __need_.progmem3.data)
	(__need_.progmem4.data, __need_.progmem5.data): Define symbols.

Patch

Index: gcc/config/avr/avr.c
===================================================================
--- gcc/config/avr/avr.c	(revision 194991)
+++ gcc/config/avr/avr.c	(working copy)
@@ -205,6 +205,10 @@  bool avr_have_dimode = true;
 bool avr_need_clear_bss_p = false;
 bool avr_need_copy_data_p = false;
 
+/* Track need for the non-generic address spaces, similar to
+   avr_need_copy_data_p.  */
+static bool avr_need_addrspace_p[ADDR_SPACE_COUNT];
+
 
 
 /* Custom function to count number of set bits.  */
@@ -8148,6 +8152,9 @@  avr_asm_function_rodata_section (tree de
     flags = frodata->common.flags;
   }
 
+  /* Just for the record... */
+  avr_need_addrspace_p[ADDR_SPACE_FLASH] = true;
+
   if (frodata != readonly_data_section
       && flags & SECTION_NAMED)
     {
@@ -8195,6 +8202,8 @@  avr_asm_named_section (const char *name,
       const char *old_prefix = ".rodata";
       const char *new_prefix = avr_addrspace[as].section_name;
 
+      avr_need_addrspace_p[as] = true;
+      
       if (STR_PREFIX_P (name, old_prefix))
         {
           const char *sname = ACONCAT ((new_prefix,
@@ -8313,6 +8322,8 @@  avr_asm_select_section (tree decl, int r
       if (ADDR_SPACE_GENERIC_P (as))
         as = ADDR_SPACE_FLASH;
       
+      avr_need_addrspace_p[as] = true;
+
       if (sect->common.flags & SECTION_NAMED)
         {
           const char * name = sect->named.name;
@@ -8393,6 +8404,27 @@  avr_file_end (void)
 
   if (avr_need_clear_bss_p)
     fputs (".global __do_clear_bss\n", asm_out_file);
+
+  for (size_t n = 0; n < ADDR_SPACE_COUNT; n++)
+    {
+      addr_space_t as = (addr_space_t) n;
+
+      if (avr_need_addrspace_p[as]
+          /* We only need the __need_ symbols in the linker script for
+             the paged sections that move the location counter forward.
+             For example, bumping the location counter to 0x10000 for
+             __flash1 / .progmem1.data is only needed if that input section
+             is non-empty.  The linker script language is not generic enough
+             to express this.  Thus, we trigger the definition of a symbol
+             that can be used as predicate via DEFINED(symbol).  */
+          && !ADDR_SPACE_GENERIC_P (as)
+          && ADDR_SPACE_FLASH != as
+          && ADDR_SPACE_MEMX != as)
+        {
+          fprintf (asm_out_file, ".global __need_%s\n",
+                   avr_addrspace[as].section_name);
+        }
+    }
 }
 
 /* Choose the order in which to allocate hard registers for
Index: libgcc/config/avr/lib1funcs.S
===================================================================
--- libgcc/config/avr/lib1funcs.S	(revision 194964)
+++ libgcc/config/avr/lib1funcs.S	(working copy)
@@ -2935,4 +2935,37 @@  ENDF __fmul
 #undef C0
 #undef C1
 
+;; Marker Symbols to be used in the Linker Script.
+;; GCC will acquire them if any data is put into .progmem<N>.data
+
+#if defined L_progmem1
+.section .progmem1.data, "a", @progbits
+.global __need_.progmem1.data
+__need_.progmem1.data:
+#endif /* L_progmem1 */
+
+#if defined L_progmem2
+.section .progmem2.data, "a", @progbits
+.global __need_.progmem2.data
+__need_.progmem2.data:
+#endif /* L_progmem2 */
+
+#if defined L_progmem3
+.section .progmem3.data, "a", @progbits
+.global __need_.progmem3.data
+__need_.progmem3.data:
+#endif /* L_progmem3 */
+
+#if defined L_progmem4
+.section .progmem4.data, "a", @progbits
+.global __need_.progmem4.data
+__need_.progmem4.data:
+#endif /* L_progmem4 */
+
+#if defined L_progmem5
+.section .progmem5.data, "a", @progbits
+.global __need_.progmem5.data
+__need_.progmem5.data:
+#endif /* L_progmem5 */
+
 #include "lib1funcs-fixed.S"
Index: libgcc/config/avr/t-avr
===================================================================
--- libgcc/config/avr/t-avr	(revision 194964)
+++ libgcc/config/avr/t-avr	(working copy)
@@ -77,7 +77,9 @@  LIB1ASMFUNCS += \
 	_ssneg_2 _ssneg_4 _ssneg_8 \
 	_ssabs_2 _ssabs_4 _ssabs_8 \
 	_ssadd_8 _sssub_8 \
-	_usadd_8 _ussub_8
+	_usadd_8 _ussub_8 \
+	\
+	_progmem1 _progmem2 _progmem3 _progmem4 _progmem5
 
 LIB2FUNCS_EXCLUDE = \
 	_moddi3 _umoddi3 \