Skip to content

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

Merged
bors merged 1 commit into
rust-lang:masterfrom
sayantn:simd-funnel-shifts
Nov 9, 2025
Merged

Implement SIMD funnel shifts in const-eval/Miri#147534
bors merged 1 commit into
rust-lang:masterfrom
sayantn:simd-funnel-shifts

Conversation

@sayantn

@sayantn sayantn commented Oct 9, 2025 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

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

r? @RalfJung

@rustbot

rustbot commented Oct 9, 2025

Copy link
Copy Markdown
Collaborator

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

@rustbot 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
RalfJung
RalfJung reviewed Nov 2, 2025
View reviewed changes

@RalfJung RalfJung left a comment •
edited by rustbot
Loading

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.

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

Comment thread compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs Outdated
Comment thread compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs Outdated
Comment thread compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs
@sayantn

sayantn commented Nov 3, 2025

Copy link
Copy Markdown
Contributor Author

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

@RalfJung

RalfJung commented Nov 3, 2025

Copy link
Copy Markdown
Member

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

bors commented Nov 3, 2025

Copy link
Copy Markdown
Collaborator

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

@sayantn

sayantn commented Nov 3, 2025

Copy link
Copy Markdown
Contributor Author

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

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

rustbot commented Nov 3, 2025

Copy link
Copy Markdown
Collaborator

The Miri subtree was changed

cc @rust-lang/miri

@rustbot

This comment has been minimized.

Sign in to view
@RalfJung

RalfJung commented Nov 4, 2025

Copy link
Copy Markdown
Member

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).

RalfJung
RalfJung reviewed Nov 4, 2025
View reviewed changes
Comment thread compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs Outdated
RalfJung
RalfJung reviewed Nov 4, 2025
View reviewed changes
Comment thread compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs Outdated
RalfJung
RalfJung reviewed Nov 4, 2025
View reviewed changes

@RalfJung RalfJung left a comment •
edited
Loading

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.

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

Comment thread compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs Outdated
@bors

bors commented Nov 5, 2025

Copy link
Copy Markdown
Collaborator

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

@oli-obk

oli-obk commented Nov 5, 2025

Copy link
Copy Markdown
Contributor

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

RalfJung commented Nov 5, 2025

Copy link
Copy Markdown
Member

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

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

rustbot commented Nov 5, 2025

Copy link
Copy Markdown
Collaborator

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.

RalfJung
RalfJung reviewed Nov 5, 2025
View reviewed changes
Comment thread src/tools/miri/tests/pass/intrinsics/portable-simd.rs Outdated
@RalfJung 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 sayantn force-pushed the simd-funnel-shifts branch 2 times, most recently from b767368 to 50a6df4 Compare November 6, 2025 00:00
@sayantn

sayantn commented Nov 6, 2025

Copy link
Copy Markdown
Contributor Author

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

@rustbot ready

@rustbot 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
RalfJung reviewed Nov 6, 2025
View reviewed changes
Comment thread compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs Outdated
@sayantn sayantn force-pushed the simd-funnel-shifts branch from 50a6df4 to a23f10a Compare November 6, 2025 23:51
@RalfJung

RalfJung commented Nov 7, 2025

Copy link
Copy Markdown
Member

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

@bors

bors commented Nov 7, 2025

Copy link
Copy Markdown
Collaborator

✌️ @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 sayantn force-pushed the simd-funnel-shifts branch from c9159c9 to a7aedeb Compare November 7, 2025 10:40
@sayantn

sayantn commented Nov 7, 2025

Copy link
Copy Markdown
Contributor Author

Tests pass now

@bors r=RalfJung

@bors

bors commented Nov 7, 2025

Copy link
Copy Markdown
Collaborator

📌 Commit a7aedeb has been approved by RalfJung

It is now in the queue for this repository.

@bors 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 added a commit to jhpratt/rust that referenced this pull request Nov 8, 2025
@jhpratt
Rollup merge of rust-lang#147534 - sayantn:simd-funnel-shifts, r=Ralf…
67c0a91 
…Jung

Implement SIMD funnel shifts in const-eval/Miri

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

r? `@RalfJung`
@jhpratt jhpratt mentioned this pull request Nov 8, 2025
Closed
bors added a commit that referenced this pull request Nov 8, 2025
@bors
Auto merge of #148681 - jhpratt:rollup-t14sr2j, r=jhpratt
b99666a 
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
jhpratt added a commit to jhpratt/rust that referenced this pull request Nov 8, 2025
@jhpratt
Rollup merge of rust-lang#147534 - sayantn:simd-funnel-shifts, r=Ralf…
7c37f3a 
…Jung

Implement SIMD funnel shifts in const-eval/Miri

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

r? ``@RalfJung``
@jhpratt jhpratt mentioned this pull request Nov 8, 2025
Closed
bors added a commit that referenced this pull request Nov 8, 2025
@bors
Auto merge of #148686 - jhpratt:rollup-jlh9fj9, r=jhpratt
939844a 
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
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 8, 2025
Merged
bors added a commit that referenced this pull request Nov 8, 2025
@bors
Auto merge of #148692 - matthiaskrgr:rollup-hryk71f, r=matthiaskrgr
e3cb0e3 
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
bors added a commit that referenced this pull request Nov 8, 2025
@bors
Auto merge of #148692 - matthiaskrgr:rollup-hryk71f, r=matthiaskrgr
328779c 
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
bors added a commit that referenced this pull request Nov 9, 2025
@bors
Auto merge of #148692 - matthiaskrgr:rollup-hryk71f, r=matthiaskrgr
acda5e9 
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
@bors bors merged commit 0f786bd into rust-lang:master Nov 9, 2025
11 checks passed
@rustbot rustbot added this to the 1.93.0 milestone Nov 9, 2025
rust-timer added a commit that referenced this pull request Nov 9, 2025
@rust-timer
Unrolled build for #147534
7b97b31 
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```
github-actions Bot pushed a commit to rust-lang/stdarch that referenced this pull request Nov 10, 2025
@bors
Auto merge of #148692 - matthiaskrgr:rollup-hryk71f, r=matthiaskrgr
925a587 
Rollup of 10 pull requests

Successful merges:

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

r? `@ghost`
`@rustbot` modify labels: rollup
github-actions Bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Nov 10, 2025
@bors
Auto merge of #148692 - matthiaskrgr:rollup-hryk71f, r=matthiaskrgr
b3e6804 
Rollup of 10 pull requests

Successful merges:

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

r? `@ghost`
`@rustbot` modify labels: rollup
github-actions Bot pushed a commit to rust-lang/miri that referenced this pull request Nov 10, 2025
@bors
Auto merge of #148692 - matthiaskrgr:rollup-hryk71f, r=matthiaskrgr
d8a1511 
Rollup of 10 pull requests

Successful merges:

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

r? `@ghost`
`@rustbot` modify labels: rollup
@sayantn sayantn deleted the simd-funnel-shifts branch November 17, 2025 22:41
Kobzol pushed a commit to Kobzol/rustc_codegen_cranelift that referenced this pull request Dec 29, 2025
@bors
Auto merge of #148692 - matthiaskrgr:rollup-hryk71f, r=matthiaskrgr
129f5f1 
Rollup of 10 pull requests

Successful merges:

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

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RalfJung RalfJung RalfJung left review comments

Assignees

@RalfJung RalfJung

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.93.0

Development

Successfully merging this pull request may close these issues.

5 participants

@sayantn @rustbot @RalfJung @bors @oli-obk