-
Notifications
You must be signed in to change notification settings - Fork 14k
Add new Tier-3 target: riscv64im-unknown-none-elf #148790
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
base: main
Are you sure you want to change the base?
Conversation
|
Some changes occurred in src/doc/rustc/src/platform-support cc @Noratrieb These commits modify compiler targets. |
|
r? compiler |
This comment has been minimized.
This comment has been minimized.
1f29113 to
0e50d8d
Compare
0e50d8d to
40b858c
Compare
src/doc/rustc/src/platform-support/riscv64im-unknown-none-elf.md
Outdated
Show resolved
Hide resolved
src/doc/rustc/src/platform-support/riscv64im-unknown-none-elf.md
Outdated
Show resolved
Hide resolved
|
|
||
| ## Target maintainers | ||
|
|
||
| * Rust Embedded Working Group, [RISC-V team](https://github.com/rust-embedded/wg#the-risc-v-team) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A tier 3 target must have a designated developer or developers (the "target maintainers") on record to be CCed when issues arise regarding the target. (The mechanism to track and CC such developers may evolve over time.)
I assigned Rust Embedded Working Group, since they are already maintaining riscv64IMAC, though I am happy to assign myself.
@almindor @dkhayes117 @romancardenas @MabezDev @jessebraham @rmsyn
Rust Embedded Working Group, are you ok with being the maintainers of this target?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok from my side!
|
@rustbot author Will probably also give this to another compiler team member after you're processed my review, because this is my first time reviewing a new tier-3 target, and wanna make sure I'm not missing anything. |
|
Reminder, once the PR becomes ready for a review, use |
|
@rustbot ready Left a clarifying question re the diff between this and rv32{e,em} |
|
following https://forge.rust-lang.org/compiler/proposals-and-stabilization.html#targets, this needs a compiler lead approval, so |
This comment was marked as resolved.
This comment was marked as resolved.
|
r? compiler_leads 😔 |
Should the docs be updated? https://doc.rust-lang.org/rustc/target-tier-policy.html#adding-a-new-target It currently says to use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. I'll wait a bit longer just to check if the embedded risc-v group are happy to be the maintainers, but otherwise this is fine.
This PR proposes to add riscv64im-unknown-none-elf, a subset of the already supported riscv64imac-unknown-none-elf.
The motivation behind this PR is that we want to standardize (most) zkVMs on riscv64im-none and riscv64ima-none. Having different variants of riscv extensions, also seems to be within expectation, atleast with respects to riscv32.
Note: This does not mean that we will be able to remove riscv32im-risc0-zkvm-elf -- I am not aware of all of the dependents for this
Tier-3 Policy
I assigned Rust Embedded Working Group, since they are already maintaining riscv64IMAC, though I am happy to assign myself.
It follows the naming convention of the other bare metal riscv targets
This has the same requirements as riscv{32, 64}imac
Acknowledging the above.