oneDPL icon indicating copy to clipboard operation
oneDPL copied to clipboard

Investigate usage of _ReverseCounter in __pstl_left_bound

Open mmichel11 opened this issue 10 months ago • 1 comments

          Revere counter iterator looks like a little bit strange for me,

As far as I know we already have oneapi::dpl::counting_iterator. Is it not enough? Also when I see the API like _Index __first, _Index __last I assume that the element before __first isn't exist and the element iterated by __last isn't exist too. From this point of view what does it mean __first - 1 below?

Originally posted by @SergeyKopienko in https://github.com/oneapi-src/oneDPL/pull/1446#discussion_r1575039040

We have a _ReverseCounter utility which is used in __pstl_left_bound to run an upper bound operation in reverse. We should be able to achieve a similar effect with oneapi::dpl::counting_iterator. Additionally, it's usage sets the end index to __first - 1 which will likely be problematic if __first is an unsigned type and is passed as 0.

mmichel11 avatar Apr 23 '24 13:04 mmichel11

The following TODO is also listed in the implementation of _ReverseCounter:

// TODO: Temporary hotfix. Investigate the necessity of _ReverseCounter
// Investigate potential user types convertible to integral
// This is the compile-time trick where we define the conversion operator to sycl::id
// conditionally. If we can call accessor::operator[] with the type that converts to the
// same integral type as _ReverseCounter (it means that we can call accessor::operator[]
// with the _ReverseCounter itself) then we don't need conversion operator to sycl::id.
// Otherwise, we define conversion operator to sycl::id.

mmichel11 avatar Apr 23 '24 13:04 mmichel11