Skip to content

RISC-V: Add Zmmul target feature - #162552

Merged
rust-bors[bot] merged 4 commits into
rust-lang:mainfrom
TechnoPorg:riscv-feature-zmmul
Sep 18, 2026
Merged

rust-bors[bot] merged 4 commits into
rust-lang:mainfrom
TechnoPorg:riscv-feature-zmmul

Conversation

@TechnoPorg

@TechnoPorg TechnoPorg commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Zmmul is a ratified subset of the M extension that only supports multiplication, but not division (spec). LLVM already supports the extension under the same name, so until #162235 I'd just been using it directly.

There are two things I would particularly appreciate input on:

  • Should M imply Zmmul?
    • From the Machine-Level ISA spec and this discussion in the RISC-V ISA Manual repo, it seems that Zmmul means all multiplication instructions are always supported, whereas M means they're only supported if the M bit is set in the misa register.
    • On the other hand, LLVM has M imply Zmmul (RISCVFeatures.td), so it feels nice to match that.
  • How might I go about stabilizing this?

No LLMs were used in the making of this PR, and all mistakes are those of a first-time rustc contributor.

@rustbot

rustbot commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

Target features are being changed; ensure all ABI effects are being accounted for

cc @RalfJung

Some changes occurred in std_detect

cc @Amanieu, @folkertdev, @sayantn

@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. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. labels Sep 9, 2026
@rustbot

rustbot commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the pull request, and welcome! The Rust Project has assigned @Darksonn (or someone else) to review your changes, you should hear from them (or someone else) within the next two weeks.

Please see the contribution instructions and our LLM policy for more information.

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: libs
  • libs expanded to 12 candidates
  • Random selection from Darksonn, JohnTitor, Mark-Simulacrum

@RalfJung

RalfJung commented Sep 9, 2026

Copy link
Copy Markdown
Member

On the other hand, LLVM has M imply Zmmul (RISCVFeatures.td), so it feels nice to match that.

LLVM can change their implications, we cannot. So we better be sure if we want to have this implication.
@rustbot ping risc-v

How might I go about stabilizing this?

It's too early to worry about this. Add it unstably first.

@rustbot rustbot added the O-riscv Target: RISC-V architecture label Sep 9, 2026
@rustbot

rustbot commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Hey RISC-V Group! This issue has been identified as a good "RISC-V candidate".
In case it's useful, here are some instructions for tackling these sorts of
issues. Maybe take a look?
Thanks! <3

cc @denisvasilik @kraj @sweiglbosker @tblah

@Darksonn

Copy link
Copy Markdown
Member

r? compiler

@rustbot rustbot assigned mejrs and unassigned Darksonn Sep 10, 2026
@beetrees

Copy link
Copy Markdown
Contributor

From the Machine-Level ISA spec and this discussion in the RISC-V ISA Manual repo, it seems that Zmmul means all multiplication instructions are always supported, whereas M means they're only supported if the M bit is set in the misa register.

While the M ISA extension means the instructions are supported when the M bit is set, the m Rust/LLVM target feature means that the instructions are always supported (i.e. if the m target feature is enabled, the M bit must be set to 1). In my opinion, this means that the m target feature implies the zmmul target feature, as the target features are just about what instructions etc. are definitely available, not about how the respective ISA extensions are enabled by the kernel.

@RalfJung

RalfJung commented Sep 10, 2026 via email

Copy link
Copy Markdown
Member

@TechnoPorg

Copy link
Copy Markdown
Contributor Author

I've put a comment explaining the difference between the ISA extension and the target feature in target_features.rs. If there's a better home for it elsewhere I can move it.

@RalfJung

Copy link
Copy Markdown
Member

Since "m" is stable it should be mentioned in the reference. Though having a comment in the code, at least a brief one saying that "m" also means that we assume that the bit is actually set at runtime, is still also a good idea.

@kraj kraj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

few nits from my end.

View changes since this review

Comment thread compiler/rustc_target/src/target_features.rs
Comment thread library/std_detect/src/detect/arch/riscv.rs Outdated
Comment thread library/std_detect/src/detect/arch/riscv.rs Outdated
@TechnoPorg
TechnoPorg requested a review from kraj September 11, 2026 14:48

@kraj kraj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@mejrs

mejrs commented Sep 15, 2026

Copy link
Copy Markdown
Member

r? @beetrees

@rustbot rustbot assigned beetrees and unassigned mejrs Sep 15, 2026

@sweiglbosker sweiglbosker left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This seems reasonable

View changes since this review

@beetrees

Copy link
Copy Markdown
Contributor

Thanks.

@bors r+

@rust-bors

rust-bors Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

📌 Commit a33e6eb has been approved by beetrees

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 Sep 17, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 17, 2026
…beetrees

RISC-V: Add Zmmul target feature

Zmmul is a ratified subset of the M extension that only supports multiplication, but not division ([spec](https://docs.riscv.org/reference/isa/v20260120/unpriv/m-st-ext.html#11-1-3-zmmul-extension-version-1-0)). LLVM already supports the extension under the same name, so until rust-lang#162235 I'd just been using it directly.

There are two things I would particularly appreciate input on:

- Should M imply Zmmul?
  - From the [Machine-Level ISA spec](https://docs.riscv.org/reference/isa/v20260120/priv/machine.html) and [this discussion](riscv/riscv-isa-manual#869) in the RISC-V ISA Manual repo, it seems that Zmmul means all multiplication instructions are always supported, whereas M means they're only supported if the M bit is set in the `misa` register.
  - On the other hand, LLVM has M imply Zmmul ([RISCVFeatures.td](https://github.com/llvm/llvm-project/blob/f6f71edb4346d586bb97485116175efd9a696ee8/llvm/lib/Target/RISCV/RISCVFeatures.td#L214-L229)), so it feels nice to match that.
- How might I go about stabilizing this?
  - Can it be done right away?
  - Should it wait a few releases?
  - Is it better to batch it with the stabilization of other RISC-V target features, similarly to how it was done in rust-lang#145948?

No LLMs were used in the making of this PR, and all mistakes are those of a first-time rustc contributor.
rust-bors Bot pushed a commit that referenced this pull request Sep 18, 2026
…uwer

Rollup of 10 pull requests

Successful merges:

 - #161987 (fix `is_single_fp_element` for `s390x` and `x86`)
 - #162366 (Replace hard linked `metadata.rmeta` with a copy when finalizing the incremental compilation session dir fails)
 - #162552 (RISC-V: Add Zmmul target feature)
 - #162602 (Ensure concurrent rustc instances in the same process correctly lock the session dir)
 - #162885 (Remove one remaining use of `Level::Help`)
 - #162606 (c-b: Export `fmaf16` now that we have an implementation)
 - #162817 (rustc_{codegen_,}llvm: adapt to new ThreadModel API)
 - #162860 (offload: add libLLVM rpath for libomptarget)
 - #162895 (Avoid derive edits on macro-generated types)
 - #162899 (Error on invalid placements of the `cfi_encoding` attribute.)
@rust-bors
rust-bors Bot merged commit 370012b into rust-lang:main Sep 18, 2026
13 checks passed
@rustbot rustbot added this to the 1.100.0 milestone Sep 18, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 18, 2026
Rollup merge of #162552 - TechnoPorg:riscv-feature-zmmul, r=beetrees

RISC-V: Add Zmmul target feature

Zmmul is a ratified subset of the M extension that only supports multiplication, but not division ([spec](https://docs.riscv.org/reference/isa/v20260120/unpriv/m-st-ext.html#11-1-3-zmmul-extension-version-1-0)). LLVM already supports the extension under the same name, so until #162235 I'd just been using it directly.

There are two things I would particularly appreciate input on:

- Should M imply Zmmul?
  - From the [Machine-Level ISA spec](https://docs.riscv.org/reference/isa/v20260120/priv/machine.html) and [this discussion](riscv/riscv-isa-manual#869) in the RISC-V ISA Manual repo, it seems that Zmmul means all multiplication instructions are always supported, whereas M means they're only supported if the M bit is set in the `misa` register.
  - On the other hand, LLVM has M imply Zmmul ([RISCVFeatures.td](https://github.com/llvm/llvm-project/blob/f6f71edb4346d586bb97485116175efd9a696ee8/llvm/lib/Target/RISCV/RISCVFeatures.td#L214-L229)), so it feels nice to match that.
- How might I go about stabilizing this?
  - Can it be done right away?
  - Should it wait a few releases?
  - Is it better to batch it with the stabilization of other RISC-V target features, similarly to how it was done in #145948?

No LLMs were used in the making of this PR, and all mistakes are those of a first-time rustc contributor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants