Skip to content

Clarified the condition of buffer write back. #76

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.

Sign up for GitHub

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
keryell merged 2 commits into from
Mar 24, 2020
Merged

Clarified the condition of buffer write back. #76

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e33b25
Clarified the condition of buffer write back.
bso-intel Mar 6, 2020
66d013b
changed wording to clarify further the final data associated with buf…
bso-intel Mar 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions latex/programming_interface.tex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,7 @@ \subsubsection{Buffer Interface}
ranging from \codeinline{first} up to one before \codeinline{last}.
The data is copied to an intermediate memory position by the runtime.
Data is written back to the same iterator set if the iterator is not
a const iterator.
a const iterator and the member function \codeinline{set_final_data()} is called with a valid non-null pointer.
Copy link
Contributor

@AerialMantis AerialMantis Mar 18, 2020

Choose a reason for hiding this comment

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

This suggests that the data is copied back to the original iterator set if a new destination is set via set_final_data, should this be is not called?

Copy link
Contributor Author

@bso-intel bso-intel Mar 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@AerialMantis
By default, the buffer created from a pair of iterators will not write back to the host memory.
Only when the user calls set_finaL_data(), the buffer is written back to host.
Here is the full sentence after my patch.

Data is written back to the same iterator set if the iterator is not a const iterator and the member function set_final_data() is called with a valid non-null pointer.

Copy link
Member

@keryell keryell Mar 18, 2020

Choose a reason for hiding this comment

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

It depends on the level of detail in the wording document...
Technically

set_final_data(valid);
set_final_data(nullptr);

cannot trigger the copy back even if set_final_data() is called with a valid non-null pointer... :-)

So perhaps it has to be clarified in term of whether the final data it set to something or not instead of detailing calls to set_final_data()`...

Copy link
Contributor Author

@bso-intel bso-intel Mar 19, 2020

Choose a reason for hiding this comment

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

@keryell
Yes, there could be two calls to set_final_data(), first call with a valid pointer and then later change it to nullptr.
In fact, most of buffer constructors in Table 4.31 already mentioned the condition of write-back as follows:

Unless the member function set_final_data() is called with a valid non-null pointer there will be no write back on destruction.

However, the spec of set_final_data() in Table 4.32 is clear enough to clarify your case.

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

Copy link
Contributor

@AerialMantis AerialMantis Mar 19, 2020

Choose a reason for hiding this comment

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

By default, the buffer created from a pair of iterators will not write back to the host memory.

Thanks for the clarification, though this wording doesn't seem quite right to me. It also suggests that the data is copied back to the original iterator the buffer is constructed with only if you specify a new final data iterator. So I would interpret this as:

{
  buffer b(begin(dataA), end(dataA);
  b.set_final_data(begin(dataB));
  /* do something with b */
} // copies back to dataA on destruction

The original wording also suggests that data will be copied back if the iterator set is non-const. So if we want the iterators to be treated as input iterators and not copy back by default, then we should also clarify this such that it's never copied back, unless set_final_data is called with a valid destination.

I agree with Ronan, we should have the wording reflect the state of the buffer rather than a specific call to set_final_data.

I would alter the wording to something like:

Data is not written back to the iterator set provided. 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.

Copy link
Contributor Author

@bso-intel bso-intel Mar 19, 2020

Choose a reason for hiding this comment

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

@AerialMantis
Thanks. I will try to use your wording for this buffer constructor and other buffer constructors.

Copy link
Contributor Author

@bso-intel bso-intel Mar 19, 2020

Choose a reason for hiding this comment

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

@AerialMantis @keryell
I fixed wording as you requested.

Copy link
Contributor

@AerialMantis AerialMantis Mar 24, 2020

Choose a reason for hiding this comment

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

Looks good thanks 👍

The constructed SYCL \codeinline{buffer} will use a default constructed \codeinline{AllocatorT} when allocating memory on the host.
Zero or more properties can be provided to the constructed SYCL \codeinline{buffer} via an instance of \codeinline{property_list}.
}
Expand All @@ -1251,7 +1251,7 @@ \subsubsection{Buffer Interface}
ranging from \codeinline{first} up to one before \codeinline{last}.
The data is copied to an intermediate memory position by the runtime.
Data is written back to the same iterator set if the iterator is not
a const iterator.
a const iterator and the member function \codeinline{set_final_data()} is called with a valid non-null pointer.
The constructed SYCL \codeinline{buffer} will use the \codeinline{allocator} parameter provided when allocating memory on the host.
Zero or more properties can be provided to the constructed SYCL \codeinline{buffer} via an instance of \codeinline{property_list}.
}
Expand Down