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

[SYCLomatic] return device_pointer from data() #1861

Conversation

danhoeflinger
Copy link
Contributor

Fix for data() function return and pointer typedef for device_vector to use device_pointer

I don't know the motivation for originally using T* rather than device_pointer<T>. This passes the tests for vector. It seems that this should be a reasonable change.

@danhoeflinger danhoeflinger requested a review from a team as a code owner April 3, 2024 14:03
Copy link
Contributor

@timmiesmith timmiesmith left a comment

Choose a reason for hiding this comment

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

A small change is needed. The rest looks good to me.

clang/runtime/dpct-rt/include/dpct/dpl_extras/vector.h Outdated Show resolved Hide resolved
@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/data_return_device_ptr branch from 8c8042d to 20a27a6 Compare April 8, 2024 21:34
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
Signed-off-by: Dan Hoeflinger <[email protected]>
@danhoeflinger danhoeflinger force-pushed the dev/dhoeflin/data_return_device_ptr branch from 20a27a6 to b58429b Compare April 15, 2024 17:37
Copy link
Contributor

@timmiesmith timmiesmith left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you.

Copy link
Contributor

@tomflinda tomflinda left a comment

Choose a reason for hiding this comment

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

LGTM

using pointer = T *;
using const_pointer = const T *;
using pointer = device_pointer<T>;
using const_pointer = const device_pointer<T>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should const_pointer be defined as device_pointer<const T>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Fixed, thanks!

Signed-off-by: Dan Hoeflinger <[email protected]>
@zhimingwang36 zhimingwang36 merged commit 7a56e37 into oneapi-src:SYCLomatic Apr 23, 2024
22 checks passed
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.

5 participants