Skip to content

Added support for more granular RISCV32/64 ISA extensions #108

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cordawyn
Copy link

Rust target for RISCV64 was hard-coded to "riscv64imac-unknown-none-elf" and it failed to link with the rest of Zephyr when some specific ISA extensions were enabled (e.g. RISCV_ISA_EXT_G), resulting in errors "can't link soft-float modules with double-float modules". This PR dynamically updates RUST_TARGET depending on the enabled extensions. While I was at it, I also did the same for RISCV32 targets.

I did not cover all possible variants, only those currently available in "rustup target list" (which means, only "riscv64gc-unknown-none-elf" and "riscv64imac-unknown-none-elf", as well as a bunch of targets for "riscv32").

Copy link
Collaborator

@d3zd3z d3zd3z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does seem to build up the right target, but do we end up in situations where upstream rust hasn't built core/alloc for a given variant? This will get a lot easier when building core/alloc/std just with cargo becomes stable.

@d3zd3z
Copy link
Collaborator

d3zd3z commented Jul 1, 2025

CI is failing with the riscv32imac-unknown-none-elf target not installed. For local changes, we can easily add this to the .github/workflows/build.yaml, although that will have to merge separately before this change will pass CI.

We will also have to get the target installed into the Docker image for Zephyr CI, which will take a bit longer to work through. The change is simple, but they don't move Docker images very often.

Question, though, do you know which targets this change actually needs based on all of the supported platforms?

@cordawyn
Copy link
Author

cordawyn commented Jul 1, 2025

This does seem to build up the right target, but do we end up in situations where upstream rust hasn't built core/alloc for a given variant?

I agree, this may happen. And looking at the CI errors, it may not be as quick of a fix as I may have imagined first.

Basically, Zephyr's Kconfig allows some wild combinations of targets already, and the main problem seems to be in somehow synchronizing those with the available Rust targets. So my idea was to have an "umbrella" coverage for them in CMakeLists.txt here, and put it on the developer to install the Rust targets they need.

Question, though, do you know which targets this change actually needs based on all of the supported platforms?

I don't know tbh. My specific use case (which lead to this MR) was my experiments with "tf-micro" (tensorflow) in Rust, and its requirement for a floating point support (i.e. "riscv64gc") - which qemu-riscv64 does not have (it's hard-coded to "riscv64imac" as you are certainly aware). And so I made this change and it was a big local success 😆. Afaik, it's currently the only way to add floating point support for Rust apps in Zephyr (for qemu targets).

@cordawyn
Copy link
Author

cordawyn commented Jul 1, 2025

So to summarize - I'm pretty new to this project and Zephyr ecosystem, and I'm not sure how we should proceed, given the issues that you've mentioned. For the time being I would be happy if I could somehow convince "west" replace the "official" zephyr-lang-rust with my branch - just for my experiments, at least (until some solution for this PR presents itself?). I've asked about this override in Discord already (in "west" channel), but got no feedback.

@d3zd3z
Copy link
Collaborator

d3zd3z commented Jul 1, 2025

So to summarize - I'm pretty new to this project and Zephyr ecosystem, and I'm not sure how we should proceed, given the issues that you've mentioned. For the time being I would be happy if I could somehow convince "west" replace the "official" zephyr-lang-rust with my branch - just for my experiments, at least (until some solution for this PR presents itself?). I've asked about this override in Discord already (in "west" channel), but got no feedback.

I don't think there is an official way to have your own branch, or to change the path. There are a few ways to do it. You can always just checkout your own branch in ../modules/lang/rust, but be aware that doing west update will switch that back to the one in your modules file.

The other way is to edit the zephyr/submanifests/optional.yaml to point to yours. You could just change the path, or add a whole new repo. But, it isn't very set up to do local development on the modules.

I just do my work directly in ../modules/lang/rust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants