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] Patch bool dtypepointer load #2815

Closed

Conversation

bgreni
Copy link
Contributor

@bgreni bgreni commented May 24, 2024

This PR offers a temporary fix to address #2813 in case the team cannot currently prioritize this, and I imagine a proper fix requires changes at a deeper level than OS contributors have access to?

@bgreni bgreni requested a review from a team as a code owner May 24, 2024 21:07
@helehex
Copy link
Contributor

helehex commented May 24, 2024

Thanks for patching this

@helehex
Copy link
Contributor

helehex commented May 24, 2024

I suppose there's a chance that the original behavior of packing the bits is desired? but I'm not sure.. at the very least it's inconsistent with itself.
it definitely confused me when i first saw it, but pointers are not meant to be safe. If it is desired, maybe have a separate method to allow packing and unpacking in a way that's consistent with it's offset

@bgreni
Copy link
Contributor Author

bgreni commented May 25, 2024

I suppose there's a chance that the original behavior of packing the bits is desired?

I think that fact that my change works could point to .bitcast[SIMD[DType.bool, width]]().load() not working as intended? Regardless I think this code should work as expected and any special logic required to make it work should be implemented internally. Seems like a pretty major footgun that has no need to exist in a brand new language.

@bgreni bgreni force-pushed the patch-bool-dtypepointer-load branch from 3ad9fca to ec4ed0e Compare May 25, 2024 00:37
@bgreni bgreni force-pushed the patch-bool-dtypepointer-load branch from ec4ed0e to 5e65619 Compare May 26, 2024 03:22
Signed-off-by: Brian Grenier <grenierb96@gmail.com>
@bgreni bgreni force-pushed the patch-bool-dtypepointer-load branch from 5e65619 to e0e40ae Compare May 27, 2024 15:45
@JoeLoser JoeLoser requested a review from abduld May 27, 2024 22:20
@JoeLoser
Copy link
Collaborator

FYI @Dan13llljws in case you notice any issues as I know you're poking in this area

@bgreni bgreni closed this Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants