Skip to content

[SYCL] Accessor tags, CTAD and host_accessor #1838

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

Merged
merged 18 commits into from
Jun 18, 2020
Merged

Conversation

iburyl
Copy link
Contributor

@iburyl iburyl commented Jun 8, 2020

This patch adds the implementation of SYCL_INTEL_accessor_simplification extension.

It adds:

  • host_accessor
  • read_only, write_only, read_write, read_constant tags
  • noinit property
  • a number of constructors for both buffer accessor and host_accessor to utilize newly added tags and properties for usage simplification

Most use cases for accessor has no longer need for specifying template arguments and rely on deduction guides and corresponding tags.

sycl::access::target is now accessible from sycl::target and sycl::access::mode from sycl::access_mode

@iburyl iburyl requested a review from a team as a code owner June 8, 2020 15:36
@iburyl iburyl requested a review from againull June 8, 2020 15:36
@bader bader added the spec extension All issues/PRs related to extensions specifications label Jun 8, 2020
iburylov added 2 commits June 8, 2020 18:43
Signed-off-by: iburylov <[email protected]>
Signed-off-by: iburylov <[email protected]>
@iburyl
Copy link
Contributor Author

iburyl commented Jun 9, 2020

@mkinsner @jbrodman @rolandschulz
Initial PR on accessors simplification

@iburyl
Copy link
Contributor Author

iburyl commented Jun 9, 2020

My local runs fail two abi tests:

  SYCL :: abi/layout_accessors.cpp
  SYCL :: abi/sycl_symbols_linux.dump

The reason: sycl::access::target was renamed into sycl::target, but using was added for keeping backward compatibility in this place:

namespace access {
using sycl::target;
}

What should I do: change the abi tests or make legacy declaration main one?

@romanovvlad
Copy link
Contributor

My local runs fail two abi tests:

  SYCL :: abi/layout_accessors.cpp
  SYCL :: abi/sycl_symbols_linux.dump

The reason: sycl::access::target was renamed into sycl::target, but using was added for keeping backward compatibility in this place:

namespace access {
using sycl::target;
}

What should I do: change the abi tests or make legacy declaration main one?

I suggest making legacy declaration main one.

@jbrodman
Copy link
Contributor

Do tests need to be rerun or are there outstanding issues?

@andreyfe1
Copy link
Contributor

@iburyl, can we rely on merging this patch till the next Monday?

@jbrodman
Copy link
Contributor

I would like to see this patch committed ASAP.

@iburyl iburyl requested a review from sergey-semenov June 16, 2020 08:48
iburylov added 2 commits June 16, 2020 11:58
@iburyl iburyl requested a review from sergey-semenov June 16, 2020 16:01
Signed-off-by: iburylov <[email protected]>
sergey-semenov
sergey-semenov previously approved these changes Jun 17, 2020
Signed-off-by: iburylov <[email protected]>
sergey-semenov
sergey-semenov previously approved these changes Jun 17, 2020
@bader bader merged commit 1f76efc into intel:sycl Jun 18, 2020
bader pushed a commit that referenced this pull request Jun 25, 2020
…tation (#1943)

This second patch finishes the implementation of SYCL_INTEL_accessor_simplification extension.

It adds:

* `get_host_accessor`  to buffer
* adds overloads for `get_access` to enable deduction guide
* refactored test, as requested in this PR: #1838
* updated deduction guides, based on limitations found via refactored tests
@iburyl iburyl deleted the accessor_tags branch June 25, 2020 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec extension All issues/PRs related to extensions specifications
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants