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

Description inconsistencies around buffer constructors and buffer::set_final_data() #535

Open
AlexeySachkov opened this issue Feb 26, 2024 · 2 comments

Comments

@AlexeySachkov
Copy link
Contributor

buffer::set_final_data is defined as follows (emphasis mine):

template <typename Destination = std::nullptr_t> void set_final_data(Destination finalData = nullptr)

The finalData points to where the outcome of all the buffer processing is going to be copied to at destruction time, if the buffer was involved with a write accessor.

Destination can be either an output iterator or a std::weak_ptr<T>.

Note that a raw pointer is a special case of output iterator and thus defines the host memory to which the result is to be copied.

In the case of a weak pointer, the output is not updated if the weak pointer has expired.

If Destination is std::nullptr_t, then the copy back will not happen.

It is referenced from a number of constructors, but differently every time (emphasis mine):

Data is not written back to the host on destruction of the buffer unless the buffer has a valid non-null pointer specified via the member function set_final_data().

Since the hostData is const, this buffer is only initialized with this memory and there is no write back after its destruction, unless the buffer has another valid non-null final data address specified via the member function set_final_data() after construction of the buffer.

However, if the buffer has a valid non-const iterator specified via the member function set_final_data(), data will be copied back to that iterator.

I think it would be better if set_final_data() was referenced uniformly from all constructors. It also doesn't make sense to account for const iterators, because they are not output iterators and therefore can't be passed to set_final_data() at all.

@psalz
Copy link
Contributor

psalz commented Feb 28, 2024

Somewhat related CTS issue: KhronosGroup/SYCL-CTS#877

@keryell
Copy link
Member

keryell commented Feb 29, 2024

It also doesn't make sense to account for const iterators, because they are not output iterators and therefore can't be passed to set_final_data() at all.

The idea was to emphasize the fact that even if you have a buffer made from a const iterator, you can still write back, by using a non-const iterator, by opposition.

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

No branches or pull requests

3 participants