Skip to content

Stabilize LoongArch64 SIMD Intrinsics - #2221

Open
heiher wants to merge 2 commits into
rust-lang:mainfrom
heiher:stabilize-loong64-simd
Open

heiher wants to merge 2 commits into
rust-lang:mainfrom
heiher:stabilize-loong64-simd

Conversation

@heiher

@heiher heiher commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Tracking issue: rust-lang/rust#162508

LoongArch64 SIMD intrinsics are already used by rustc itself and by a number of downstream projects. Experience from these real-world uses indicates that the API is ready for stabilization. Stabilizing the intrinsics would remove the nightly requirement for applications that currently depend on them.

API surface

Note for subtree update

Remove this line: https://github.com/rust-lang/rust/blob/1edd55dcfcd573872c727fa3e086369a71661ee0/compiler/rustc_span/src/lib.rs#L20

@rustbot

rustbot commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

r? @folkertdev

rustbot has assigned @folkertdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @Amanieu, @adamgemmell, @davidtwco, @folkertdev, @sayantn
  • @Amanieu, @adamgemmell, @davidtwco, @folkertdev, @sayantn expanded to Amanieu, adamgemmell, davidtwco, folkertdev, sayantn
  • Random selection from Amanieu, adamgemmell, davidtwco, folkertdev, sayantn

Comment on lines 910 to 915
#[inline]
#[target_feature(enable = "lasx")]
#[unstable(feature = "stdarch_loongarch", issue = "117427")]
#[stable(feature = "stdarch_loongarch_simd", since = "CURRENT_RUSTC_VERSION")]
pub fn lasx_xvsrar_b(a: m256i, b: m256i) -> m256i {
unsafe { transmute(__lasx_xvsrar_b(transmute(a), transmute(b))) }
}

@tgross35 tgross35 Sep 9, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What's the reason for changing the feature name?

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What's the reason for changing the feature name?

There are also some unstable APIs gated by stdarch_loongarch. Having both stable and unstable APIs gated by the same feature name causes the following error:

error[E0711]: feature `stdarch_loongarch` is declared stable, but was previously declared unstable

Also, putting the SIMD intrinsics under stdarch_loongarch_simd makes the categorization more explicit, similar to stdarch_loongarch_crc.

@tgross35

tgross35 commented Sep 9, 2026

Copy link
Copy Markdown
Member

Note that this stabilizes more cases of rustc_legacy_const_generics, which was discussed at rust-lang/rust#149654. Cc @rust-lang/lang

Nominating since that's probably worth clarifying, and also this is a lot of surface area with potential sharp edges.

@rustbot label +I-libs-nominated

@rustbot

rustbot commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Error: The feature relabel is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip.

@heiher

heiher commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Note that this stabilizes more cases of rustc_legacy_const_generics, which was discussed at rust-lang/rust#149654.

Thanks for pointing this out. As far as I know, no projects currently depend on the LoongArch SIMD API signatures using the rustc_legacy_const_generics form, so there are no legacy compatibility concerns here. If adding new instances of rustc_legacy_const_generics is not desirable, we can simply remove it.

@heiher
heiher force-pushed the stabilize-loong64-simd branch from 5f1c38d to bc04e14 Compare September 9, 2026 13:11
@heiher

heiher commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

rustc_legacy_const_generics has been removed from LoongArch64 SIMD in v2.

@clarfonthey clarfonthey added the I-libs-nominated Nominated for discussion in a libs team meeting. label Sep 13, 2026
@clarfonthey

Copy link
Copy Markdown
Contributor

@rfcbot merge libs

@rust-rfcbot

rust-rfcbot commented Sep 13, 2026 •

Copy link
Copy Markdown
Collaborator

@clarfonthey 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!

See this document for info about what commands tagged team members can give me.

@rust-rfcbot rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This PR / issue is in pFCP or FCP with a disposition to merge it. labels Sep 13, 2026
@clarfonthey clarfonthey removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This PR / issue is in pFCP or FCP with a disposition to merge it. I-libs-nominated Nominated for discussion in a libs team meeting. labels Sep 13, 2026
@Amanieu

Amanieu commented Sep 13, 2026

Copy link
Copy Markdown
Member

It would be nice to have this fuzz tested against the C intrinsics using intrinsic-test before stabilizing.

@rfcbot concern fuzz testing

@Amanieu Amanieu added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This PR / issue is in pFCP or FCP with a disposition to merge it. labels Sep 13, 2026
@adamgemmell

Copy link
Copy Markdown
Contributor

There was a previous attempt at doing so at #1900, though it never fully passed CI and will need some rebasing.

@heiher

heiher commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

There was a previous attempt at doing so at #1900, though it never fully passed CI and will need some rebasing.

Thanks! There's a new impl in #2226 with all related CI checks passing.

@traviscross

traviscross commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

With these intrinsics, Miri reports UB in safe code:

#![feature(stdarch_loongarch)]
use std::arch::is_loongarch_feature_detected;
use std::arch::loongarch64::*;

#[target_feature(enable = "lsx")]
fn probe() -> i32 {
  let ones = lsx_vrepli_b::<1>();
  let zero = lsx_vrepli_b::<0>();
  let r = lsx_vdiv_b(ones, zero);
  lsx_vpickve2gr_b::<0>(r)
}

fn main() {
  if is_loongarch_feature_detected!("lsx") {
    // SAFETY: We just checked: the `lsx` feature is present.
    let lane0 = unsafe { probe() };
    println!("lane 0 = {lane0}");
  }
}

Godbolt link

From #2078, see:

cc @sayantn

@heiher

heiher commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

With these intrinsics, Miri reports UB in safe code:

Good catch!

I double-checked this with the LoongArch ISA developer. It differs from the previous comment in PR #2078. The ISA defines vdiv/vmod to return 0 if the divisor is 0. (This behavior will be documented in the SIMD reference)

So it seems we can't use portable simd_div/simd_rem here. I'll revert this part. Any other better approach? Thanks!

@Amanieu

Amanieu commented Sep 17, 2026

Copy link
Copy Markdown
Member

There are 2 options, depending on what the intrinsic behavior should be:

  1. Make it unsafe and declare that it is UB if any vector lane of the divisor is 0.
  2. Keep it safe and use LLVM target-specific intrinsics or inline asm to implement it.

@heiher

heiher commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

There are 2 options, depending on what the intrinsic behavior should be:

  1. Make it unsafe and declare that it is UB if any vector lane of the divisor is 0.
  2. Keep it safe and use LLVM target-specific intrinsics or inline asm to implement it.

Thanks! A new PR has been created based on option 2: #2229

heiher added a commit to heiher/stdarch that referenced this pull request Sep 18, 2026
Miri reports UB on division by zero with the LSX vdiv/vmod
intrinsics when implemented via the portable simd_div/simd_rem
helpers. [^1]

Per clarification from the LoongArch ISA developers, the hardware
defines that vdiv/vmod return 0 when the divisor is zero (this
will be documented in the SIMD reference). This differs from the
earlier assumption in PR rust-lang#2078, so the portable helpers are not
a correct match.

Switch back to the corresponding LLVM intrinsics so the Rust
wrappers preserve the defined ISA behavior.

[^1]: rust-lang#2221 (comment)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition-merge This PR / issue is in pFCP or FCP with a disposition to merge it. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants