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

[Georg-Johann Lay]

On 11.05.2017 09:05, Pitchumani Sivanupandi wrote:
> On Wednesday 10 May 2017 03:56 PM, Georg-Johann Lay wrote:
>> On 10.05.2017 09:26, Pitchumani Sivanupandi wrote:
>>> On Tuesday 09 May 2017 04:24 PM, Nick Clifton wrote:
>>>> Hi Georg-Johann,
>>>>
>>>>>> On 09.05.2017 07:51, Senthil Kumar Selvaraj wrote:
>>>>>> I'm still not convinced we need a new emulation for this.
>>>> I would really prefer it if you two can come to an agreement about
>>>> the best way to handle this.  I am not adverse to adding a new
>>>> emulation
>>>> if this is what you want, but I would be worried if this leads to an
>>>> explosion in the number of linker scripts later on.
>>>>
>>>>
>>>>> I don't see a way how to provide a linker description file without
>>>>> supplying a new emulation.
>>>> How about defining __RODATA_PM_OFFSET__ on the linker command line
>>>> and using the current avrtiny.sc script ?
>>>>
>>>>
>>>>>>> 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
>>>> I would much prefer it if you did not create a new script, but instead
>>>> added
>>>> parametrisation to the current avr and avrtiny scripts.  (In fact it
>>>> would be
>>>> even better if you could combine avr.sc and avrtiny.sc and just have
>>>> one script).
>>>>
>>>> The reason for this is that the more scripts you have, the greater the
>>>> chances
>>>> of making an error or missing one out when it comes to future changes.
>>>>
>>>> Take a look at the elf.sc script.  It is used by lots of different
>>>> targets, but
>>>> it is highly customizable via definitions in the target's specific
>>>> emulparams
>>>> files.
>>>>
>>>> The alternative approach, which I would also consider to be
>>>> reasonable, is to have
>>>> a base avr script that defines all of the things that are consistent
>>>> between all
>>>> three proposed avr linker scripts and to include this script into
>>>> smaller,
>>>> avr-variant scripts that just defines those things that are specific
>>>> to that variant.
>>>> Kind of like how the DWARF.sc script is included into the elf.sc
>>>> script.
>>> We have tentative patch to put rodata in flash conditionally (e.g.
>>> option flag) that
>>> seems to be working.
>>>
>>> gcc:
>>> Based on flag, put rodata in new section (tentatively named
>>> .rodataFlash). Also generates
>>
>> This is not sound, IMO.  Amongst the problems this might cause are:
>>
>> * Testsuite problems because of off .rodata name.
> -fdata-sections manipulates the section names, Is it ok if the proposed
> section name follows that format (e.g. .rodata.__flash or .rodata.progmem)?

 From my experience we can expect problems if we try to patch .rodata
and that are hard to fix, one reason being the weird varasm of gcc.

So I'd strongly recommend *not* to hack .rodata in the compiler.

>> * Assembler programs might still use .rodata.
> It'd still work though, just that the contents won't go to flash. Not sure
> if it's a deal breaker - even now, do_copy_data doesn't get linked in
> if C code doesn't generate .data/.rodata/.gnu.linkonce* and
> .data/.rodata section
> is used only in assembler programs.
>> * Placement of .gnu.linkonce.r is not handled.
>>
>> * The handling of .progmem appears to be broken.
>>
>> Anyway, this is all GCC stuff.

As said, It's preferred to avoid GCC problems in the first place and
do what you'd do as programmer: Use proper ld script.

I can understand that Mircochip rejects all changes that go in a
different direction than what Microchip has locally in their sources.

But what I am proposing does not render anything existing invalid
or incompatible.  You can still use avrxmega2 in your device file
archives and everything will work out fine.

We could of course add an option to binutils that changes ld script
selection without changing the emulation, but that's unnecessarily
complicated and I don't see the advantage to avoid my any means a
new -mmcu=

> Yes, that patch is tentative. Detecting linear memory and adding options to
> avr specs file are few other things to be done.
>> You you explain why a new ld script is so bad?
> Linker script is ok, it's the new emulation that's a concern.
> As explained earlier by Senthil, it doesn't sound good adding new emulation
> if future devices in other emulations are made with linear memory. Another
> reason is that we could place the .rodata* in flash optionally for
> devices with
> non-linear memory (thinking of supporting this feature if possible).

I still don't get it.  We currently have 17 emulations supported, and
this adds another 1 or 2.  We have more than 250 devices described in
gcc.  What is it what new emulation is bad? Local changes at
Microchip?

Johann

> Regards,
> Pitchumani
>
>