Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Align with SYCL 2020: remove sycl::mode_tag_t #1880

Merged
merged 6 commits into from
Oct 4, 2024

Conversation

dmitriy-sobolev
Copy link
Contributor

@dmitriy-sobolev dmitriy-sobolev commented Oct 4, 2024

sycl::mode_tag_t is not a part of SYCL 2020. It was a part of the provisional specification, but it is absent in the newer revisions.
There is a more radical approach: remove seemingly unused access_mode from sycl_iterator class entirely, but I am not sure about design / ABI implications.

Note, that the host_task tags (such as read_write_host_task) were not added, because they are not currently supported, and there is no sense to do so. The oneDPL specification suggest that the begin and end functions should be provided during code submission to a device, which is not applicable within a host task context:

The begin and end functions can take SYCL 2020 deduction tags and sycl::no_init as arguments to explicitly mention which access mode should be applied to the buffer accessor when submitting a SYCL kernel to a device.

Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
@akukanov
Copy link
Contributor

akukanov commented Oct 4, 2024

Looks reasonable to me, but why not combine two distinct helpers ("converter" and "resolver") into a single one having two template parameters?

There is a more radical approach: remove seemingly unused access_mode from sycl_iterator class entirely, but I am not sure about design / ABI implications.

If we are sure it is not only unused but also unnecessary, the first step would be to just set the parameter always to read_write. Since the type is unspecified, there should be nobody relying on the specific template arguments. Then we would need to change the specification to deprecate the extra parameters of begin()/end(), including no_init, declaring that they have no effect. Then at some point we could change the implementation.

However I am not sure about the tag being unnecessary. Support for C++20 concepts such as indirectly_writable, if we decide to try implementing it, might need this information.

Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
danhoeflinger
danhoeflinger previously approved these changes Oct 4, 2024
Copy link
Contributor

@danhoeflinger danhoeflinger left a comment

Choose a reason for hiding this comment

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

LGTM, happy to reapprove for decay changes if you want to make them.

Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
@dmitriy-sobolev
Copy link
Contributor Author

LGTM, happy to reapprove for decay changes if you want to make them.

Thank you. I would prefer leaving it as is with decay provided only in the specializations. It looks simpler to me compared to the alternatives.

@dmitriy-sobolev dmitriy-sobolev merged commit b9c58ae into main Oct 4, 2024
22 checks passed
@dmitriy-sobolev dmitriy-sobolev deleted the dev/dmitriy-sobolev/sycl-mode-tag branch October 4, 2024 18:31
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants