Make SIMD intrinsics available in const-contexts
#147521
Conversation
|
Some changes occurred to the platform-builtins intrinsics. Make sure the cc @antoyo, @GuillaumeGomez, @bjorn3, @calebzulawski, @programmerjake The Miri subtree was changed cc @rust-lang/miri Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter |
|
|
|
Cc @Amanieu |
| let r: f32 = simd_reduce_mul_unordered(x); | ||
| assert_eq!(r, -24_f32); | ||
| let x = f32x4::from_array([1., -2., 3., 4.]); | ||
| let r: f32 = simd_reduce_add_ordered(x, 0.); |
There was a problem hiding this comment.
I know this wasn't there originally, but maybe we should add a test that requires being added in the proper order to return the right result? something like 1.0e20f32 + 1.0 - 1.0e20 - 1.0 == -1.0
There was a problem hiding this comment.
that seems like a nice idea, but I really don't want to add new tests in this PR - this is pretty big by itself anyway
8c71ce0 to
48324c2
Compare
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #148507) made this pull request unmergeable. Please resolve the merge conflicts. |
48324c2 to
76ddb6f
Compare
This comment has been minimized.
This comment has been minimized.
76ddb6f to
d1b2151
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
d1b2151 to
8715316
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
There was a problem hiding this comment.
Haven't looked through all the tests, will wait until we figure out whether we can just overwrite assert_eq!, from a quick glance they look fine (they were also reviewed previously in #146568 by @tgross35).
| #[rustc_intrinsic] | ||
| #[rustc_nounwind] | ||
| pub unsafe fn simd_add<T>(x: T, y: T) -> T; | ||
| pub const unsafe fn simd_add<T>(x: T, y: T) -> T; |
There was a problem hiding this comment.
I am unsure of the process here: does this need a compiler FCP to confirm that, yes, we do want t-libs to be able to start depending on these intrinsics being const?
#146568 was reviewed by Ralf, so I guess that's close to as good as it gets in terms of "wg-const-eval is fine with this".
There was a problem hiding this comment.
I don't know, @Amanieu do we need to do some kind of libs fcp??
There was a problem hiding this comment.
The team in charge of intrinsics is t-lang. They will have to be involved when stabilizing any of this, i.e., when adding rustc_intrinsic_const_stable_indirect to an intrinsic.
For the unstable part, notifying @rust-lang/wg-const-eval is enough. From my side this is fine: the only part that needs closer scrutiny is the fact that these are float operations, but we made them behave just like their scalar equivalents which seems right to me.
There was a problem hiding this comment.
Ah so we can continue on this 🎉
Trevor left two comments but didn't indicate whether they even looked at the entire rest of the PR. |
8715316 to
e48c21b
Compare
|
Thanks for the answers, feel free to r=me when you've rebased, and when we've figured out if we've followed the right process. |
24bfe49 to
82ff614
Compare
|
I had forgotten about @bors r+ rollup |
Rollup of 7 pull requests Successful merges: - #147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher) - #147421 (Add check if span is from macro expansion) - #147521 (Make SIMD intrinsics available in `const`-contexts) - #148201 (Start documenting autodiff activities) - #148797 (feat: Add `bit_width` for unsigned `NonZero<T>`) - #148798 (Match <OsString as Debug>::fmt to that of str) - #149082 (autodiff: update formating, improve examples for the unstable-book) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #147521 - sayantn:simd-const-intrinsics, r=madsmtm Make SIMD intrinsics available in `const`-contexts successor to #146568, this PR actually makes the SIMD intrinsics `const`, and modifies the tests to test the const-eval implementations r? `@tgross35` ig (although feel free to reassign, this is not anything targeted really)
Rollup of 7 pull requests Successful merges: - rust-lang/rust#147171 (recommend using a HashMap if a HashSet's second generic parameter doesn't implement BuildHasher) - rust-lang/rust#147421 (Add check if span is from macro expansion) - rust-lang/rust#147521 (Make SIMD intrinsics available in `const`-contexts) - rust-lang/rust#148201 (Start documenting autodiff activities) - rust-lang/rust#148797 (feat: Add `bit_width` for unsigned `NonZero<T>`) - rust-lang/rust#148798 (Match <OsString as Debug>::fmt to that of str) - rust-lang/rust#149082 (autodiff: update formating, improve examples for the unstable-book) r? `@ghost` `@rustbot` modify labels: rollup
…adsmtm Make SIMD intrinsics available in `const`-contexts successor to rust-lang#146568, this PR actually makes the SIMD intrinsics `const`, and modifies the tests to test the const-eval implementations r? `@tgross35` ig (although feel free to reassign, this is not anything targeted really)
successor to #146568, this PR actually makes the SIMD intrinsics
const, and modifies the tests to test the const-eval implementationsr? @tgross35 ig (although feel free to reassign, this is not anything targeted really)