Consolidate .iram.text matcher in linker to wildcard matcher

[kylefleming]

Any symbol that is specified to be placed in the iram.text section (for example with the ICACHE_RAM_ATTR attribute) should generally be placed in that section regardless of where it came from, especially since this isn't something a non-Arduino/ESP8266 library would specify. As such, it seems like a wildcard matcher could be used here, in order to make sure no symbols fall through the cracks. This is how it's done with irom and irom0, so one could reasonably assume iram to be consistent. Does this seem reasonable?

[ivankravets]

REMOVED

[igrr]

@ivankravets sorry, I don't follow. How is this PR related to library.json? It fixes a bug in the linker script that .iram.text sections from some files could be missed by the wildcard rules. It's not related to platformio (although it happens to reduce the number of platformio-specific rules).

[ivankravets]

@igrr sorry, I thought that this issue is #4355 :(

I'll review it and answer here.

[ivankravets]

Tested with this complex project. Works well!

• https://github.com/euphi/Homie_BareMinimum

Before PR

text	   data	    bss	    dec	    hex	filename
390880	   9104	  32464	 432448	  69940	.pioenvs/nodemcuv2/firmware.elf

After PR

text	   data	    bss	    dec	    hex	filename
392435	   7544	  32472	 432451	  69943	.pioenvs/nodemcuv2/firmware.elf

[ivankravets]

I'm not expert in LD scripting. Could we have the same improvements for https://github.com/kylefleming/Arduino/blob/17a96a93ad8bd904663cf8c23a56ae747e191021/tools/sdk/ld/eagle.app.v6.common.ld#L91:L104 ?

[kylefleming]

@ivankravets It depends on what is being caught by the catch-all matcher in iram (found here). Converting the section you mention, which is in flash, to a catch-all matcher will change what the default section is for .literal* and .text* symbols. A lot of symbols are currently being whitelisted to be put in flash (here), but it's unclear which ones are left out and are expecting to be located in iram.

@igrr do you have a good sense of this? Since all user supplied .literal* and .text* symbols are being defaulted to flash (here), it certainly would make sense to default all .literal* and .text* symbols to flash and add a whitelist in iram for any system libraries that are expecting them in iram.

[igrr]

it certainly would make sense to default all .literal* and .text* symbols to flash and add a whitelist in iram for any system libraries that are expecting them in iram.

The problem is with defining this whitelist. Consider a hypothetical object file which is part of an SDK library. This object file has a few functions without any attribute (which get placed into .text or .text.functionname sections), and a few functions with ICACHE_FLASH_ATTR (which get placed into .irom.text section). How do we a) write a rule to place only some of these functions into flash and others into IRAM, and b) make sure that when SDK libraries are updated (and some functions are moved between sections), the rules are updated as well?

Could we have the same improvements for https://github.com/kylefleming/Arduino/blob/17a96a93ad8bd904663cf8c23a56ae747e191021/tools/sdk/ld/eagle.app.v6.common.ld#L91:L104 ?

I have suggested in the past to generate object files named xxx.c.o and yyy.cpp.o for xxx.c and yyy.cpp source files, when building with platformio. Then the behaviour would match that of arduino-builder, and this fragment of the linker script could be reduced to three lines.

[ivankravets]

@bdbaddog do we have an API in SCons to declare a custom $OBJSUFFIX per C and C++ files? Arduino IDE generates $OBJSUFFIX following the next template $FILENAME.o, they don't strip an extension from a source file.

Some LD scripts depend on these files

*.cpp.o(.iram.text)
*.c.o(.iram.text)

Thanks!

[ivankravets]

@kylefleming Could you try this temporary hook? Please navigate to 37 line https://github.com/esp8266/Arduino/blob/master/tools/platformio-build.py#L37 and add this

env.Replace(OBJSUFFIX =".c.o")

Now, you can try to remove/comment all PlatformIO-related lines in LD script. All should work, including #4355

Please confirm.

P.S: I know that we will append .c.o to CPP or ASM file... Nevertheless, it's a better solution and we will be more close to Arduino IDE behavior.

[ivankravets]

Starting from PIO Core 3.5.2b5 we will generate suffixes similar to Arduino IDE (thanks @bdbaddog for the idea with patching SCons.Builder).

I'll provide new PR tomorrow which will resolve #4355. Also, I'll remove all lines from LD related to PlatformIO.

P.S: @igrr should be happy :)

[ivankravets]

@igrr all things related to PlatformIO were removed from LD script. So, you can merge this PR

[kylefleming]

Updated to integrate #4399 changes.

[d-a-v]

Sorry about #4567, can you report your changes in the new .h file ?

[kylefleming]

Updated

[earlephilhower]

@kylefleming I updated this via the online editor and think we're OK now.