-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bump riscv-rt to v0.11.0 #35
Conversation
riscv-rt v0.10.* have been yanked, use v0.11.0 instead |
Hmm odd linker errors in the builds, i'll try and look at them later |
I tested on rustc 1.65.0. Running same command in build on rustc 1.67.0 fails 🤔 |
The following examples fail to compile when building w/o
Edit: Spoke too soon w/ prev message! |
Ok I had a look on local. I don't currently have this board to actually test anything with. I also updated Switching to the I'm a bit strapped for time atm. We need to look at by how much the riscv-rt change grew our code. What is a bit odd is that the |
@Disasm any ideas on this one? |
I'd suggest merging this PR and excluding sdcard_test from CI (or maybe replacing release build with a simpler check). |
We could just switch to using |
Sorry I wasn't able to look into this sooner, real life has taken over my free time lately. I did some digging into this, and I'm pretty sure that this isn't a riscv-rt version issue but is instead a breakage when using rustc versions v1.61+. I'm not sure what the best way to fix this issue is, but it would be a bummer if we couldn't get sdcard_test to build using Using rustc 1.64 (pre-LLVM issue which requires bumping riscv-rt) and This lead me to testing other rustc versions using commit Somewhat related, building the sdcard_test example (successfully!) using commit $ size sdcard_test_1.60_memory-cb
text data bss dec hex filename
64928 8 32760 97696 17da0 sdcard_test_1.60_memory-cb
$ size sdcard_test_1.61_memory-cb
text data bss dec hex filename
66780 8 32760 99548 184dc sdcard_test_1.61_memory-cb In case you'd like to check as well, here's what I did for each: # Checkout current master
git checkout 328be12
# Build with rustc 1.60
cargo clean
rustup install 1.60
rustup target add riscv32imac-unknown-none-elf --toolchain 1.60-x86_64-unknown-linux-gnu
cargo +1.60-x86_64-unknown-linux-gnu build --example sdcard_test --all-features --release # Should succeed
# Build with rustc 1.61
cargo clean
rustup install 1.61
rustup target add riscv32imac-unknown-none-elf --toolchain 1.61-x86_64-unknown-linux-gnu
cargo +1.61-x86_64-unknown-linux-gnu build --example sdcard_test --all-features --release # Should fail |
Also, I'm not really sure what changed between v1.60 to v1.61 that would affect this. I can try to look into it, but I'm not sure if that will bear fruit. Here is the v1.61 changelog for reference. |
I think it has to do with this:
However using It does seem to point to an ordering issue with our link scripts though. @Disasm any thoughts? This is a bit above my paygrade wrt. linking knowledge :) |
After looking into this a bit more, I couldn't find anything specific that caused it but it seems rust v1.60 -> 1.61 had a size regression. Using
Will make this work even with the smaller memory profile. @a-gavin could you please add that, at least to the CI? We should be good to go afterwards. |
No idea :(
Not that I am aware of, but I am not sure. |
Fixes issue where some examples will not build using memory-c8.x due to flash size constraints
As per your suggestion, I added a commit w/ a release profile verbatim as well as a development profile with only All examples build locally using rustc 1.67 for both release and development using the following commands : cargo build --examples --all-features
cargo build --examples --all-features --release |
Perfect. As a last point, sorry I forgot to mention this before, but could you also bump riscv to latest as well? It does need one small change in the interrupt handling code (param in the closures) but it should be trivial. It seems to me we'd want to keep the riscv dependency in sync together with riscv-rt |
Also, kind of found what I was looking for regarding rustc regression testing--maybe you'll find useful as well. Regression tests and associated triaging documents (which happen on a weekly basis) can be found in this repo. Performance results can be found here, with specific test results on the compare page. A summary of these results appear in the This Week in Rust newsletter. |
Since Longan Nano GD32VF103 chip only supports a single hart, still safe to use the recently- updated riscv crate interrupt free section which only disables interrupts for the current hart.
Just pushed the riscv crate update to v0.10.1 w/ required interrupt closure changes. As detailed in the commit message, using the recently-changed riscv interrupt free section impl is still safe for the Longan Nano as the GD32VF103 chip it uses only supports a single hart (which is the limitation of the recently-changed interrupt free section impl) |
Fix #34