Re: [patch,avr] Add support for devices with flash accessible by LD.

[Georg-Johann Lay <avr at gjlay dot de>]

On 05.07.2017 10:17, Georg-Johann Lay wrote:
> On 04.07.2017 20:11, Richard Sandiford wrote:
> > Georg-Johann Lay <avr@gjlay.de> writes:
> > > Hi,
> > >
> > > This patch adds support for devices that can access flash memory
> > > by LD* instructions, hence there is no need to put .rodata in RAM.
> > >
> > > The default linker script for the new multilib versions already
> > > supports this feature, it's similar to avrtiny, cf.
> > >
> > > https://sourceware.org/PR21472
> > >
> > > This patch does the following:
> > >
> > > * Add multilib variants avrxmega3 and avrxmega3/short-calls.
> > >
> > > * Add new option -mshort-calls for multilib selection between
> > >     devices with <= 8KiB flash and > 8KiB flash.
> > >
> > > * Add specs handling for -mshort-calls:  The compiler knows
> > >     if this option is needed or not appropriate (similar to -msp8).
> > >
> > > * Add new ISA feature AVR_ISA_RCALL for multilib selection
> > >     via -mshort-calls.
> > >
> > > * Add a new row to architecture description that contains the
> > >     start address of flash memory in the RAM address range.
> > >     (The actual value is not needed).
> > >
> > > * For devices with flash in RAM space, don't let .rodata
> > >     objects trigger need for __do_copy_data.
> > >
> > > * Add some devices.
> > >
> > > * Add configure test for Binutils PR21472.
> >
> > Sorry if this has already been discussed, but it's useful to be
> > able to do things like:
> >
> >    .../configure --target=avr-elf --with-cpu=arc700
> >    make -j... all-gcc
> >
> > as a basic sanity test of a pan-target patch.  (I usually do
> > before-and-after assembly comparisons too if no changes are
> > expected.)  The way the configure test is written means that
> > it's no longer possible to do this without first building a
> > trunk version of binutils for avr-elf.
> >
> > Thanks,
> > Richard
>
> Okay, I already thought of a less aggressive approach, I'll
> try to address it soon.

Is the following addendum in order?

The avr maintainers appear to be offline since several weeks already,
maybe a global maintainer can have a look and approve it for trunk?


Johann

