-
Notifications
You must be signed in to change notification settings - Fork 17
Placeholder accessors can be default constructed #18
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
Open
Ruyk
wants to merge
2
commits into
master
Choose a base branch
from
issue-11
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ | |
| Reply-to | Ruyman Reyes <[email protected]> | | ||
| Original author | Ruyman Reyes <[email protected]> | | ||
| Requirements | CP001 | | ||
| Contributors | Gordon Brown <[email protected]>, Victor Lomuller <[email protected]>, Mehdi Goli <[email protected]>, Peter Zuzek <[email protected]>, Luke Iwanski <[email protected]> | | ||
| Contributors | Gordon Brown <[email protected]>, Victor Lomuller <[email protected]>, Mehdi Goli <[email protected]>, Peter Zuzek <[email protected]>, Luke Iwanski <[email protected]>, Michael Haidl <[email protected]> | | ||
|
||
## Overview | ||
|
||
|
@@ -172,7 +172,10 @@ except for the aforementioned modifications. | |
|
||
### When `is_placeholder` returns true | ||
|
||
The accessor API features constructors that don't take the handler parameter. | ||
The accessor API features constructors that don't take the handler parameter | ||
and/or memory object as a constructor. Accessors can then be default | ||
constructed, and the memory object can be assigned later when registering | ||
the accessor in the command group. | ||
|
||
In addition, a new method to obtain a normal accessor from the placeholder | ||
accessor is provided. | ||
|
@@ -192,6 +195,22 @@ The handler gains a new method, | |
that registers the requirements of the placeholder accessor on the given | ||
command group. | ||
|
||
Another method, that allows specifying the memory object the placeholder | ||
accessor will be associated is also provided: | ||
|
||
`handler::require(buffer<T, dim> b, | ||
Ruyk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
accessor<T, dim, mode, target, access::placeholder::true_t>)` | ||
|
||
### Placeholder `accessor` without a buffer | ||
|
||
If a placeholder accessor which was not constructed with a buffer is not tied | ||
to a buffer within a command group, then an exception is thrown. An accessor | ||
can be checked for the existence of an associated a buffer using `has_buffer()`. | ||
|
||
|Member function |Description | | ||
|-----------------------|---------------------------------------------------------| | ||
|bool has_buffer() const|Returns true if the accessor is associated with a buffer,| | ||
| |and false otherwise. | | ||
|
||
[1]: https://github.com/codeplaysoftware/sycl-blas "SYCL-BLAS" | ||
[2]: https://github.com/lukeiwanski/tensorflow "TensorFlow/Eigen" | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we wish to support the option for constructing a regular accessor from a placeholder accessor (i.e.
get_access
) then this interface should also be extended to allow you to specify a memory object. We should also specify that an exception should be thrown ifget_access
is called without providing a memory object on a placeholder accessor which doesn't have a memory object already.On a side note, perhaps we should consider changing the naming of this member function,
get_access
could be confused with the regularget_access
member functions.