Skip to content

[SYCL] Add fsycl-id-queries-fit-in-int support for id class #5997

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

Closed
wants to merge 1 commit into from

Conversation

wenju-he
Copy link
Contributor

This patch also removes two __SYCL_ASSUME_INT from item class. They
becomes redundant since MIndex is implemented using id.

This patch intends to fix performance regression of vectorization on cpu device due to change in #5385: when sycl::id type is used, it won't be converted to sycl::item.
sycl::item already has fsycl-id-queries-fit-in-int support support since #1685.

This patch also removes two __SYCL_ASSUME_INT from item class. They
becomes redundant since MIndex is implemented using id.
@wenju-he wenju-he requested a review from a team as a code owner April 11, 2022 04:14
@wenju-he wenju-he requested a review from cperkinsintel April 11, 2022 04:14
@Naghasan
Copy link
Contributor

I don't think this is a valid assumption to add, id and range are user constructible, item, nd_item etc. aren't as they represent one of the underlying id query. I believe that's the reason the assumption wasn't added to id and range.

@wenju-he
Copy link
Contributor Author

@wenju-he
Copy link
Contributor Author

wenju-he commented Apr 14, 2022

This PR has unnecessary impact on other classes like accessor which also uses sycl::id, so I think we might need a specialized sycl::id with fsycl-id-queries-fit-in-int support that is used within parallel_for kernel for querying global_id. The implementation might be complicated.
After some investigation, I choose to revert #5385 and close this PR.
Revert PR: #6013

@wenju-he wenju-he closed this Apr 14, 2022
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.

2 participants