Update to official oneDPL histogram implementation#2978
Update to official oneDPL histogram implementation#2978danhoeflinger wants to merge 3 commits intooneapi-src:SYCLomaticfrom
Conversation
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
| d_samples + num_samples, d_histogram, | ||
| num_levels - 1, lower_level, upper_level); | ||
| oneapi::dpl::histogram(::std::forward<Policy>(policy), d_samples, | ||
| d_samples + num_samples, num_levels - 1, |
There was a problem hiding this comment.
Do we need to run clang-format again here?
There was a problem hiding this comment.
mmm, yes. Fixed.
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
timmiesmith
left a comment
There was a problem hiding this comment.
LGTM. Thank you.
The iterator.size() method doesn't make sense, but is likely an old workaround. It is enabling the USM-based device_iterator to be treated as a SYCL buffer backed device_iterator. If the iterator is defined over a buffer you can use it.get_buffer().size() to get the size of the input range. The USM-based device_iterator is returning *this for get_buffer() and thus needed a nonsense size method in order for that usage in an algorithm to compile.
|
CI Windows failures are unrelated to the PR. |
@timmiesmith Do you know where if anywhere this functionality is actually used? Since |
This PR updates the histogram implementation from a temporary feature preview to use the official oneDPL interfaces, and removes the temporary histogram code.
It also removes a
size()function from device iterator which causes problems with some internals of oneDPL separating ranges and iterator implementations. This size function seems strange (size of an iterator?), and only appears in one of thedevice_iteratorimplementations.Technically this is removing part of a public interface of
device_iterator, but I'm not sure how it makes any sense, and makes usage within oneDPL problematic.