Fix placeholder accessor wrong checking #683
Conversation
Moved placeholder accessor constructor checking from sycl::handler command.
|
I can wait for the other PRs to land first. |
| std::fill(conditions_check, conditions_check + conditions_checks_size, true); | ||
|
|
||
| auto acc = get_accessor_functor(data_buf); | ||
| if constexpr (AccType != accessor_type::host_accessor) { |
There was a problem hiding this comment.
Why is there a difference for iterators between host and device?
There was a problem hiding this comment.
In paragraph 4.7.6.12. Common members for buffer and local accessors I found this:
| iterator begin() const noexcept | Returns an iterator to the first element of the memory this accessor may access.For a buffer accessor this is an iterator to the first element of the underlying buffer, unless this is a ranged accessor in which case it is an iterator to first element within the accessor’s range.For accessor and local_accessor, this function may only be called from within a command. |
|---|
Also in paragraph 4.9.4.1. SYCL functions for adding requirements we have restriction for empty accessors:
| template <typename DataT, int Dimensions, access_mode AccessMode, target AccessTarget, access::placeholder IsPlaceholder> void require( accessor<DataT, Dimensions, AccessMode, AccessTarget, IsPlaceholder> acc) | Requires access to the memory object associated with the accessor.The command group now has a requirement to gain access to the given memory object before executing the kernel. If the accessor has already been registered with the command group, calling this function has no effect.Throws exception with the errc::invalid error code if (acc.empty() == true). |
|---|
So we can't check iteration methods of empty placeholder sycl::accessor and we can check sycl::host_accessor iteration methods.
There was a problem hiding this comment.
Could you summarize this in some comments around the code? We might forget about the rationale... :-)
There was a problem hiding this comment.
@keryell Comments added.
|
@aleksmesh, could you take a look at new conflicts, please? |
|
@bader Conflicts are merged. |
Moved placeholder accessor constructor checking from sycl::handler command.
Cherry-picked PR #669 to avoid CI failures.
To review changes follow this link.