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

Update sycl::vec class to reflect SYCL 2020 requirements #907

Merged
merged 5 commits into from
Feb 9, 2023
Merged

Update sycl::vec class to reflect SYCL 2020 requirements #907

merged 5 commits into from
Feb 9, 2023

Conversation

nilsfriess
Copy link
Collaborator

This PR introduces the following small changes in the sycl::vec class to reflect the SYCL 2020 spec:

  • A size method is added which replaces the get_count method (which is now marked deprecated)
  • A byte_size method is added which replaces the get_size (which is now marked deprecated)
  • The signature of the as method is changed: according to Table 144 of the SYCL 2020 spec, the template parameter asT should refer to the whole vector type that the vec is reinterpreted as. Previously, asT referred to the type of the new vec's elements.

These all came up when trying to compile the CTS vec_api test; it is still not compiling, but the remaining errors seem to be be CTS-related, not hipSYCL-related.

The `get_size` member is deprecated and replaced by `byte_size`
According to the spec (Table 144), the method `sycl::vec::as` is
supposed to have the signature `template <typename asT> asT as()
const`. As can be seen from the return type, the template type
`asT` should therefore be the whole vec type, not just the type
of its elements.
@illuhad
Copy link
Collaborator

illuhad commented Jan 13, 2023

Thanks! Have you made a survey whether we use those deprecated functions somewhere internally in hipSYCL? E.g. I strongly suspect that get_count() is used by the builtin implementations to iterate over vec elements. It would be unfortunate if users get a deprecated-warning triggered by our own headers :-)

@nilsfriess
Copy link
Collaborator Author

nilsfriess commented Jan 17, 2023

No, I didn't really think about that...
I've now replaced get_count by size in the builtins and the tests that use it.

Actually, there is one function in the group function tests that still uses it but this function isn't actually used anywhere so maybe this function can just be deleted anyway?

@illuhad
Copy link
Collaborator

illuhad commented Jan 18, 2023

@DieGoldeneEnte can you comment on the function in the group function tests?

@illuhad illuhad merged commit b12e677 into AdaptiveCpp:develop Feb 9, 2023
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

2 participants