Move --relax from Compiler.java to some boards.txt param

[ffissore]

This piece of code should be removed and become part of a generic board specific flags configuration in boards.txt
https://github.com/arduino/Arduino/blob/ide-1.5.x/app/src/processing/app/debug/Compiler.java#L733-L739

[matthijskooijman]

Woah, indeed, I was already wondering where that option came from.

I don't think we already have a an "append compiler flags" feature from boards.txt? I wonder if we should take this opportunity to rethink where compiler flags come from, since there is also the wish to override or extend compiler flags from the commandline (#421) as well as using -D options to configure libraries, which would need per-sketch compiler flags (though #1808 solves library configuration differently, by using include files, having per-sketch compiler flags would still be useful in some cases).

I did spend some time thinking about this though, and haven't come up with a satisfactory solution yet. Especially the "overriding" part is hard to get right.

[matthijskooijman]

After @xxxajk pointed out that his Makefile produced smaller binaries than the IDE, I stumbled on this option. Apparently, passing --relax to the linker on non-mega targets turns out to be beneficial as well. The option tells to compiler to use RJMP / RCALL when possible, and JMP / CALL (which are a bit bigger) in other situations (e.g. when jumping to another 128kiB segment). You'd expect that on MCU's with < 128kiB, a RJMP / RCALL would be used unconditionally, regardless of the --relax option, but it turns out that it uses JMP / CALL instead. Perhaps this changed in recent compiler versions?

In any case - we could just enable --relax always it seems. This would get rid of the hack in Compiler.java, as well as compiler smaller programs (an empty sketch for the Uno drops from 450 bytes to 436 bytes, bigger sketch will have more calls and jumps, so savings will be bigger there).

[xxxajk]

Two more options help too.
-freorder-blocks and -fno-inline-small-functions
The first tells the compiler to reorder basic blocks in the compiled function in order to reduce number of taken branches and improve code locality. This can improve both speed and size if successful.

The second can save a ton if you use small functions a lot within a library and sketch. The only offset is that code will be delayed by a couple cycles, however with a large sketch on a smaller uC, it may be worth doing.

[matthijskooijman]

Hm, not sure I like to add -fno-inline-small-functions, letting the compiler decide when to inline and when not to inline is often a good idea. Even more, the description of the option says:

-finline-small-functions
Integrate functions into their callers when their body is smaller than expected function call code (so overall size of program gets smaller). The compiler heuristically decides which functions are simple enough to be worth integrating in this way. This inlining applies to all functions, even those not declared inline.

Which would actually indicate that enabling inline-small-functions can only shrink code, not enlarge it (but perhaps the heuristic are wrong too often?). In any case, I don't think disabling inline entirely (which I think is what this function effectively does) might be too much.

Regarding reorder-blocks, that option is enabled at -O2, but -Os explicitly disables it again. So I'd be surprised if it actually shrinks code size.

I'll test both options against the full list of Arduino examples some time to see if they help.

[xxxajk]

They do help, that's why I use them :-D

On Mon, Nov 3, 2014 at 7:55 AM, Matthijs Kooijman notifications@github.com
wrote:

Hm, not sure I like to add -fno-inline-small-functions, letting the
compiler decide when to inline and when not to inline is often a good idea.
Even more, the description of the option says:

-finline-small-functions
Integrate functions into their callers when their body is smaller than
expected function call code (so overall size of program gets smaller). The
compiler heuristically decides which functions are simple enough to be
worth integrating in this way. This inlining applies to all functions, even
those not declared inline.

Which would actually indicate that enabling inline-small-functions can
only shrink code, not enlarge it (but perhaps the heuristic are wrong too
often?). In any case, I don't think disabling inline entirely (which I
think is what this function effectively does) might be too much.

Regarding reorder-blocks, that option is enabled at -O2, but -Os
explicitly disables it again. So I'd be surprised if it actually
shrinks code size.

I'll test both options against the full list of Arduino examples some time
to see if they help.

—
Reply to this email directly or view it on GitHub
#2039 (comment).

Visit my github for awesome Arduino code @ https://github.com/xxxajk

[jantje]

This issue just got reported on the arduino eclipse plugin as well Sloeber/arduino-eclipse-plugin#270
I was not aware of this.
The code frederico points to https://github.com/arduino/Arduino/blob/ide-1.5.x/app/src/processing/app/debug/Compiler.java#L733-L739 gives a page not found.
So what other things are in there?

I'm wondering:
In my eclipse plugin the boards.txt settings can overwrite the platform.txt settings by simply declaring the same name again.
If the Arduino IDE does it this way too, one could easily solve this as follows:
platform.txt
compiler.c.default.elf.flags=-w -Os -Wl,--gc-sections
compiler.c.elf.flags={compiler.c.default.elf.flags}

boards.txt
mega.compiler.c.elf.flags={compiler.c.default.elf.flags},--relax
..(something similar for other mega based boards)

[ffissore]

While we already have a handful of extra_flags options both in boards.txt and platform.local.txt, none of them is useful for --relax. AFAICS fixing this, whatever way we choose, will break backwards compatibility with older and especially custom cores. @cmaglie ?

[cmaglie]

As said by @xxxajk and @matthijskooijman we can just add it permanently to the command line for all AVR boards. Does anyone know a reason to not do so?

Previous versions of the IDE will continue to add the parameter to the command line and eventually ends to -Wl,--gc-sections,--relax,--relax that it seems to be tolerated by gcc, so this solution should be fully-compatible.

Just for the record, the relevant piece of code is now here

Arduino/arduino-core/src/processing/app/debug/Compiler.java

Line 1118 in 740a14e

String optRelax = "";

The current linker command is: compiler.c.elf.flags=-w -Os -Wl,--gc-sections
with the relax options becomes: compiler.c.elf.flags=-w -Os -Wl,--gc-sections,--relax
so ,--relax should be added specifically to the parameter -Wl,... (just appending to the whole command line as it now may not work if the command line is reordered).

[per1234]

Looks like the code is now here:

• https://github.com/arduino/arduino-builder/blob/5fd49b6d4c14abc31ae9e1a9f18d4c1e7ba8b355/phases/linker.go#L74
• https://github.com/arduino/arduino-builder/blob/5fd49b6d4c14abc31ae9e1a9f18d4c1e7ba8b355/phases/linker.go#L94-L99

[matthijskooijman]

This issue is essentially duplicated by arduino/arduino-cli#639 (which I reported last month, totally forgetting that this issue already existed). Regardless, that is now the location where the relevant code lives (nothing left to fix in this repo), so I'm going to close this issue in favor of that one.