Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stdlib] SIMD.shuffle with StaticIntTuple mask #2315

Closed
wants to merge 8 commits into from

Conversation

mikowals
Copy link
Contributor

Implement #2293. New overloads for SIMD.shuffle() like this:

fn shuffle[mask: StaticIntTuple[size]](self) -> Self:

additionally:

  • added tests for SIMD.join() and SIMD.shuffle()
  • simplified join implementation and always use shuffle rather than 2 inserts when mask size > 32
  • cleaned up doc strings about shuffle output length
  • minor refactor of original _shuffle_list to use output_size parameter rather than calculate len(mask) many times

As I mentioned in the issue, this can be nicer when inferred parameters are implemented but I guess that is true of most things.

@mikowals mikowals requested a review from a team as a code owner April 17, 2024 02:50
@mikowals mikowals changed the title SIMD.shuffle with StaticIntTuple mask [stdlib] SIMD.shuffle with StaticIntTuple mask Apr 17, 2024
@JoeLoser
Copy link
Collaborator

Thanks for the contribution! 🎉

I'm out of office for the first part of this week (Monday - Wednesday), so I've added a few additional reviewers to this PR in hopes they can take a look before I'm back.

@mikowals mikowals force-pushed the tuple-shuffle branch 6 times, most recently from 12dc3de to fe8ab96 Compare April 26, 2024 09:43
Copy link
Collaborator

@JoeLoser JoeLoser left a comment

Choose a reason for hiding this comment

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

Nice! Just a few quibbles.

Thanks so much for the contribution! 🎉

stdlib/src/builtin/simd.mojo Show resolved Hide resolved
stdlib/test/builtin/test_simd.mojo Outdated Show resolved Hide resolved
Signed-off-by: Michael Kowalski <1331470+mikowals@users.noreply.github.com>
Signed-off-by: Michael Kowalski <1331470+mikowals@users.noreply.github.com>
Signed-off-by: Michael Kowalski <1331470+mikowals@users.noreply.github.com>
Signed-off-by: Michael Kowalski <1331470+mikowals@users.noreply.github.com>
Signed-off-by: Michael Kowalski <1331470+mikowals@users.noreply.github.com>
Signed-off-by: Michael Kowalski <1331470+mikowals@users.noreply.github.com>
Signed-off-by: Michael Kowalski <1331470+mikowals@users.noreply.github.com>
Signed-off-by: Michael Kowalski <1331470+mikowals@users.noreply.github.com>
@JoeLoser JoeLoser added imported-internally Signals that a given pull request has been imported internally. merged-internally Indicates that this pull request has been merged internally labels May 1, 2024
@JoeLoser
Copy link
Collaborator

JoeLoser commented May 1, 2024

✅🟣 This contribution has been merged 🟣✅

Hey @mikowals,

Thanks so much for the contribution! 🎉

We're moving to a new infrastructure for merging contributions to Mojo (we're using a tool called Copybara), and your contribution has now been merged into our internal copy of the Mojo Standard Library. I've added the "merged-internally" label on this PR.

The changes in this PR will appear here in the mojo repo nightly branch when we do our next outbound synchronization at the time that the next Mojo nightly is released. That should happen within the next 24 hours.

Please let me know if you have any questions or concerns.

JoeLoser pushed a commit that referenced this pull request May 3, 2024
[External] [stdlib] SIMD.shuffle with StaticIntTuple mask

Implement #2293.  New overloads for `SIMD.shuffle()` like this:
```
fn shuffle[mask: StaticIntTuple[size]](self) -> Self:
```
additionally:
- added tests for `SIMD.join()` and `SIMD.shuffle()`
- simplified `join` implementation and always use shuffle rather than 2
inserts when mask size > 32
- cleaned up doc strings about shuffle output length
- minor refactor of original `_shuffle_list` to use `output_size`
parameter rather than calculate `len(mask)` many times

As I mentioned in the issue, this can be nicer when inferred parameters
are implemented but I guess that is true of most things.
ORIGINAL_AUTHOR=Michael Kowalski
<1331470+mikowals@users.noreply.github.com>
PUBLIC_PR_LINK=#2315

Co-authored-by: Michael Kowalski <1331470+mikowals@users.noreply.github.com>
Closes #2315
MODULAR_ORIG_COMMIT_REV_ID: db6215999badc9da09f81700ee9b070db8e93a5b
@JoeLoser
Copy link
Collaborator

JoeLoser commented May 3, 2024

🎉 🔥 This landed in tonight's nightly release in #2480!

Thanks again for the contribution!

@JoeLoser JoeLoser closed this May 3, 2024
@JoeLoser JoeLoser added the merged-externally Merged externally in public mojo repo label May 3, 2024
@mikowals mikowals deleted the tuple-shuffle branch May 7, 2024 20:52
lsh pushed a commit to lsh/mojo that referenced this pull request May 17, 2024
[External] [stdlib] SIMD.shuffle with StaticIntTuple mask

Implement modularml#2293.  New overloads for `SIMD.shuffle()` like this:
```
fn shuffle[mask: StaticIntTuple[size]](self) -> Self:
```
additionally:
- added tests for `SIMD.join()` and `SIMD.shuffle()`
- simplified `join` implementation and always use shuffle rather than 2
inserts when mask size > 32
- cleaned up doc strings about shuffle output length
- minor refactor of original `_shuffle_list` to use `output_size`
parameter rather than calculate `len(mask)` many times

As I mentioned in the issue, this can be nicer when inferred parameters
are implemented but I guess that is true of most things.
ORIGINAL_AUTHOR=Michael Kowalski
<1331470+mikowals@users.noreply.github.com>
PUBLIC_PR_LINK=modularml#2315

Co-authored-by: Michael Kowalski <1331470+mikowals@users.noreply.github.com>
Closes modularml#2315
MODULAR_ORIG_COMMIT_REV_ID: db6215999badc9da09f81700ee9b070db8e93a5b

Signed-off-by: Lukas Hermann <lukashermann28@gmail.com>
modularbot added a commit that referenced this pull request Jun 7, 2024
[External] [stdlib] SIMD.shuffle with StaticIntTuple mask

Implement #2293.  New overloads for `SIMD.shuffle()` like this:
```
fn shuffle[mask: StaticIntTuple[size]](self) -> Self:
```
additionally:
- added tests for `SIMD.join()` and `SIMD.shuffle()`
- simplified `join` implementation and always use shuffle rather than 2
inserts when mask size > 32
- cleaned up doc strings about shuffle output length
- minor refactor of original `_shuffle_list` to use `output_size`
parameter rather than calculate `len(mask)` many times

As I mentioned in the issue, this can be nicer when inferred parameters
are implemented but I guess that is true of most things.
ORIGINAL_AUTHOR=Michael Kowalski
<1331470+mikowals@users.noreply.github.com>
PUBLIC_PR_LINK=#2315

Co-authored-by: Michael Kowalski <1331470+mikowals@users.noreply.github.com>
Closes #2315
MODULAR_ORIG_COMMIT_REV_ID: db6215999badc9da09f81700ee9b070db8e93a5b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imported-internally Signals that a given pull request has been imported internally. merged-externally Merged externally in public mojo repo merged-internally Indicates that this pull request has been merged internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants