[SYCL] Change return of get_native for opencl backend to vector #4952
Conversation
Change return of get_native<sycl::backend::opencl>(event) from cl_event to vector<cl_event>
sycl/include/CL/sycl/event.hpp
Outdated
| detail::enable_if_t<(BackendName != backend::opencl), | ||
| typename interop<BackendName, event>::type> get_native() | ||
| const { | ||
| return reinterpret_cast<typename interop<BackendName, event>::type>( | ||
| getNative()); | ||
| } | ||
| /// | ||
| template <backend BackendName> | ||
| detail::enable_if_t<(BackendName == backend::opencl), std::vector<cl_event>> | ||
| get_native() const { | ||
| backend_return_t<BackendName, event> ReturnValue; | ||
| for (auto const &element : getNativeVector()) { | ||
| ReturnValue.push_back( | ||
| reinterpret_cast<typename interop<BackendName, event>::value_type>( | ||
| element)); | ||
| } | ||
| return ReturnValue; | ||
| } |
There was a problem hiding this comment.
Why do we need this function at all?
There was a problem hiding this comment.
According to SYCL 2020 specification (revision 3) get_nativesycl::backend::opencl(event) should return vector<cl_event>, whereas for others backends it is not specified. Because C++14 has no "if constexpr", I chose to change SFINAE.
There was a problem hiding this comment.
event::get_native is deprecated. We should not change it in any way.
There was a problem hiding this comment.
At this moment, sycl::get_native free function calls a deprecated function with backend=opencl, so to change the free function we need to work with deprecated.
There was a problem hiding this comment.
I think Alex is right - I'm not even sure where this public function event::get_native() came from.
Regardless, since it's an existing function with different behavior, we should leave it as-is to avoid the breaking change.
I think we need to change the impl of our free function to not use this deprecated method.
There was a problem hiding this comment.
@jbrodman is it ok that with any changes API will break, because backend_return_t for opencl event will return std::vector<cl_event> instead of cl_event?
|
/verify with intel/llvm-test-suite#682 |
|
buildbot/Lit_With_Cuda crash is a known issue (report exists). |
denis-kabanov
requested review from
alexbatashev,
pvchupin,
jbrodman and
cperkinsintel
|
@pvchupin , a friendly ping |
|
Please update PreprocessorMacros.md |
|
@pvchupin, @intel/llvm-reviewers-runtime, can review this pull request? |
Change return of get_nativesycl::backend::opencl(event) from cl_event to vector<cl_event>