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

Add dtype for numpy.uintp which is compatible with C uintptr_t #1544

Merged
merged 10 commits into from
Mar 25, 2024
Merged

Conversation

kotsaloscv
Copy link
Collaborator

@kotsaloscv kotsaloscv commented Mar 8, 2024

Need this to pass C pointers to DaCe sdfg and reinterpret cast them inside a tasklet

@kotsaloscv kotsaloscv requested a review from alexnick83 March 8, 2024 09:36
@kotsaloscv kotsaloscv self-assigned this Mar 8, 2024
Copy link
Contributor

@alexnick83 alexnick83 left a comment

Choose a reason for hiding this comment

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

Can you add a small test to show example usage and ensure that proper code is generated?

@kotsaloscv kotsaloscv requested a review from tbennun March 11, 2024 14:30
@kotsaloscv kotsaloscv requested a review from tbennun March 12, 2024 08:16
Copy link
Contributor

@alexnick83 alexnick83 left a comment

Choose a reason for hiding this comment

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

Looks good now. Thanks!

@alexnick83 alexnick83 enabled auto-merge March 13, 2024 09:28
@tbennun tbennun disabled auto-merge March 13, 2024 21:58
@kotsaloscv
Copy link
Collaborator Author

@tbennun is it ok to merge?

@tbennun
Copy link
Collaborator

tbennun commented Mar 18, 2024

No. The test is not portable (assumes g++ exists, assumes /tmp exists, assumes Linux) and too elaborate for what it should test. It also leaks 4 bytes. I’ll make a simple test myself and edit your branch if you don’t mind

@kotsaloscv
Copy link
Collaborator Author

No. The test is not portable (assumes g++ exists, assumes /tmp exists, assumes Linux) and too elaborate for what it should test. It also leaks 4 bytes. I’ll make a simple test myself and edit your branch if you don’t mind

more than welcome to suggest another test.

@tbennun
Copy link
Collaborator

tbennun commented Mar 22, 2024

@kotsaloscv see new test

@tbennun tbennun added this pull request to the merge queue Mar 25, 2024
Merged via the queue into master with commit da0cde2 Mar 25, 2024
11 checks passed
@tbennun tbennun deleted the uintp branch March 25, 2024 15:15
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.

3 participants