diff mbox

[avr] Add support for devices with flash accessible by LD.

Message ID 8f1d1920-029d-3cfe-a136-716745cb7e30@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay July 5, 2017, 10:09 a.m. UTC 
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.

Comments

Richard Sandiford July 5, 2017, 10:30 a.m. UTC | #1 
Georg-Johann Lay <avr@gjlay.de> writes:
> 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?

Thanks for doing this.  LGTM (though obviously I can't approve)

Richard
diff mbox

Patch

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-*-*)