-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Stabilize 28 RISC-V target features (riscv_ratified_v2
)
#145948
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
Conversation
Some changes occurred in compiler/rustc_codegen_ssa
I expect that this PR lands on the version 1.92 cycle (I chose extensions with relatively low risk factors) but happy if it could be merged in the version 1.91 cycle.
LGTM, these are all officially ratified RISC-V extensions.
@rfcbot merge
Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:
- @Amanieu
- @BurntSushi
- @dtolnay
- @joshtriplett
- @m-ou-se
- @nikomatsakis
- @scottmcm
- @the8472
- @tmandry
- @traviscross
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.
Consideration: Feature Naming (for std_detect
)
The only initial intent of riscv_ratified_v2
(which feels a bit blunt) was to avoid crashing with existing riscv_ratified
1 and later noticed that x86 uses various feature names on stabilization.
I considered the similar after the initial submission but didn't for several reasons:
- Many extensions to be stabilized are general purpose (even some of them are split from an early version of the
I
base ISA) and clear distinction will confuse the users between plainriscv_ratified
(it's just a collection of extensions where runtime detection of them is stabilized in Rust 1.78.0 and contains rich extensions like scalar cryptography). - Considering future extensions to be stabilized in Rust and the history (some RISC-V extensions are split from others and semantic naming may not work in the near future), either of them seems inevitable:
- Using the extension name itself or similar or
- Using blunt version schemes like in this PR.
After consideration, I prefer blunt naming scheme (pure versioning regardless of the category) for runtime detection of RISC-V extensions.
Consideration: Deferring to Version 1.92 Cycle may Have its Benefit
Although I'll rush for the version 1.91 cycle if possible, I have to say that deferring this might be an option.
I'm talking about #145071, which raises the minimum LLVM version to 20. After this PR is merged, I think we can safely introduce the Zacas
extension without riscv_ratified_v3
(I excluded Zacas
from this PR because of this minimum LLVM version issue).
So, if we don't make it to the version 1.91 cycle, waiting for #145071 might be an option.
Footnotes
-
The same feature name does not allow multiple stabilized versions. ↩
The `Zb` extension does not exist and we instead have the `B` extension which is a superset of the three subextensions: `Zba`, `Zbb` and `Zbs`. This commit fixes this issue and updates the reference URL to the source code of the latest ratified ISA Manual (version 20250508). To align with rust-lang/rust#145948, this commit performs rename, not removal.
Because unstable "B" (incorrectly named as `zb`) was there, this commit adds 28 minus one ("B") new extensions to be stabilized. This commit directly corresponds to rust-lang/rust#145948. References: 1. RISC-V Instruction Set Manual (version 20250508): <https://github.com/riscv/riscv-isa-manual/tree/20250508> 2. RISC-V Profiles (version 1.0 - RVA23 is not stabilized at the time): <https://github.com/riscv/riscv-profiles/tree/v1.0> 3. RISC-V Profiles (RVA23/RVB23-ratified version): <https://github.com/riscv/riscv-profiles/tree/rva23-rvb23-ratified>
The `Zb` extension does not exist and we instead have the `B` extension which is a superset of the three subextensions: `Zba`, `Zbb` and `Zbs`. This commit fixes this issue and updates the reference URL to the source code of the latest ratified ISA Manual (version 20250508). To align with rust-lang/rust#145948, this commit performs rename, not removal.
Because unstable "B" (incorrectly named as `zb`) was there, this commit adds 28 minus one ("B") new extensions to be stabilized. This commit directly corresponds to rust-lang/rust#145948. References: 1. RISC-V Instruction Set Manual (version 20250508): <https://github.com/riscv/riscv-isa-manual/tree/20250508> 2. RISC-V Profiles (version 1.0 - RVA23 is not stabilized at the time): <https://github.com/riscv/riscv-profiles/tree/v1.0> 3. RISC-V Profiles (RVA23/RVB23-ratified version): <https://github.com/riscv/riscv-profiles/tree/rva23-rvb23-ratified>
The `Zb` extension does not exist and we instead have the `B` extension which is a superset of the three subextensions: `Zba`, `Zbb` and `Zbs`. This commit fixes this issue and updates the reference URL to the source code of the latest ratified ISA Manual (version 20250508). To align with rust-lang/rust#145948, this commit performs rename, not removal.
Because unstable "B" (incorrectly named as `zb`) was there, this commit adds 28 minus one ("B") new extensions to be stabilized. This commit directly corresponds to rust-lang/rust#145948. References: 1. RISC-V Instruction Set Manual (version 20250508): <https://github.com/riscv/riscv-isa-manual/tree/20250508> 2. RISC-V Profiles (version 1.0 - RVA23 is not stabilized at the time): <https://github.com/riscv/riscv-profiles/tree/v1.0> 3. RISC-V Profiles (RVA23/RVB23-ratified version): <https://github.com/riscv/riscv-profiles/tree/rva23-rvb23-ratified>
@traviscross Thanks for pointing out!
I opened the PR rust-lang/reference#1987 to align with this.
Because unstable "B" (incorrectly named as `zb`) was there, this commit adds 28 minus one ("B") new extensions to be stabilized. This commit directly corresponds to rust-lang/rust#145948. References: 1. RISC-V Instruction Set Manual (version 20250508): <https://github.com/riscv/riscv-isa-manual/tree/20250508> 2. RISC-V Profiles (version 1.0 - RVA23 is not stabilized at the time): <https://github.com/riscv/riscv-profiles/tree/v1.0> 3. RISC-V Profiles (RVA23/RVB23-ratified version): <https://github.com/riscv/riscv-profiles/tree/rva23-rvb23-ratified>
No ABI issues will not occur unless floating point registers are involved. But for integer-only vector ones, we'll be stabilizing in another time.
I can't parse this comment, too many negations.^^ Integer vector registers can also cause ABI issues, if they are sometimes used by the calling convention but not always available.
I can't parse this comment, too many negations.
Double negation in the first sentence is intended. Still, there are too much negations after that, I have to admit. I'll try if I can improve this.
Integer vector registers can also cause ABI issues, if they are sometimes used by the calling convention but not always available.
Did I miss something?
In the standard ABI and normal cases, all vector registers are function-local temporaries.
If variant calling convention is used in a function, standard ABI defines how to communicate between the function itself (callee) and the caller using vector registers but nothing more (and all preserved vector registers are callee-saved). I can't find a case where vector integer ops can cause ABI issues.
Double negation in the first sentence is intended
Please avoid that, it makes things very hard to read and very easy to misunderstand.
"No ABI issues will not occur" means that ABI issues will occur which I hope is not what you meant.
If variant calling convention is used in a function, standard ABI defines how to communicate between the function itself (callee) and the caller using vector registers but nothing more (and all preserved vector registers are callee-saved). I can't find a case where vector integer ops can cause ABI issues.
I don't know what the "variant calling convention" is. I also don't know the specifics of any of these extensions, or really much about RISC-V at all. I am just replying to a comment that seemed to somehow say that "float registers could cause a problem but integers cannot", which doesn't make sense to me. Any extensions that makes more registers available could become a problem if arguments start to be passed in those registers. If there are no new registers (nor other new machine state) or the registers are never used for arguments, then great, we have no problem.
The comment seems to say that something with integer vectors should be stabilized "another time", so I hope we are fine. But I am not sure since I have a hard time parsing that sentence. It first talks about floats and then about ints, but ints happen another time -- so are floats affected now in this PR or not?
Sorry for being so nitpicky here, but I'd rather not risk introducing some soundness issue over a misunderstanding caused by a language barrier. Better to double-check. :)
Please avoid that, it makes things very hard to read and very easy to misunderstand.
Understood. I think I need to rewrite the entire paragraph.
I am just replying to a comment that seemed to somehow say that "float registers could cause a problem but integers cannot", which doesn't make sense to me.
The reason behind "floating point registers cause an ABI issue" is something like #[target_feature(enable = "f")]
can change how to pass floating point numbers.
However, passing something through vector registers require the callee to enable variant calling conventions (e.g. vector intrinsics in future core::arch
would enable variant calling convention) and we can only pass vector data (unavailable on non-vector environment).
On all other cases, vector registers are never used to pass information between functions.
b4455e4
to
a410473
Compare
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.
Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.
Rebased and made minor adjustments to the commit message (no changes in the code).
@RalfJung I think this is at least better than before.
- Not vector-related (floating point and integer).
While integer vector subsets should not cause any ABI issues (as they don't use ABI-dependent floating point registers), we need to discuss before stabilizing them.
EDIT: Minor correction is made after initial correction.
Sorry for being so nitpicky here, but I'd rather not risk introducing some soundness issue over a misunderstanding caused by a language barrier. Better to double-check. :)
I completely agree that. We'd better to discuss it.
This commit stabilizes RISC-V target features with following constraints: * Describes a ratified extension. * Implemented on Rust 1.88.0 or before. Waiting for three+ version cycles seems sufficient. * Does not disrupt current rustc's target feature + ABI handling. It excludes "E" and all floating point-arithmetic extensions. "Zfinx" family does not involve floating point registers but not stabilizing for now to avoid possible confusion between the "F" extension family. * Not vector-related (floating point and integer). While integer vector subsets should not cause any ABI issues (as they don't use ABI-dependent floating point registers), we need to discuss before stabilizing them. * Supported by the lowest LLVM version supported by rustc. It excludes the "Zacas" extension, newly supported in LLVM 20 (while the minimum LLVM version supported by rustc is 19). List of target features to be stabilized: 1. "b" 2. "za64rs" (no-RT) 3. "za128rs" (no-RT) 4. "zaamo" 5. "zabha" 6. "zalrsc" 7. "zama16b" (no-RT) 8. "zawrs" 9. "zca" 10. "zcb" 11. "zcmop" 12. "zic64b" (no-RT) 13. "zicbom" 14. "zicbop" (no-RT) 15. "zicboz" 16. "ziccamoa" (no-RT) 17. "ziccif" (no-RT) 18. "zicclsm" (no-RT) 19. "ziccrse" (no-RT) 20. "zicntr" 21. "zicond" 22. "zicsr" 23. "zifencei" 24. "zihintntl" 25. "zihintpause" 26. "zihpm" 27. "zimop" 28. "ztso" Of which, 19 of them (28 minus 9 "no-RT" target features) support runtime detection through `std::arch::is_riscv_feature_detected!()`.
a410473
to
873a0fc
Compare
Thanks! I'm still confused about what's happening with integer vectors, but since they are not being stabilized here we don't have to resolve that now.
Uh oh!
There was an error while loading. Please reload this page.
This commit stabilizes RISC-V target features with following constraints:
Waiting for three+ version cycles seems sufficient.
target_feature
compat #140570 ) + ABI (cf. Some-Ctarget-feature
s must be restrained on RISCV #132618 ) handling.It excludes
E
and all floating point-arithmetic extensions. TheZfinx
family does not involve floating point registers but not stabilizing for now to avoid possible confusion between theF
extension family.While integer vector subsets should not cause any ABI issues (as they don't use ABI-dependent floating point registers), we need to discuss before stabilizing them.
It excludes the
Zacas
extension, newly supported in LLVM 20 (while the minimum LLVM version supported by rustc is 19).List of target features to be stabilized:
b
za64rs
(no-RT)za128rs
(no-RT)zaamo
zabha
zalrsc
zama16b
(no-RT)zawrs
zca
zcb
zcmop
zic64b
(no-RT)zicbom
zicbop
(no-RT)zicboz
ziccamoa
(no-RT)ziccif
(no-RT)zicclsm
(no-RT)ziccrse
(no-RT)zicntr
zicond
zicsr
zifencei
zihintntl
zihintpause
zihpm
zimop
ztso
Of which, 19 of them (28 minus 9 "no-RT" target features) support runtime detection through
std::arch::is_riscv_feature_detected!()
.Corresponding PR for the Reference: rust-lang/reference#1987
Description in older revision(s)
The original text is rewritten to avoid confusion as per @RalfJung's request.
This is rewritten to correlate this PR with a public API (instead of an internal crate).
r? @Amanieu
@rustbot label +O-riscv