Skip to content

rustc_target: Add more RISC-V vector-related features and use zvl*b target features in vector ABI check #138742

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

Merged
merged 2 commits into from
Mar 30, 2025

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Mar 20, 2025

Currently, we have only unstable v target feature, but RISC-V have more vector-related extensions. The first commit of this PR adds them to unstable riscv_target_feature.

  • unaligned-vector-mem: Has reasonably performant unaligned vector
    • LLVM definition
    • Similar to currently unstable unaligned-scalar-mem target feature, but for vector instructions.
  • zvfh: Vector Extension for Half-Precision Floating-Point
  • zvfhmin: Vector Extension for Minimal Half-Precision Floating-Point
  • zve32x, zve32f, zve64x, zve64f, zve64d: Vector Extensions for Embedded Processors
    • ISA Manual
    • LLVM definitions
    • zve32x implies zvl32b
    • zve32f implies zve32x and f
    • zve64x implies zve32x and zvl64b
    • zve64f implies zve32f and zve64x
    • zve64d implies zve64f and d
    • v implies zve64d
  • zvl*b: Minimum Vector Length Standard Extensions
  • Vector Cryptography and Bit-manipulation Extensions
    • ISA Manual
    • LLVM definitions
    • zvkb: Vector Bit-manipulation used in Cryptography
      • This implies zve32x
    • zvbb: Vector basic bit-manipulation instructions
      • This implies zvkb
    • zvbc: Vector Carryless Multiplication
      • This implies zve64x
    • zvkg: Vector GCM instructions for Cryptography
      • This implies zve32x
    • zvkned: Vector AES Encryption & Decryption (Single Round)
      • This implies zve32x
    • zvknha: Vector SHA-2 (SHA-256 only))
      • This implies zve32x
    • zvknhb: Vector SHA-2 (SHA-256 and SHA-512)
      • This implies zve64x
      • This is superset of zvknha, but doesn't imply that feature at least in LLVM
    • zvksed: SM4 Block Cipher Instructions
      • This implies zve32x
    • zvksh: SM3 Hash Function Instructions
      • This implies zve32x
    • zvkt: Vector Data-Independent Execution Latency
      • Similar to already stabilized scalar cryptography extension zkt.
    • zvkn: Shorthand for 'Zvkned', 'Zvknhb', 'Zvkb', and 'Zvkt'
      • Similar to already stabilized scalar cryptography extension zkn.
    • zvknc: Shorthand for 'Zvkn' and 'Zvbc'
    • zvkng: shorthand for 'Zvkn' and 'Zvkg'
    • zvks: shorthand for 'Zvksed', 'Zvksh', 'Zvkb', and 'Zvkt'
      • Similar to already stabilized scalar cryptography extension zks.
    • zvksc: shorthand for 'Zvks' and 'Zvbc'
    • zvksg: shorthand for 'Zvks' and 'Zvkg'

Also, our vector ABI check wants zvl*b target features, the second commit of this PR updates vector ABI check to use them.

const RISCV_FEATURES_FOR_CORRECT_VECTOR_ABI: &'static [(u64, &'static str)] =
&[/*(64, "zvl64b"), */ (128, "v")];


r? @Amanieu

@rustbot label +O-riscv +A-target-feature

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. O-riscv Target: RISC-V architecture labels Mar 20, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@a4lg
Copy link
Contributor

a4lg commented Mar 22, 2025

Second this.
Note: if #138823 is merged, zve32x should have dependency to zicsr.

@Amanieu
Copy link
Member

Amanieu commented Mar 28, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 28, 2025

📌 Commit 8a6f96a has been approved by Amanieu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 28, 2025
@bors
Copy link
Collaborator

bors commented Mar 30, 2025

⌛ Testing commit 8a6f96a with merge 85f518e...

@bors
Copy link
Collaborator

bors commented Mar 30, 2025

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 85f518e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 30, 2025
@bors bors merged commit 85f518e into rust-lang:master Mar 30, 2025
7 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Mar 30, 2025
Copy link

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 2196aff (parent) -> 85f518e (this PR)

Test differences

No test diffs found

Job duration changes

  1. dist-apple-various: 5427.7s -> 9487.7s (74.8%)
  2. x86_64-apple-1: 7033.8s -> 7703.2s (9.5%)
  3. dist-riscv64-linux: 4805.3s -> 5123.0s (6.6%)
  4. aarch64-gnu-debug: 5928.0s -> 6319.5s (6.6%)
  5. aarch64-apple: 3708.3s -> 3906.9s (5.4%)
  6. dist-aarch64-apple: 4946.3s -> 5199.0s (5.1%)
  7. i686-mingw-3: 8258.8s -> 8630.5s (4.5%)
  8. dist-aarch64-linux: 7372.3s -> 7658.6s (3.9%)
  9. dist-x86_64-apple: 6913.5s -> 7176.4s (3.8%)
  10. i686-mingw-1: 7153.9s -> 7400.2s (3.4%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@taiki-e taiki-e deleted the riscv-vector branch March 30, 2025 05:36
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (85f518e): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.4% [0.3%, 0.5%] 5
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -2.5%, secondary 2.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.3% [0.8%, 4.3%] 4
Improvements ✅
(primary)
-2.5% [-2.5%, -2.5%] 1
Improvements ✅
(secondary)
-2.1% [-2.1%, -2.1%] 1
All ❌✅ (primary) -2.5% [-2.5%, -2.5%] 1

Cycles

Results (secondary 5.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
5.5% [2.6%, 8.4%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 777.614s -> 776.851s (-0.10%)
Artifact size: 365.95 MiB -> 365.87 MiB (-0.02%)

Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 2, 2025
…, r=Amanieu

rustc_target: RISC-V: add base `I`-related important extensions

Of ratified RISC-V features defined, this commit adds extensions satisfying following criteria:

*   Formerly a part of the `I` extension and splitted thereafter (now ratified as `I` + `Zifencei` + `Zicsr` + `Zicntr` + `Zihpm`) or
*   Dicoverable from newer versions of the Linux kernel and implemented as a part of `std_detect`'s feature (`Zihintpause`) and
*   Available on LLVM 18.

This is based on [the latest ratified ISA Manuals (version 20240411)](https://lf-riscv.atlassian.net/wiki/spaces/HOME/pages/16154769/RISC-V+Technical+Specifications).

LLVM Definitions:

*   [`Zifencei`](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L133-L137)
*   [`Zicsr`](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L116-L120)
*   [`Zicntr`](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L122-L124)
*   [`Zihpm`](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L153-L155)
*   [`Zihintpause`](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L139-L144)

Additional (1):
One of those, `Zicsr`, is a dependency of many other ISA extensions and this commit adds correct dependencies to `Zicsr`.

Additional (2):
In RISC-V, `G` is an abbreviation of following extensions:
*   `I`
*   `M`
*   `A`
*   `F`
*   `D`
*   `Zicsr` (although implied by `F`)
*   `Zifencei`

and all RISC-V targets with the `G` abbreviation and targets for Android / VxWorks are updated accordingly.

Note:

Android will require RVA22 (likely RVA22U64) and some more extensions, which is a superset of RV64GC.  For VxWorks, all BSPs currently distributed by Wind River are for boards with RV64GC (this commit also updates `riscv32-wrs-vxworks` though).

--------

This is the version 4.
`Ztso` in the original proposal is removed on the PR version 2 due to the minimum LLVM version (non-experimental `Ztso` requires LLVM 19 while minimum LLVM version of Rust is 18). This is not back in PR version 3 and 4 after noticing adding `Ztso` is possible by checking host LLVM version because PR version 3 introduces compiler target changes (and adding more extensions would complicate the problems; sorry `Zihintpause`).

Version 4:
*   Fixed some commit messages,
*   Added Android / VxWorks targets to imply `G` and
*   Added an implication from `Zve32x` to `Zicsr` (which makes all vector extension subsets to imply `Zicsr`)
    since rust-lang#138742 is now merged.

Related:
*   rust-lang#44839
    (`riscv_target_feature`)
*   rust-lang#114544
    (This PR can be a prerequisite of resolving a part of that tracking issue)
*   rust-lang#138742
    (Touches the same place and vector extensions depend on `Zicsr`)

NOT Related but linked:
*   rust-lang#132618
    (This PR won't be blocked by this issue since none of those extensions do not change the ABI)

`@rustbot` r? `@Amanieu`
`@rustbot` label +T-compiler +O-riscv +A-target-feature
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 2, 2025
Rollup merge of rust-lang#138823 - a4lg:riscv-feature-addition-base-i, r=Amanieu

rustc_target: RISC-V: add base `I`-related important extensions

Of ratified RISC-V features defined, this commit adds extensions satisfying following criteria:

*   Formerly a part of the `I` extension and splitted thereafter (now ratified as `I` + `Zifencei` + `Zicsr` + `Zicntr` + `Zihpm`) or
*   Dicoverable from newer versions of the Linux kernel and implemented as a part of `std_detect`'s feature (`Zihintpause`) and
*   Available on LLVM 18.

This is based on [the latest ratified ISA Manuals (version 20240411)](https://lf-riscv.atlassian.net/wiki/spaces/HOME/pages/16154769/RISC-V+Technical+Specifications).

LLVM Definitions:

*   [`Zifencei`](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L133-L137)
*   [`Zicsr`](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L116-L120)
*   [`Zicntr`](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L122-L124)
*   [`Zihpm`](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L153-L155)
*   [`Zihintpause`](https://github.com/llvm/llvm-project/blob/llvmorg-20.1.0/llvm/lib/Target/RISCV/RISCVFeatures.td#L139-L144)

Additional (1):
One of those, `Zicsr`, is a dependency of many other ISA extensions and this commit adds correct dependencies to `Zicsr`.

Additional (2):
In RISC-V, `G` is an abbreviation of following extensions:
*   `I`
*   `M`
*   `A`
*   `F`
*   `D`
*   `Zicsr` (although implied by `F`)
*   `Zifencei`

and all RISC-V targets with the `G` abbreviation and targets for Android / VxWorks are updated accordingly.

Note:

Android will require RVA22 (likely RVA22U64) and some more extensions, which is a superset of RV64GC.  For VxWorks, all BSPs currently distributed by Wind River are for boards with RV64GC (this commit also updates `riscv32-wrs-vxworks` though).

--------

This is the version 4.
`Ztso` in the original proposal is removed on the PR version 2 due to the minimum LLVM version (non-experimental `Ztso` requires LLVM 19 while minimum LLVM version of Rust is 18). This is not back in PR version 3 and 4 after noticing adding `Ztso` is possible by checking host LLVM version because PR version 3 introduces compiler target changes (and adding more extensions would complicate the problems; sorry `Zihintpause`).

Version 4:
*   Fixed some commit messages,
*   Added Android / VxWorks targets to imply `G` and
*   Added an implication from `Zve32x` to `Zicsr` (which makes all vector extension subsets to imply `Zicsr`)
    since rust-lang#138742 is now merged.

Related:
*   rust-lang#44839
    (`riscv_target_feature`)
*   rust-lang#114544
    (This PR can be a prerequisite of resolving a part of that tracking issue)
*   rust-lang#138742
    (Touches the same place and vector extensions depend on `Zicsr`)

NOT Related but linked:
*   rust-lang#132618
    (This PR won't be blocked by this issue since none of those extensions do not change the ABI)

`@rustbot` r? `@Amanieu`
`@rustbot` label +T-compiler +O-riscv +A-target-feature
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-target-feature Area: Enabling/disabling target features like AVX, Neon, etc. merged-by-bors This PR was explicitly merged by bors. O-riscv Target: RISC-V architecture S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants