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] Clean up memory.unsafe #3588

Closed
wants to merge 1 commit into from

Conversation

soraros
Copy link
Contributor

@soraros soraros commented Oct 1, 2024

  • Make more things infer-only
  • Remove unnecessary overload
  • Rename the bitcast overload that performs the 'movemask' operation to pack_mask. This change is intended to prevent issues where the wrong overload might be selected, and since there is an implicit conversion from scalar to simd at return, the user won't get a type mismatch error to warn them about that. @JoeLoser, do you think this change is a good idea? The function in NumPy is called packbits, maybe that's a better name.

@soraros soraros force-pushed the cleanup-bitcast branch 2 times, most recently from 0cbc317 to 130184e Compare October 1, 2024 20:50
@soraros soraros marked this pull request as ready for review October 1, 2024 21:03
@soraros soraros requested a review from a team as a code owner October 1, 2024 21:03
@soraros soraros force-pushed the cleanup-bitcast branch 5 times, most recently from 7509649 to 282ea3c Compare October 1, 2024 22:56
stdlib/src/memory/unsafe.mojo Outdated Show resolved Hide resolved
@JoeLoser
Copy link
Collaborator

JoeLoser commented Oct 7, 2024

!sync

@JoeLoser
Copy link
Collaborator

JoeLoser commented Oct 9, 2024

!sync

@modularbot modularbot added the imported-internally Signals that a given pull request has been imported internally. label Oct 9, 2024
@soraros soraros force-pushed the cleanup-bitcast branch 3 times, most recently from 3dfc9db to e2aebab Compare October 14, 2024 21:16
@soraros soraros requested a review from JoeLoser October 14, 2024 21:16
@JoeLoser
Copy link
Collaborator

!sync

- Make more things infer-only
- Remove unnecessary overload
- Rename `bitcast` overload that performs "movemask" to `pack_bits`

Signed-off-by: Yiwu Chen <210at85@gmail.com>
@JoeLoser
Copy link
Collaborator

!sync

@modularbot
Copy link
Collaborator

✅🟣 This contribution has been merged 🟣✅

Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the nightly branch during the next Mojo nightly release, typically within the next 24-48 hours.

We use Copybara to merge external contributions, click here to learn more.

@modularbot modularbot added the merged-internally Indicates that this pull request has been merged internally label Nov 1, 2024
@soraros
Copy link
Contributor Author

soraros commented Nov 1, 2024

@JoeLoser Do we need to add changelog entries for this and perhaps the to_bits changes in SIMD? The changes might be too trivial to warrant it, but people could encounter issues with the _floats_to_bits name change.

@JoeLoser
Copy link
Collaborator

JoeLoser commented Nov 1, 2024

@JoeLoser Do we need to add changelog entries for this and perhaps the to_bits changes in SIMD? The changes might be too trivial to warrant it, but people could encounter issues with the _floats_to_bits name change.

Wouldn't hurt, I agree. Want to write something up? If not, I can next week.

@soraros
Copy link
Contributor Author

soraros commented Nov 1, 2024

@JoeLoser I guess it's the best if you could do is since both PRs are now merged. Thanks in advance!

modularbot pushed a commit that referenced this pull request Nov 2, 2024
[External] [stdlib] Clean up `memory.unsafe`

- Make more things infer-only
- Remove unnecessary overload
- Rename the `bitcast` overload that performs the 'movemask' operation
to `pack_mask`. This change is intended to prevent issues where the
wrong overload might be selected, and since there is an implicit
conversion from scalar to simd at return, the user won't get a type
mismatch error to warn them about that.

Co-authored-by: soraros <soraros@users.noreply.github.com>
Closes #3588
MODULAR_ORIG_COMMIT_REV_ID: 18e72968cbdadf593e6bc9bba0794e326bada362
@modularbot modularbot added the merged-externally Merged externally in public mojo repo label Nov 2, 2024
@modularbot
Copy link
Collaborator

Landed in bb4a60c! Thank you for your contribution 🎉

@modularbot modularbot closed this Nov 2, 2024
@soraros soraros deleted the cleanup-bitcast branch November 2, 2024 11:54
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.

3 participants