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

Clarify access::target::host_task #537

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

hdelan
Copy link
Contributor

@hdelan hdelan commented Mar 4, 2024

An accessor with access::target::host_task is accessible on the host from within the host task lambda.

Specify that access::target::host_task means not only 'accessible within host_task' but 'accessible within host_task from the host'. This is necessary as host_task defines a scope, but it doesn't define where the memory is accessible from.

@hdelan hdelan force-pushed the host-task-accessor branch from c595226 to 4234c45 Compare March 4, 2024 18:00
Copy link
Member

@keryell keryell left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification.

hdelan added a commit to hdelan/SYCL-Docs that referenced this pull request Mar 19, 2024
This reflects the restructure that @gmlueck suggested in
KhronosGroup#537.
hdelan added a commit to hdelan/SYCL-Docs that referenced this pull request Mar 19, 2024
This reflects the restructure that @gmlueck suggested in
KhronosGroup#537.
@hdelan hdelan force-pushed the host-task-accessor branch from 0a7190b to c4df016 Compare March 19, 2024 17:21
@hdelan hdelan force-pushed the host-task-accessor branch from a011e40 to 1c02843 Compare March 20, 2024 12:12
@hdelan
Copy link
Contributor Author

hdelan commented Mar 20, 2024

Thanks @gmlueck . Would you be able to merge this?

@gmlueck gmlueck added the Agenda To be discussed during a SYCL committee meeting label Mar 20, 2024
@gmlueck
Copy link
Contributor

gmlueck commented Mar 20, 2024

I only have unilateral authority to merge editorial changes. I added the "agenda" label, so we can discuss this at the next WG meeting.

@hdelan
Copy link
Contributor Author

hdelan commented Mar 20, 2024

Ok sure thanks

An accessor with access::target::host_task is accessible on
the host from within the host task lambda. It is necessary
to specify that no get_native_mem method is needed and also
that target::host_task means not only 'accessible within host_task'
but 'accessible within host_task on the host'. This is necessary
as host_task defines a scope, but it doesn't define where the
memory is accessible from.

It also adds reference to deprecated targets for accessors:
target::local and target::constant_buffer for accessors.
@hdelan hdelan force-pushed the host-task-accessor branch from b98911b to 75162eb Compare March 22, 2024 12:08
Copy link
Member

@keryell keryell left a comment

Choose a reason for hiding this comment

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

Thanks.

@tomdeakin tomdeakin removed the Agenda To be discussed during a SYCL committee meeting label Mar 25, 2024
@hdelan
Copy link
Contributor Author

hdelan commented Mar 28, 2024

@gmlueck is this OK to merge now?

@gmlueck
Copy link
Contributor

gmlueck commented Mar 28, 2024

Merge approvals normally happen during a WG call. Since there are now 2 organization approvals on this PR, I would expect it to be approved in next week's call.

Is there a reason you want this merged sooner? If so, we could ask for an exception to the process (but no guarantees).

@hdelan
Copy link
Contributor Author

hdelan commented Mar 28, 2024

Merge approvals normally happen during a WG call. Since there are now 2 organization approvals on this PR, I would expect it to be approved in next week's call.

Thanks for info. I didn't know this process. Just out of curiosity - how many organizations need to approve a patch in order for a SYCL spec change to be made?

Is there a reason you want this merged sooner? If so, we could ask for an exception to the process (but no guarantees).

No reason at all. Happy to wait for more approvals.

@gmlueck
Copy link
Contributor

gmlueck commented Mar 28, 2024

Just out of curiosity - how many organizations need to approve a patch in order for a SYCL spec change to be made?

For changes to the SYCL spec, two organizations must approve, which are both different from the organization that authored the patch.

For changes to the CTS, we decided that only one organization (different from the author) is required.

@hdelan
Copy link
Contributor Author

hdelan commented Mar 28, 2024

Thanks for info

@tomdeakin tomdeakin added the Agenda To be discussed during a SYCL committee meeting label Mar 28, 2024
@tomdeakin
Copy link
Contributor

WG approved to merge.

@gmlueck gmlueck merged commit 01b97ec into KhronosGroup:SYCL-2020/master Apr 4, 2024
1 check passed
keryell pushed a commit that referenced this pull request Sep 10, 2024
Clarify access::target::host_task
gmlueck added a commit that referenced this pull request Nov 7, 2024
Clarify access::target::host_task

(cherry picked from commit 01b97ec)
gmlueck added a commit that referenced this pull request Nov 7, 2024
Clarify access::target::host_task

(cherry picked from commit 01b97ec)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agenda To be discussed during a SYCL committee meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants