-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Stabilize 29 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:
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. |
Consideration: Feature Naming (for
|
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>
|
So…the idea was to wait for the 1.92 cycle. But now we're on the 1.94 cycle. Any plans? |
|
This is currently waiting for a checkbox by @BurntSushi, @scottmcm, or @the8472. |
|
Also Cc @kito-cheng @michaelmaitland @robin-randhawa-sifive @topperc as Risc-V target maintainers, to double-check that these target features are safe to stabilize, in particular from an ABI perspective. :) |
The list looks good to me. |
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
|
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. |
|
@bors r+ |
|
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 50d5940 (parent) -> 00f49d2 (this PR) Test differencesShow 2 test diffs2 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 00f49d22042dce59561b36eb075bbf5e343c23c4 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (00f49d2): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (secondary 3.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 470.889s -> 472.503s (0.34%) |
This commit stabilizes RISC-V target features with following constraints:
Waiting for four+ version cycles seems sufficient.
target_featurecompat #140570) + ABI (cf. Some-Ctarget-features must be restrained on RISCV #132618) handling.It excludes
Eand all floating point-arithmetic extensions. TheZfinxfamily does not involve floating point registers but not stabilizing for now to avoid possible confusion between theFextension 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.
List of target features to be stabilized:
bza64rs(no-RT)za128rs(no-RT)zaamozabhazacaszalrsczama16b(no-RT)zawrszcazcbzcmopzic64b(no-RT)zicbomzicbop(no-RT)zicbozziccamoa(no-RT)ziccif(no-RT)zicclsm(no-RT)ziccrse(no-RT)zicntrzicondzicsrzifenceizihintntlzihintpausezihpmzimopztsoOf which, 20 of them (29 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