Update Rust toolchain to nightly-2025-03-03 #634
Conversation
tones111
force-pushed
the
toolchain
branch
5 times, most recently
from
1a0f125 to
d4538ec
Compare
|
I'd like to phone a friend (@Patryk27) When I build locally I get an error like I'm not sure where symbols.o comes from. Maybe we need to (somehow) pass extra flags when building the It appears the my versions |
Yes, this is supposed to be done automatically by rustc: I'll take a look later at what's going on here! |
tones111
changed the title
|
Pinging @Rahix for another look now that the CI is passing. |
|
At last! Thanks everyone, especially @Patryk27, for the effort! I just smoke tested the uno examples to make sure we're not masking any obvious regressions - see below for documentation.
If someone is willing and able to run some of the ATtiny examples to gather a few more data points, that would not hurt. Footnotes
|
| run: cd "examples/${{ matrix.m.name }}" && cargo build --release --bins | ||
| - name: Compile board crate (without examples) | ||
| if: "${{ matrix.m.type == 'board' && !matrix.m.examples }}" | ||
| run: cd "arduino-hal/" && cargo build --features "${{ matrix.m.name }}" | ||
| run: cd "arduino-hal/" && cargo build --release --features "${{ matrix.m.name }}" |
There was a problem hiding this comment.
I assume you added --release here because some of the examples no longer build without release-mode, right?
From what I noticed, it is mainly the usual problem of panic bloat that gets cut down a bit more in release builds where less panicking code-paths remain.
There was a problem hiding this comment.
Correct, some of the example builds were running out of space so building for release feels like a better default. This is consistent with the AVR platform notes.
Note that since AVRs have rather small amounts of registers, ROM and RAM, it's
recommended to always use--releaseto avoid running out of space.
I'm not familiar with any workflows that would make use of debug info so documenting --release as the default way to build seems ok. If there's a way to remotely gdb that would be great to document.
There was a problem hiding this comment.
The meaning of --release is entirely dependent on the profile-configuration. We're advertising a recommended set of profiles here in avr-hal and in the template repository. Currently, they look like this:
Lines 1 to 11 in 1d158e0
(and imagine those being merged with the default profiles from cargo).
I do want to keep the situation where both dev and release profiles are usable for most applications. So I guess we'll have to dig in a bit why this is happening now. As mentioned, the smoking gun is panic bloat. The main remaining difference between dev and release is this:
[profile.dev]
debug-assertions = true
overflow-checks = true
[profile.release]
debug-assertions = false
overflow-checks = falseBoth settings lead to a number of additional panicking code paths being introduced. If they cannot be optimized out, they quickly fill up RAM with useless formatting info.
I really don't think we should disable either of them in the dev profile because that would really degrade the Rust experience in my eyes. But maybe we can tweak dependency profiles or the like to work around the regression.
There was a problem hiding this comment.
uno-panic is one of the problematic binaries (see #639). I agree changing those dev profile flags would be surprising to users as it essentially turns dev into a release build. At that point I think telling users to explicitly build with --release is the better option.
Baseline sizes for a couple binaries are
text data bss dec hex filename
892 0 1 893 37d ../../target/avr-none/debug/uno-timer.elf
1016 0 1 1017 3f9 ../../target/avr-none/release/uno-timer.elf
1504 0 1 1505 5e1 ../../target/avr-none/release/uno-panic.elf
Turning off debug-assertions
text data bss dec hex filename
908 0 1 909 38d ../../target/avr-none/debug/uno-timer.elf
4336 0 1 4337 10f1 ../../target/avr-none/debug/uno-panic.elf
Turning off debug-assertions and overflow-checks
text data bss dec hex filename
1016 0 1 1017 3f9 ../../target/avr-none/debug/uno-timer.elf
3062 0 1 3063 bf7 ../../target/avr-none/debug/uno-panic.elf
Only if we also set codegen-units=1 does the size of uno-panic come down.
For these devices which have a reputation for being extremely space limited I fail to understand the value in trying to support the debug profile. However, tracking code size regressions for release builds seems like a worthwhile endeavor. Panic bloat will become increasingly problematic as application complexity scales beyond the small examples here.
EDIT:
For comparison the sizes when building with the 2024-03-22 toolchain (main branch) are:
text data bss dec hex filename
936 0 1 937 3a9 ../../target/avr-atmega328p/debug/uno-timer.elf
838 0 1 839 347 ../../target/avr-atmega328p/release/uno-timer.elf
5512 0 1 5513 1589 ../../target/avr-atmega328p/debug/uno-panic.elf
3164 0 1 3165 c5d ../../target/avr-atmega328p/release/uno-panic.elf
There was a problem hiding this comment.
After more investigation I've narrowed the debug bloat down to printing the panic information. Debug binary size is small when commented out, but as soon as we try to format anything from the PanicInfo (even just the line number) we get the linker error.
| @@ -47,7 +47,7 @@ Go into `./examples/arduino-uno` (or the directory for whatever board you want), | |||
| cd examples/arduino-uno | |||
|
|
|||
| # Build and run it on a connected board | |||
| cargo run --bin uno-blink | |||
| cargo run --release --bin uno-blink | |||
There was a problem hiding this comment.
We really shouldn't require release builds. If panic bloat is more of a problem now, we'll have to be more explicit about this topic or, worst-case, adjust the dev-profile even further.
Rahix
changed the title
This change bumps the toolchain to a recent nightly that incorporates LLVM codegen improvements and avr-none target.
Much thanks to @Patryk27 for all your hard work
closes #537