gcc/
	PR target/81072
	* configure.ac [target=avr]: WARN instead of ERROR if avrxmega3
	.rodata in flash test fails.
	<HAVE_LD_AVR_AVRXMEGA3_RODATA_IN_FLASH>: Define if test passes.
	* confgure: Regenerate.
	* config.in: Regenerate.
	* config/avr/avr.c (avr_asm_named_section)
	[HAVE_LD_AVR_AVRXMEGA3_RODATA_IN_FLASH]: Only trigger
	__do_copy_data for stuff in .rodata if flash_pm_offset = 0.
	(avr_asm_init_sections): Same.

Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 249982)
+++ config/avr/avr.c	(working copy)
@@ -10001,7 +10001,9 @@ avr_asm_init_sections (void)
      resp. `avr_need_copy_data_p'.  If flash is not mapped to RAM then
      we have also to track .rodata because it is located in RAM then.  */
 
+#if defined HAVE_LD_AVR_AVRXMEGA3_RODATA_IN_FLASH
   if (0 == avr_arch->flash_pm_offset)
+#endif
     readonly_data_section->unnamed.callback = avr_output_data_section_asm_op;
   data_section->unnamed.callback = avr_output_data_section_asm_op;
   bss_section->unnamed.callback = avr_output_bss_section_asm_op;
@@ -10037,7 +10039,10 @@ avr_asm_named_section (const char *name,
                             || STR_PREFIX_P (name, ".gnu.linkonce.d"));
 
   if (!avr_need_copy_data_p
-      && 0 == avr_arch->flash_pm_offset)
+#if defined HAVE_LD_AVR_AVRXMEGA3_RODATA_IN_FLASH
+      && 0 == avr_arch->flash_pm_offset
+#endif
+      )
     avr_need_copy_data_p = (STR_PREFIX_P (name, ".rodata")
                             || STR_PREFIX_P (name, ".gnu.linkonce.r"));
 
Index: config.in
===================================================================
--- config.in	(revision 249982)
+++ config.in	(working copy)
@@ -1460,6 +1460,13 @@ that are supported for each access macro
 #endif
 
 
+/* Define if your default avr linker script for avrxmega3 leaves .rodata in
+   flash. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_LD_AVR_AVRXMEGA3_RODATA_IN_FLASH
+#endif
+
+
 /* Define if your linker supports -z bndplt */
 #ifndef USED_FOR_TARGET
 #undef HAVE_LD_BNDPLT_SUPPORT
Index: configure
===================================================================
--- configure	(revision 249982)
+++ configure	(working copy)
@@ -24851,29 +24851,32 @@ EOF
   ac_status=$?
   $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
   test $ac_status = 0; }; }
-    if test -f conftest.nm
+    if test -s conftest.nm
     then
 	if grep ' R xxvaryy' conftest.nm > /dev/null; then
 	    { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
 $as_echo "yes" >&6; }
-	    rm -f conftest.s conftest.o conftest.elf conftest.nm
+
+$as_echo "#define HAVE_LD_AVR_AVRXMEGA3_RODATA_IN_FLASH 1" >>confdefs.h
+
 	else
 	    { $as_echo "$as_me:${as_lineno-$LINENO}: result: no: avrxmega3 .rodata located in RAM" >&5
 $as_echo "no: avrxmega3 .rodata located in RAM" >&6; }
 	    echo "$as_me: nm output was" >&5
 	    cat conftest.nm >&5
-	    rm -f conftest.s conftest.o conftest.elf conftest.nm
 	    avr_ld_ver="`$gcc_cv_ld -v | sed -e 's:^.* ::'`"
-	    as_fn_error "support for avrxmega3 needs Binutils 2.29 or higher (have $avr_ld_ver)" "$LINENO" 5
+	    { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: support for avrxmega3 .rodata in flash needs Binutils 2.29 or higher (have $avr_ld_ver)" >&5
+$as_echo "$as_me: WARNING: support for avrxmega3 .rodata in flash needs Binutils 2.29 or higher (have $avr_ld_ver)" >&2;}
 	fi
     else
 	{ $as_echo "$as_me:${as_lineno-$LINENO}: result: test failed" >&5
 $as_echo "test failed" >&6; }
 	echo "$as_me: failed program was" >&5
 	cat conftest.s >&5
-	rm -f conftest.s conftest.o conftest.elf
-	as_fn_error "see \`config.log' for details" "$LINENO" 5
+	{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: see \`config.log' for details" >&5
+$as_echo "$as_me: WARNING: see \`config.log' for details" >&2;}
     fi
+    rm -f conftest.s conftest.o conftest.elf conftest.nm
     ;;
 
   cris-*-*)
Index: configure.ac
===================================================================
--- configure.ac	(revision 249982)
+++ configure.ac	(working copy)
@@ -3832,26 +3832,26 @@ EOF
     AC_TRY_COMMAND([$gcc_cv_as -mmcu=avrxmega3 conftest.s -o conftest.o])
     AC_TRY_COMMAND([$gcc_cv_ld -mavrxmega3 conftest.o -o conftest.elf])
     AC_TRY_COMMAND([$gcc_cv_nm conftest.elf > conftest.nm])
-    if test -f conftest.nm
+    if test -s conftest.nm
     then
 	if grep ' R xxvaryy' conftest.nm > /dev/null; then
 	    AC_MSG_RESULT(yes)
-	    rm -f conftest.s conftest.o conftest.elf conftest.nm
+	    AC_DEFINE(HAVE_LD_AVR_AVRXMEGA3_RODATA_IN_FLASH, 1,
+		[Define if your default avr linker script for avrxmega3 leaves .rodata in flash.])
 	else
 	    AC_MSG_RESULT(no: avrxmega3 .rodata located in RAM)
 	    echo "$as_me: nm output was" >&AS_MESSAGE_LOG_FD
 	    cat conftest.nm >&AS_MESSAGE_LOG_FD
-	    rm -f conftest.s conftest.o conftest.elf conftest.nm
 	    avr_ld_ver="`$gcc_cv_ld -v | sed -e 's:^.* ::'`"
-	    AC_MSG_ERROR([[support for avrxmega3 needs Binutils 2.29 or higher (have $avr_ld_ver)]])
+	    AC_MSG_WARN([[support for avrxmega3 .rodata in flash needs Binutils 2.29 or higher (have $avr_ld_ver)]])
 	fi
     else
 	AC_MSG_RESULT(test failed)
 	echo "$as_me: failed program was" >&AS_MESSAGE_LOG_FD
 	cat conftest.s >&AS_MESSAGE_LOG_FD
-	rm -f conftest.s conftest.o conftest.elf
-	AC_MSG_ERROR([[see `config.log' for details]])
+	AC_MSG_WARN([[see `config.log' for details]])
     fi
+    rm -f conftest.s conftest.o conftest.elf conftest.nm
     ;;
 
   cris-*-*)