Implement SIMD funnel shifts in const-eval/Miri #147534

sayantn · 2025-10-09T17:55:56Z

Split off from #147520 with just this change for easier review

r? @RalfJung

rustbot · 2025-10-09T17:55:59Z

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

RalfJung

Sorry for the slow review, and thanks!

Please also add tests. All new code should always come with tests. :) Testing this in const-eval is enough since Miri uses the same implementation.

View changes since this review

compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs

sayantn · 2025-11-03T02:57:45Z

We can't test in const-eval yet right? This doesn't make the intrinsics const. So we have to test in Miri

RalfJung · 2025-11-03T07:28:01Z

Oh right, this has the impl in rustc but doesn't expose it yet as that is blocked or more extensive testing... yeah a Miri test also works.

bors · 2025-11-03T20:53:54Z

☔ The latest upstream changes (presumably #148446) made this pull request unmergeable. Please resolve the merge conflicts.

sayantn · 2025-11-03T22:08:43Z

Also @RalfJung , #147521 has been blocked for quite some time. Should I reroll??

rustbot · 2025-11-03T23:10:49Z

The Miri subtree was changed

cc @rust-lang/miri

RalfJung · 2025-11-04T07:01:07Z

Those tests check that we find the UB; please also add tests ensuring we return the correct result (including the corner case shift_bits == 0).

compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs

RalfJung

I edited the comment a little. You didn't talk about the implementation doing << and >> and | at all! You were supposed to explain why those are the right operations to use. Imagine explaining this to someone who has never seen a funnel shift before in their life (i.e., someone like me).

But even with all that, I still don't get it. You basically say "because they live in the lower bits, the implementation is easy". That explains nothing, it just asserts without justification that it is easy. What is the argument for why the bits end up in the right place?

View changes since this review

compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs

bors · 2025-11-05T11:46:48Z

☔ The latest upstream changes (presumably #148507) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk · 2025-11-05T11:59:20Z

Since there's more work happening in this direction, what are your thoughts on adding a to/from array conversion for all simd types via a trait and adding that trait bound to the intrinsic? Then the intrinsic body could be written in library code and work for all backends that do not implement the simd intrinsics (#93145)

RalfJung · 2025-11-05T14:19:27Z

Yeah if we could have fallback impls for the simd_* intrinsics that'd be great.

rustbot · 2025-11-05T21:33:36Z

This PR was rebased onto a different master 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.

src/tools/miri/tests/pass/intrinsics/portable-simd.rs

sayantn · 2025-11-06T00:01:48Z

I have added a lot more explanation, could you check if that is enough?

@rustbot ready

compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs

RalfJung · 2025-11-07T07:39:59Z

Yeah that is the entire patch I think, r=me after squashing and when CI is green.
@bors delegate+

bors · 2025-11-07T07:40:03Z

✌️ @sayantn, you can now approve this pull request!

If @RalfJung told you to "r=me" after making some further change, please make that change, then do @bors r=@RalfJung

sayantn · 2025-11-07T12:04:48Z

Tests pass now

@bors r=RalfJung

bors · 2025-11-07T12:04:50Z

📌 Commit a7aedeb has been approved by RalfJung

It is now in the queue for this repository.

…Jung Implement SIMD funnel shifts in const-eval/Miri Split off from rust-lang#147520 with just this change for easier review r? `@RalfJung`

Rollup of 15 pull requests Successful merges: - #147404 (Fix issue with callsite inline attribute not being applied sometimes.) - #147534 (Implement SIMD funnel shifts in const-eval/Miri) - #147686 (update isolate_highest_one for NonZero<T>) - #148020 (Show backtrace on allocation failures when possible) - #148204 (Modify contributor email entries in .mailmap) - #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks) - #148555 (Fix rust-by-example spanish translation) - #148556 (Fix suggestion for returning async closures) - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability) - #148600 (re-use `self.get_all_attrs` result for pass indirectly attribute) - #148612 (Add note for identifier with attempted hygiene violation) - #148613 (Switch hexagon targets to rust-lld) - #148644 ([bootstrap] Make `--open` option work with `doc src/tools/error_index_generator`) - #148649 (don't completely reset `HeadUsages`) - #148675 (Remove eslint-js from npm dependencies) r? `@ghost` `@rustbot` modify labels: rollup

…Jung Implement SIMD funnel shifts in const-eval/Miri Split off from rust-lang#147520 with just this change for easier review r? ``@RalfJung``

Rollup of 16 pull requests Successful merges: - #147534 (Implement SIMD funnel shifts in const-eval/Miri) - #147686 (update isolate_highest_one for NonZero<T>) - #148020 (Show backtrace on allocation failures when possible) - #148204 (Modify contributor email entries in .mailmap) - #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks) - #148279 (rustc_builtin_macros: rename bench parameter to avoid collisions with user-defined function names) - #148555 (Fix rust-by-example spanish translation) - #148556 (Fix suggestion for returning async closures) - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability) - #148600 (re-use `self.get_all_attrs` result for pass indirectly attribute) - #148612 (Add note for identifier with attempted hygiene violation) - #148613 (Switch hexagon targets to rust-lld) - #148619 (Enable std locking functions on AIX) - #148644 ([bootstrap] Make `--open` option work with `doc src/tools/error_index_generator`) - #148649 (don't completely reset `HeadUsages`) - #148675 (Remove eslint-js from npm dependencies) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 10 pull requests Successful merges: - #145656 (Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro) - #147024 (std_detect: Support run-time detection on OpenBSD using elf_aux_info) - #147534 (Implement SIMD funnel shifts in const-eval/Miri) - #147540 (Stabilise `as_array` in `[_]` and `*const [_]`; stabilise `as_mut_array` in `[_]` and `*mut [_]`.) - #147686 (update isolate_highest_one for NonZero<T>) - #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks) - #148555 (Fix rust-by-example spanish translation) - #148556 (Fix suggestion for returning async closures) - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability) - #148600 (re-use `self.get_all_attrs` result for pass indirectly attribute) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #147534 - sayantn:simd-funnel-shifts, r=RalfJung Implement SIMD funnel shifts in const-eval/Miri Split off from #147520 with just this change for easier review r? ```@RalfJung```

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. labels Oct 9, 2025

rustbot assigned RalfJung Oct 9, 2025

RalfJung reviewed Nov 2, 2025

View reviewed changes

compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs Outdated Show resolved Hide resolved

compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs Outdated Show resolved Hide resolved

compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs Show resolved Hide resolved

sayantn force-pushed the simd-funnel-shifts branch from 3947340 to b3afd22 Compare November 3, 2025 23:10

This comment has been minimized.

Sign in to view

RalfJung reviewed Nov 4, 2025

View reviewed changes

compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs Outdated Show resolved Hide resolved

RalfJung reviewed Nov 4, 2025

View reviewed changes

compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs Outdated Show resolved Hide resolved

RalfJung reviewed Nov 4, 2025

View reviewed changes

compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs Outdated Show resolved Hide resolved

sayantn force-pushed the simd-funnel-shifts branch from b3afd22 to 87bd458 Compare November 5, 2025 21:33

RalfJung reviewed Nov 5, 2025

View reviewed changes

src/tools/miri/tests/pass/intrinsics/portable-simd.rs Outdated Show resolved Hide resolved

RalfJung added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 5, 2025

sayantn force-pushed the simd-funnel-shifts branch 2 times, most recently from b767368 to 50a6df4 Compare November 6, 2025 00:00

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 6, 2025

RalfJung reviewed Nov 6, 2025

View reviewed changes

compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs Outdated Show resolved Hide resolved

sayantn force-pushed the simd-funnel-shifts branch from 50a6df4 to a23f10a Compare November 6, 2025 23:51

implement SIMD funnel shifts in const-eval

a7aedeb

sayantn force-pushed the simd-funnel-shifts branch from c9159c9 to a7aedeb Compare November 7, 2025 10:40

bors 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 Nov 7, 2025

jhpratt mentioned this pull request Nov 8, 2025

Rollup of 15 pull requests #148681

Closed

jhpratt mentioned this pull request Nov 8, 2025

Rollup of 16 pull requests #148686

Closed

matthiaskrgr mentioned this pull request Nov 8, 2025

Rollup of 10 pull requests #148692

Merged

bors merged commit 0f786bd into rust-lang:master Nov 9, 2025
11 checks passed

rustbot added this to the 1.93.0 milestone Nov 9, 2025

Implement SIMD funnel shifts in const-eval/Miri #147534

Implement SIMD funnel shifts in const-eval/Miri #147534

Conversation

sayantn commented Oct 9, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Oct 9, 2025

Uh oh!

RalfJung left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sayantn commented Nov 3, 2025

Uh oh!

RalfJung commented Nov 3, 2025

Uh oh!

bors commented Nov 3, 2025

Uh oh!

sayantn commented Nov 3, 2025

Uh oh!

rustbot commented Nov 3, 2025

Uh oh!

This comment has been minimized.

RalfJung commented Nov 4, 2025

Uh oh!

Uh oh!

Uh oh!

RalfJung left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bors commented Nov 5, 2025

Uh oh!

oli-obk commented Nov 5, 2025

Uh oh!

RalfJung commented Nov 5, 2025

Uh oh!

rustbot commented Nov 5, 2025

Uh oh!

Uh oh!

sayantn commented Nov 6, 2025

Uh oh!

Uh oh!

RalfJung commented Nov 7, 2025

Uh oh!

bors commented Nov 7, 2025

Uh oh!

sayantn commented Nov 7, 2025

Uh oh!

bors commented Nov 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sayantn commented Oct 9, 2025 •

edited by rustbot

Loading

RalfJung left a comment •

edited by rustbot

Loading

RalfJung left a comment •

edited

Loading