Skip to content

Several fixes to the loongarch CRC intrinsics#2136

Merged
sayantn merged 2 commits into
rust-lang:mainfrom
Gelbpunkt:la64-crc
May 25, 2026
Merged

Several fixes to the loongarch CRC intrinsics#2136
sayantn merged 2 commits into
rust-lang:mainfrom
Gelbpunkt:la64-crc

Conversation

@Gelbpunkt
Copy link
Copy Markdown
Contributor

@Gelbpunkt Gelbpunkt commented May 24, 2026

The first commit makes the CRC intrinsics exclusive to 64-bit LoongArch, since they are exclusive to LA64 in Clang and GCC (see https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp#L4797).

The second commit aligns the CRC intrinsic definitions for 8 and 16-bit wide data with Clang and GCC, which both use char and short rather than int for the data parameter (see https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/larchintrin.h#L60 and https://github.com/gcc-mirror/gcc/blob/master/gcc/config/loongarch/larchintrin.h#L152). The ARM CRC intrinsics are handled identically in stdarch already: there, the LLVM intrinsic also takes an i32 but we cast from u8 or u16 (https://doc.rust-lang.org/stable/src/core/stdarch/crates/core_arch/src/arm_shared/neon/generated.rs.html#63).

Gelbpunkt added 2 commits May 24, 2026 23:46
GCC and LLVM both only allow using the CRC intrinsics on LA64.
Clang and GCC use char and short for the data parameter type where
appropriate. The LLVM intrinsics take an i32. We should match clang and
perform a cast here.
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 24, 2026

r? @sayantn

rustbot has assigned @sayantn.
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

@Gelbpunkt
Copy link
Copy Markdown
Contributor Author

cc @heiher

Copy link
Copy Markdown
Contributor

@folkertdev folkertdev left a comment

Choose a reason for hiding this comment

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

LGTM but needs the target maintainer's approval too

View changes since this review

Copy link
Copy Markdown
Contributor

@heiher heiher left a comment

Choose a reason for hiding this comment

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

@sayantn sayantn added this pull request to the merge queue May 25, 2026
Merged via the queue into rust-lang:main with commit 5e930b6 May 25, 2026
82 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants