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

Improve wording around buffer mutexes #497

Merged
merged 7 commits into from
Dec 14, 2023

Conversation

mkinsner
Copy link

No description provided.

Co-authored-by: John Pennycook <[email protected]>
Co-authored-by: Greg Lueck <[email protected]>
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.

@gmlueck
Copy link
Contributor

gmlueck commented Dec 6, 2023

This PR needs one more reviewer. @TApplencourt, I see that you left comments. If you are happy with this PR, could you approve?

@TApplencourt
Copy link
Contributor

I still dislike the host allocation. host allocation for me should be reserved for memory allocated via sycl::mallloc_host and not the regular malloc memory.

I think we use hostData in other parts of the spec to refer to memory on the host regardless of how it has been allocated.

IMO it's important to have a distinction between the two. Or maybe just to define what we mean by host allocation somewhere.

But my rant can be considered orthogonal with this issue. We can deal with it another time.

@gmlueck
Copy link
Contributor

gmlueck commented Dec 6, 2023

I still dislike the host allocation. host allocation for me should be reserved for memory allocated via sycl::mallloc_host and not the regular malloc memory.

We should probably use the term "host USM" or "host USM allocation" when we mean memory from sycl::malloc_host. This is a bigger change, though, that is outside of the scope of this PR.

@gmlueck
Copy link
Contributor

gmlueck commented Dec 12, 2023

We agreed in the WG meeting last week that this PR can be merged once @mkinsner makes the wording change that the WG proposed. He has now done this in 01e0449, so I think this is ready to merge.

I'll wait another day for comments. If there are no more comments, I'll merge it.

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 even better clarification.

@gmlueck
Copy link
Contributor

gmlueck commented Dec 14, 2023

Merging as agreed in last WG meeting.

@gmlueck gmlueck merged commit 6ab3d11 into SYCL-2020/master Dec 14, 2023
keryell pushed a commit that referenced this pull request Sep 10, 2024
gmlueck added a commit that referenced this pull request Nov 7, 2024
Improve wording around buffer mutexes

(cherry picked from commit 6ab3d11)
gmlueck added a commit that referenced this pull request Nov 7, 2024
Improve wording around buffer mutexes

(cherry picked from commit 6ab3d11)
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.

5 participants