Skip to content

Conversation

@sayantn
Copy link
Contributor

@sayantn sayantn commented Sep 25, 2025

Tracking issue: rust-lang/rust#149298
closes: #1076

(This uses stdarch_const_x86 for the intrinsics only, and stdarch_const_helpers for everything else, so we can go for aarch64 next)

Ik the diffcount is a lot, but most of it is just boilerplate. It is easier to review on a per-commit basis, and the following commits contain the "interesting" bits (others are just soulless adding of const keyword everywhere)

  • 7134a49 (modifies the integer reduce_add and reduce_mul intrinsics to use simd_reduce_add_ordered and simd_ordered_mul_ordered, respectively. As integer algebra is associative, it doesn't matter for correctness, and LLVM will use the most performant arrangement. The reason is that rustc_const_eval only implements these ordered reduction intrinsics, unordered reduction seems to be tricky to implement in const_eval)
  • 1f94820 (rewrites some test helper functions for const)
  • 90ca47b (modifies #[simd_test] quite a bit, adds a new "no arguments" mode to ease testing of intrinsics that require no target features and adds support for const testing)
  • 962e4ab and 36905a2 (contains some modifications to avoid pointer-to-usize casts and for loops)

@sayantn sayantn force-pushed the make-const branch 2 times, most recently from b73089f to f8fe8b8 Compare October 1, 2025 06:02
@sayantn sayantn force-pushed the make-const branch 2 times, most recently from 42d6966 to 220fc5a Compare October 5, 2025 01:49
@sayantn sayantn force-pushed the make-const branch 2 times, most recently from 2830224 to ec6eef8 Compare November 13, 2025 11:44
@sayantn sayantn force-pushed the make-const branch 2 times, most recently from 4d1ed15 to 58c2424 Compare November 23, 2025 05:54
@sayantn sayantn marked this pull request as ready for review November 23, 2025 06:12
@rustbot
Copy link
Collaborator

rustbot commented Nov 23, 2025

r? @Amanieu

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

#[track_caller]
#[target_feature(enable = "sse2")]
pub unsafe fn assert_eq_m128i(a: __m128i, b: __m128i) {
#[rustc_const_unstable(feature = "stdarch_const_helpers", issue = "none")]
Copy link
Member

Choose a reason for hiding this comment

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

Is this actually needed? These functions aren't exported and shouldn't need stability attributes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, unfortunately const-stability attributes are needed even for private functions

#[target_feature(enable = "sse2")]
pub unsafe fn assert_eq_m128d(a: __m128d, b: __m128d) {
ct: unsafe {
assert_eq!(transmute::<_, [f64; 2]>(a), transmute::<_, [f64; 2]>(b))
Copy link
Member

Choose a reason for hiding this comment

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

Since this is only used in tests, could we always us the ct version here? That would simplify the code.

Copy link
Contributor Author

@sayantn sayantn Dec 3, 2025

Choose a reason for hiding this comment

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

The ct version doesn't print the left and right arguments, because debug-printing is not const yet. I wanted to print the values in rt at least, so had 2 separate impls.

Also one more problem is the ct version uses simple float equality, which differs from intel's definition for denormals and nans. In the functions we actually call the const version, we don't have such problems because those have standard float conventions.

@Amanieu Amanieu added this pull request to the merge queue Dec 4, 2025
@Amanieu Amanieu removed this pull request from the merge queue due to a manual request Dec 4, 2025
@Amanieu Amanieu added this pull request to the merge queue Dec 4, 2025
Merged via the queue into rust-lang:main with commit 816e303 Dec 4, 2025
73 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.

Most AVX and AVX2 intrinsics should be const

3 participants