Re: [patch,avr] PR21472: Upgrade emulation avrxmega3 so it has .rodata in flash instead of in SRAM.

[Senthil Kumar Selvaraj <senthilkumar dot selvaraj at microchip dot com>]

Georg-Johann Lay writes:

> ATtiny416, ATtiny417, ATtiny816, ATtiny817 are devices which implement 
> avrxmega2 ISA with the additional feature that flash memory is visible 
> in the SRAM address range (starting at 0x8000).  In order to optimally 
> support this feature, an according linker description file should be 
> used per default, i.e. we want a new emulation which comes with this 
> liker description file.

I'm still not convinced we need a new emulation for this. Sure, we need
a new multilib option in gcc, but can't we do it the way msp8 aka
tiny-stack is handled now? IMO, mapping flash to the data address space
is arch independent, and breaking it out into a separate emulation could
lead to a combinatorial explosion of emulations if devices with that
feature come out for other existing archs. Can't help it for multilibs,
but I'm not sure we need emulations as well.

>
> The patch uses avrxmega3 for this.  Actually, avrxmega3 is an already 
> existing emulation, but one containing no devices so far, and one which 
> avr-gcc doesn't even generate a multilib variant for.
>
> Besides optimal support by a linker description file, the compiler might 
> come up with different code for these devices.  For example, referencing 
> __do_copy_data is no more needed for stuff in .rodata.  This is a second 
> reason for why we wand a separate emulation for these devices.

Again, can't we do this the way we handle msp8 now i.e. set a global var
based on a command line option (mrodata-in-flash?) and then use it to
modify code gen?

Regards
Senthil

>
> As there are currently no devices in this emulation, the patch doesn't 
> break anything existing, is just makes use of code that was dead so far.
>
> The relevant portion of avrxmega3.x is the handling of .rodata which 
> reads similar to the avrtiny.x (PR20849):
>
>
> __RODATA_PM_OFFSET__= DEFINED(__RODATA_PM_OFFSET__) ? 
> __RODATA_PM_OFFSET__: 0x8000;
>
>
>    .text   :
>    {
>       ...
>    }  > text
>    .rodata ADDR(.text) + SIZEOF (.text) + __RODATA_PM_OFFSET__:
>    {
>      *(.rodata)
>      *(.rodata*)
>      *(.gnu.linkonce.r*)
>    } AT> text
>    .data :
>    {
>       ...
>
> Ok for master?
>
> If approved, please someone with commit privileges apply the patch.
>
>
> Johann
>
>
> ld/
> 	Upgrade the currently unused emulation avrxmega3 to one which
> 	supports avrxmega2 devices with flash memory visible in the
> 	SRAM address range.
>
> 	PR21472
> 	* scripttempl/avr_rodata.sc: New file.
> 	* emulparams/avrxmega3.sh (SCRIPT_NAME): Use avr_rodata.
> 	(RODATA_PM_OFFSET): Set to 0x8000.
>
> gas/
> 	PR21472
> 	* config/tc-avr.c (mcu_types): Add entries for: attiny416,
> 	attiny417, attiny816, attiny817.
>
>
> diff --git a/gas/config/tc-avr.c b/gas/config/tc-avr.c
> index 7214c07..79837c8 100644
> --- a/gas/config/tc-avr.c
> +++ b/gas/config/tc-avr.c
> @@ -300,6 +300,10 @@ static struct mcu_type_s mcu_types[] =
>    {"atxmega16e5", AVR_ISA_XMEGA,  bfd_mach_avrxmega2},
>    {"atxmega8e5",  AVR_ISA_XMEGA,  bfd_mach_avrxmega2},
>    {"atxmega32x1", AVR_ISA_XMEGA,  bfd_mach_avrxmega2},
> +  {"attiny416",   AVR_ISA_XMEGA,  bfd_mach_avrxmega3},
> +  {"attiny417",   AVR_ISA_XMEGA,  bfd_mach_avrxmega3},
> +  {"attiny816",   AVR_ISA_XMEGA,  bfd_mach_avrxmega3},
> +  {"attiny817",   AVR_ISA_XMEGA,  bfd_mach_avrxmega3},
>    {"atxmega64a3", AVR_ISA_XMEGA,  bfd_mach_avrxmega4},
>    {"atxmega64a3u",AVR_ISA_XMEGAU, bfd_mach_avrxmega4},
>    {"atxmega64a4u",AVR_ISA_XMEGAU, bfd_mach_avrxmega4},
> diff --git a/ld/emulparams/avrxmega3.sh b/ld/emulparams/avrxmega3.sh
> index abaa5b3..504c492 100644
> --- a/ld/emulparams/avrxmega3.sh
> +++ b/ld/emulparams/avrxmega3.sh
> @@ -1,6 +1,6 @@
>  ARCH=avr:103
>  MACHINE=
> -SCRIPT_NAME=avr
> +SCRIPT_NAME=avr_rodata
>  OUTPUT_FORMAT="elf32-avr"
>  MAXPAGESIZE=1
>  EMBEDDED=yes
> @@ -9,4 +9,5 @@ TEMPLATE_NAME=elf32
>  TEXT_LENGTH=1024K
>  DATA_ORIGIN=0x802000
>  DATA_LENGTH=0xffa0
> +RODATA_PM_OFFSET=0x8000
>  EXTRA_EM_FILE=avrelf
> diff --git a/ld/scripttempl/avr_rodata.sc b/ld/scripttempl/avr_rodata.sc
> new file mode 100644
> index 0000000..eecfedf
> --- /dev/null
> +++ b/ld/scripttempl/avr_rodata.sc
> @@ -0,0 +1,272 @@
> +# Copyright (C) 2014-2017 Free Software Foundation, Inc.
> +#
> +# Copying and distribution of this file, with or without modification,
> +# are permitted in any medium without royalty provided the copyright
> +# notice and this notice are preserved.
> +
> +cat <<EOF
> +/* Copyright (C) 2014-2017 Free Software Foundation, Inc.
> +
> +   Copying and distribution of this script, with or without modification,
> +   are permitted in any medium without royalty provided the copyright
> +   notice and this notice are preserved.  */
> +
> +OUTPUT_FORMAT("${OUTPUT_FORMAT}","${OUTPUT_FORMAT}","${OUTPUT_FORMAT}")
> +OUTPUT_ARCH(${ARCH})
> +
> +__TEXT_REGION_LENGTH__ = DEFINED(__TEXT_REGION_LENGTH__) ? __TEXT_REGION_LENGTH__ : $TEXT_LENGTH;
> +__DATA_REGION_LENGTH__ = DEFINED(__DATA_REGION_LENGTH__) ? __DATA_REGION_LENGTH__ : $DATA_LENGTH;
> +__EEPROM_REGION_LENGTH__ = DEFINED(__EEPROM_REGION_LENGTH__) ? __EEPROM_REGION_LENGTH__ : 64K;
> +__FUSE_REGION_LENGTH__ = DEFINED(__FUSE_REGION_LENGTH__) ? __FUSE_REGION_LENGTH__ : 1K;
> +__LOCK_REGION_LENGTH__ = DEFINED(__LOCK_REGION_LENGTH__) ? __LOCK_REGION_LENGTH__ : 1K;
> +__SIGNATURE_REGION_LENGTH__ = DEFINED(__SIGNATURE_REGION_LENGTH__) ? __SIGNATURE_REGION_LENGTH__ : 1K;
> +__USER_SIGNATURE_REGION_LENGTH__ = DEFINED(__USER_SIGNATURE_REGION_LENGTH__) ? __USER_SIGNATURE_REGION_LENGTH__ : 1K;
> +__RODATA_PM_OFFSET__ = DEFINED(__RODATA_PM_OFFSET__) ? __RODATA_PM_OFFSET__ : $RODATA_PM_OFFSET;
> +
> +MEMORY
> +{
> +  text   (rx)   : ORIGIN = 0, LENGTH = __TEXT_REGION_LENGTH__
> +  data   (rw!x) : ORIGIN = $DATA_ORIGIN, LENGTH = __DATA_REGION_LENGTH__
> +  eeprom (rw!x) : ORIGIN = 0x810000, LENGTH = __EEPROM_REGION_LENGTH__
> +  fuse      (rw!x) : ORIGIN = 0x820000, LENGTH = __FUSE_REGION_LENGTH__
> +  lock      (rw!x) : ORIGIN = 0x830000, LENGTH = __LOCK_REGION_LENGTH__
> +  signature (rw!x) : ORIGIN = 0x840000, LENGTH = __SIGNATURE_REGION_LENGTH__
> +  user_signatures (rw!x) : ORIGIN = 0x850000, LENGTH = __USER_SIGNATURE_REGION_LENGTH__
> +}
> +
> +SECTIONS
> +{
> +  /* Read-only sections, merged into text segment: */
> +  ${TEXT_DYNAMIC+${DYNAMIC}}
> +  .hash        ${RELOCATING-0} : { *(.hash)		}
> +  .dynsym      ${RELOCATING-0} : { *(.dynsym)		}
> +  .dynstr      ${RELOCATING-0} : { *(.dynstr)		}
> +  .gnu.version ${RELOCATING-0} : { *(.gnu.version)	}
> +  .gnu.version_d ${RELOCATING-0} : { *(.gnu.version_d)	}
> +  .gnu.version_r ${RELOCATING-0} : { *(.gnu.version_r)	}
> +
> +  .rel.init    ${RELOCATING-0} : { *(.rel.init)		}
> +  .rela.init   ${RELOCATING-0} : { *(.rela.init)	}
> +  .rel.text    ${RELOCATING-0} :
> +    {
> +      *(.rel.text)
> +      ${RELOCATING+*(.rel.text.*)}
> +      ${RELOCATING+*(.rel.gnu.linkonce.t*)}
> +    }
> +  .rela.text   ${RELOCATING-0} :
> +    {
> +      *(.rela.text)
> +      ${RELOCATING+*(.rela.text.*)}
> +      ${RELOCATING+*(.rela.gnu.linkonce.t*)}
> +    }
> +  .rel.fini    ${RELOCATING-0} : { *(.rel.fini)		}
> +  .rela.fini   ${RELOCATING-0} : { *(.rela.fini)	}
> +  .rel.rodata  ${RELOCATING-0} :
> +    {
> +      *(.rel.rodata)
> +      ${RELOCATING+*(.rel.rodata.*)}
> +      ${RELOCATING+*(.rel.gnu.linkonce.r*)}
> +    }
> +  .rela.rodata ${RELOCATING-0} :
> +    {
> +      *(.rela.rodata)
> +      ${RELOCATING+*(.rela.rodata.*)}
> +      ${RELOCATING+*(.rela.gnu.linkonce.r*)}
> +    }
> +  .rel.data    ${RELOCATING-0} :
> +    {
> +      *(.rel.data)
> +      ${RELOCATING+*(.rel.data.*)}
> +      ${RELOCATING+*(.rel.gnu.linkonce.d*)}
> +    }
> +  .rela.data   ${RELOCATING-0} :
> +    {
> +      *(.rela.data)
> +      ${RELOCATING+*(.rela.data.*)}
> +      ${RELOCATING+*(.rela.gnu.linkonce.d*)}
> +    }
> +  .rel.ctors   ${RELOCATING-0} : { *(.rel.ctors)	}
> +  .rela.ctors  ${RELOCATING-0} : { *(.rela.ctors)	}
> +  .rel.dtors   ${RELOCATING-0} : { *(.rel.dtors)	}
> +  .rela.dtors  ${RELOCATING-0} : { *(.rela.dtors)	}
> +  .rel.got     ${RELOCATING-0} : { *(.rel.got)		}
> +  .rela.got    ${RELOCATING-0} : { *(.rela.got)		}
> +  .rel.bss     ${RELOCATING-0} : { *(.rel.bss)		}
> +  .rela.bss    ${RELOCATING-0} : { *(.rela.bss)		}
> +  .rel.plt     ${RELOCATING-0} : { *(.rel.plt)		}
> +  .rela.plt    ${RELOCATING-0} : { *(.rela.plt)		}
> +
> +  /* Internal text space or external memory.  */
> +  .text ${RELOCATING-0} :
> +  {
> +    *(.vectors)
> +    KEEP(*(.vectors))
> +
> +    /* For data that needs to reside in the lower 64k of progmem.  */
> +    ${RELOCATING+ *(.progmem.gcc*)}
> +
> +    /* PR 13812: Placing the trampolines here gives a better chance
> +       that they will be in range of the code that uses them.  */
> +    ${RELOCATING+. = ALIGN(2);}
> +    ${CONSTRUCTING+ __trampolines_start = . ; }
> +    /* The jump trampolines for the 16-bit limited relocs will reside here.  */
> +    *(.trampolines)
> +    ${RELOCATING+ *(.trampolines*)}
> +    ${CONSTRUCTING+ __trampolines_end = . ; }
> +
> +    /* avr-libc expects these data to reside in lower 64K. */
> +    ${RELOCATING+ *libprintf_flt.a:*(.progmem.data)}
> +    ${RELOCATING+ *libc.a:*(.progmem.data)}
> +
> +    ${RELOCATING+ *(.progmem*)}
> +
> +    ${RELOCATING+. = ALIGN(2);}
> +
> +    /* For future tablejump instruction arrays for 3 byte pc devices.
> +       We don't relax jump/call instructions within these sections.  */
> +    *(.jumptables)
> +    ${RELOCATING+ *(.jumptables*)}
> +
> +    /* For code that needs to reside in the lower 128k progmem.  */
> +    *(.lowtext)
> +    ${RELOCATING+ *(.lowtext*)}
> +
> +    ${CONSTRUCTING+ __ctors_start = . ; }
> +    ${CONSTRUCTING+ *(.ctors) }
> +    ${CONSTRUCTING+ __ctors_end = . ; }
> +    ${CONSTRUCTING+ __dtors_start = . ; }
> +    ${CONSTRUCTING+ *(.dtors) }
> +    ${CONSTRUCTING+ __dtors_end = . ; }
> +    KEEP(SORT(*)(.ctors))
> +    KEEP(SORT(*)(.dtors))
> +
> +    /* From this point on, we don't bother about wether the insns are
> +       below or above the 16 bits boundary.  */
> +    *(.init0)  /* Start here after reset.  */
> +    KEEP (*(.init0))
> +    *(.init1)
> +    KEEP (*(.init1))
> +    *(.init2)  /* Clear __zero_reg__, set up stack pointer.  */
> +    KEEP (*(.init2))
> +    *(.init3)
> +    KEEP (*(.init3))
> +    *(.init4)  /* Initialize data and BSS.  */
> +    KEEP (*(.init4))
> +    *(.init5)
> +    KEEP (*(.init5))
> +    *(.init6)  /* C++ constructors.  */
> +    KEEP (*(.init6))
> +    *(.init7)
> +    KEEP (*(.init7))
> +    *(.init8)
> +    KEEP (*(.init8))
> +    *(.init9)  /* Call main().  */
> +    KEEP (*(.init9))
> +    *(.text)
> +    ${RELOCATING+. = ALIGN(2);}
> +    ${RELOCATING+ *(.text.*)}
> +    ${RELOCATING+. = ALIGN(2);}
> +    *(.fini9)  /* _exit() starts here.  */
> +    KEEP (*(.fini9))
> +    *(.fini8)
> +    KEEP (*(.fini8))
> +    *(.fini7)
> +    KEEP (*(.fini7))
> +    *(.fini6)  /* C++ destructors.  */
> +    KEEP (*(.fini6))
> +    *(.fini5)
> +    KEEP (*(.fini5))
> +    *(.fini4)
> +    KEEP (*(.fini4))
> +    *(.fini3)
> +    KEEP (*(.fini3))
> +    *(.fini2)
> +    KEEP (*(.fini2))
> +    *(.fini1)
> +    KEEP (*(.fini1))
> +    *(.fini0)  /* Infinite loop after program termination.  */
> +    KEEP (*(.fini0))
> +    ${RELOCATING+ _etext = . ; }
> +  } ${RELOCATING+ > text}
> +
> +  .rodata ${RELOCATING+ ADDR(.text) + SIZEOF (.text) + __RODATA_PM_OFFSET__ } ${RELOCATING-0} :
> +  {
> +    *(.rodata)
> +    ${RELOCATING+ *(.rodata*)}
> +    *(.gnu.linkonce.r*)
> +  } ${RELOCATING+AT> text}
> +
> +  .data        ${RELOCATING-0} :
> +  {
> +    ${RELOCATING+ PROVIDE (__data_start = .) ; }
> +    *(.data)
> +    ${RELOCATING+ *(.data*)}
> +    *(.gnu.linkonce.d*)
> +    ${RELOCATING+. = ALIGN(2);}
> +    ${RELOCATING+ _edata = . ; }
> +    ${RELOCATING+ PROVIDE (__data_end = .) ; }
> +  } ${RELOCATING+ > data ${RELOCATING+AT> text}}
> +
> +  .bss ${RELOCATING+ ADDR(.data) + SIZEOF (.data)} ${RELOCATING-0} :${RELOCATING+ AT (ADDR (.bss))}
> +  {
> +    ${RELOCATING+ PROVIDE (__bss_start = .) ; }
> +    *(.bss)
> +    ${RELOCATING+ *(.bss*)}
> +    *(COMMON)
> +    ${RELOCATING+ PROVIDE (__bss_end = .) ; }
> +  } ${RELOCATING+ > data}
> +
> +  ${RELOCATING+ __data_load_start = LOADADDR(.data); }
> +  ${RELOCATING+ __data_load_end = __data_load_start + SIZEOF(.data); }
> +
> +  /* Global data not cleared after reset.  */
> +  .noinit ${RELOCATING+ ADDR(.bss) + SIZEOF (.bss)} ${RELOCATING-0}: ${RELOCATING+ AT (ADDR (.noinit))}
> +  {
> +    ${RELOCATING+ PROVIDE (__noinit_start = .) ; }
> +    *(.noinit*)
> +    ${RELOCATING+ PROVIDE (__noinit_end = .) ; }
> +    ${RELOCATING+ _end = . ;  }
> +    ${RELOCATING+ PROVIDE (__heap_start = .) ; }
> +  } ${RELOCATING+ > data}
> +
> +  .eeprom ${RELOCATING-0}:
> +  {
> +    /* See .data above...  */
> +    KEEP(*(.eeprom*))
> +    ${RELOCATING+ __eeprom_end = . ; }
> +  } ${RELOCATING+ > eeprom}
> +
> +  .fuse ${RELOCATING-0}:
> +  {
> +    KEEP(*(.fuse))
> +    KEEP(*(.lfuse))
> +    KEEP(*(.hfuse))
> +    KEEP(*(.efuse))
> +  } ${RELOCATING+ > fuse}
> +
> +  .lock ${RELOCATING-0}:
> +  {
> +    KEEP(*(.lock*))
> +  } ${RELOCATING+ > lock}
> +
> +  .signature ${RELOCATING-0}:
> +  {
> +    KEEP(*(.signature*))
> +  } ${RELOCATING+ > signature}
> +
> +  /* Stabs debugging sections.  */
> +  .stab 0 : { *(.stab) }
> +  .stabstr 0 : { *(.stabstr) }
> +  .stab.excl 0 : { *(.stab.excl) }
> +  .stab.exclstr 0 : { *(.stab.exclstr) }
> +  .stab.index 0 : { *(.stab.index) }
> +  .stab.indexstr 0 : { *(.stab.indexstr) }
> +  .comment 0 : { *(.comment) } 
> +  .note.gnu.build-id : { *(.note.gnu.build-id) }
> +EOF
> +
> +. $srcdir/scripttempl/DWARF.sc
> +
> +cat <<EOF
> +}
> +EOF