This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]