simd_shuffle: require index argument to be a vector #130268
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
rustbot has assigned @petrochenkov. Use |
rustbot
added
S-waiting-on-review
|
Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 Portable SIMD is developed in its own repository. If possible, consider making this change to rust-lang/portable-simd instead. cc @calebzulawski, @programmerjake The Miri subtree was changed cc @rust-lang/miri Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri Some changes occurred in compiler/rustc_codegen_gcc Some changes occurred to the platform-builtins intrinsics. Make sure the cc @antoyo, @GuillaumeGomez, @bjorn3, @calebzulawski, @programmerjake |
This comment has been minimized.
This comment has been minimized.
RalfJung
force-pushed
the
simd-shuffle-idx-vector
branch
from
8a3d96d
to
eba25b4
Compare
This comment has been minimized.
This comment has been minimized.
RalfJung
force-pushed
the
simd-shuffle-idx-vector
branch
from
eba25b4
to
40b0292
Compare
deltragon
reviewed
Sep 13, 2024
RalfJung
force-pushed
the
simd-shuffle-idx-vector
branch
from
40b0292
to
45872e8
Compare
compiler-errors
approved these changes
Sep 13, 2024
| mask_elements | ||
| }; | ||
| let mask_num_units = mask_elements.len(); | ||
| let vector_type = mask.get_type().dyncast_vector().unwrap(); |
| // third argument must be constant. This is | ||
| // checked by the type-checker. | ||
| if i == 2 && intrinsic.name == sym::simd_shuffle { | ||
| // FIXME: the simd_shuffle argument is actually an array, |
There was a problem hiding this comment.
❤️ glad to see this hack go away lol
|
i left a nit but whatever, probably not worth fixing @bors r+ |
bors
added
S-waiting-on-bors
RalfJung
force-pushed
the
simd-shuffle-idx-vector
branch
from
45872e8
to
5287fa3
Compare
|
Too late, nit is fixed now. ;) @bors r=compiler-errors |
RalfJung
force-pushed
the
simd-shuffle-idx-vector
branch
from
5287fa3
to
7a1b5e5
Compare
|
And another conflict. @bors r=compiler-errors |
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Sep 14, 2024
…r=compiler-errors simd_shuffle: require index argument to be a vector Remove some codegen hacks by forcing the SIMD shuffle `index` argument to be a vector, which means (thanks to rust-lang#128537) that it will automatically be passed as an immediate in LLVM. The only special-casing we still have is for the extra sanity-checks we add that ensure that the indices are all in-bounds. (And the GCC backend needs to do a bunch of work since the Rust intrinsic is modeled after what LLVM expects, which seems to be quite different from what GCC expects.) Fixes rust-lang#128738, see that issue for more context.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 14, 2024
Rollup of 3 pull requests Successful merges: - rust-lang#130053 (fix doc comments for Peekable::next_if(_eq)) - rust-lang#130268 (simd_shuffle: require index argument to be a vector) - rust-lang#130334 (Fix `SDKROOT` ignore on macOS) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 14, 2024
…compiler-errors simd_shuffle: require index argument to be a vector Remove some codegen hacks by forcing the SIMD shuffle `index` argument to be a vector, which means (thanks to rust-lang#128537) that it will automatically be passed as an immediate in LLVM. The only special-casing we still have is for the extra sanity-checks we add that ensure that the indices are all in-bounds. (And the GCC backend needs to do a bunch of work since the Rust intrinsic is modeled after what LLVM expects, which seems to be quite different from what GCC expects.) Fixes rust-lang#128738, see that issue for more context.
This comment has been minimized.
This comment has been minimized.
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
RalfJung
force-pushed
the
simd-shuffle-idx-vector
branch
from
7a1b5e5
to
f7bb2dd
Compare
|
Seems like I have to make sure to not emit an intrinsic when the index is OOB. Makes sense. The last patch should fix that. @compiler-errors does that seem right to you? |
RalfJung
force-pushed
the
simd-shuffle-idx-vector
branch
from
f7bb2dd
to
60ee1b7
Compare
|
Yeah, makes sense. @bors r+ |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 14, 2024
Rollup of 6 pull requests Successful merges: - rust-lang#130017 (coverage: Extract `executor::block_on` from several async coverage tests) - rust-lang#130268 (simd_shuffle: require index argument to be a vector) - rust-lang#130290 (Stabilize entry_insert) - rust-lang#130294 (Lifetime cleanups) - rust-lang#130343 (docs: Enable required feature for 'closure_returning_async_block' lint) - rust-lang#130349 (Fix `Parser::break_up_float`'s right span) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 14, 2024
Rollup merge of rust-lang#130268 - RalfJung:simd-shuffle-idx-vector, r=compiler-errors simd_shuffle: require index argument to be a vector Remove some codegen hacks by forcing the SIMD shuffle `index` argument to be a vector, which means (thanks to rust-lang#128537) that it will automatically be passed as an immediate in LLVM. The only special-casing we still have is for the extra sanity-checks we add that ensure that the indices are all in-bounds. (And the GCC backend needs to do a bunch of work since the Rust intrinsic is modeled after what LLVM expects, which seems to be quite different from what GCC expects.) Fixes rust-lang#128738, see that issue for more context.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove some codegen hacks by forcing the SIMD shuffle
indexargument to be a vector, which means (thanks to #128537) that it will automatically be passed as an immediate in LLVM. The only special-casing we still have is for the extra sanity-checks we add that ensure that the indices are all in-bounds. (And the GCC backend needs to do a bunch of work since the Rust intrinsic is modeled after what LLVM expects, which seems to be quite different from what GCC expects.)Fixes #128738, see that issue for more context.