Skip to content

[SYCL] Fix set_final_data(..) for vector<bool> iterator #3099

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
romanovvlad merged 9 commits into from
Feb 1, 2021
Merged

[SYCL] Fix set_final_data(..) for vector<bool> iterator #3099

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1c56b1c
[SYCL] Fix set_final_data(..) for vector<bool>
maximdimakov Jan 26, 2021
a988a0b
[SYCL] Clang-format fix
maximdimakov Jan 26, 2021
02dbc6f
[SYCL] Adress review comments
maximdimakov Jan 26, 2021
a353a66
[SYCL] Clang-format fix
maximdimakov Jan 26, 2021
6d5d798
[SYCL] More correct usage of unique_ptr
maximdimakov Jan 26, 2021
ccc2627
[SYCL] Added <memory> lib
maximdimakov Jan 26, 2021
2efc552
Merge branch 'sycl' into fix-set_final_data
maximdimakov Jan 27, 2021
f2a2195
[SYCL] Use unique_ptr constructor instead of make_unique
maximdimakov Jan 27, 2021
d841469
[SYCL] Replace iterator_traits with iterators
maximdimakov Jan 28, 2021
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
25 changes: 24 additions & 1 deletion sycl/include/CL/sycl/detail/sycl_mem_obj_t.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <CL/sycl/stl.hpp>

#include <cstring>
#include <memory>
#include <type_traits>

__SYCL_INLINE_NAMESPACE(cl) {
Expand Down Expand Up @@ -51,7 +52,13 @@ class __SYCL_EXPORT SYCLMemObjT : public SYCLMemObjI {

template <typename T>
using EnableIfOutputIteratorT = enable_if_t<
/*is_output_iterator<T>::value &&*/ !std::is_pointer<T>::value>;
/*is_output_iterator<T>::value &&*/ !std::is_pointer<T>::value &&
!std::is_same<typename T::value_type, bool>::value>;
Copy link
Contributor

@s-kanaev s-kanaev Jan 29, 2021

Choose a reason for hiding this comment

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

@alexbatashev, @sergey-semenov, could you, please, help reviewing this change?

Copy link
Contributor

@alexbatashev alexbatashev Jan 29, 2021

Choose a reason for hiding this comment

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

I'm afraid this is going to break some use cases: https://godbolt.org/z/nznfq1

Copy link
Contributor Author

@maximdimakov maximdimakov Jan 29, 2021

Choose a reason for hiding this comment

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

There is the test SYCL/buffer/buffer.cpp . There are a lot of checks, including pointer (for ex int*, void*). The only compilation error was the error with void*, because there was an iterator_value_type_t and void type was trying to convert to void&. With typename T::value_type all the checks were passed

Copy link
Contributor

@alexbatashev alexbatashev Jan 29, 2021

Choose a reason for hiding this comment

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

Ok, sounds good to me


template <typename T>
using EnableIfOutputIteratorBool =
enable_if_t<!std::is_pointer<T>::value &&
std::is_same<typename T::value_type, bool>::value>;

template <typename T>
using EnableIfDefaultAllocator =
Expand Down Expand Up @@ -184,6 +191,22 @@ class __SYCL_EXPORT SYCLMemObjT : public SYCLMemObjI {
};
}

template <typename Destination>
__SYCL_DLL_LOCAL EnableIfOutputIteratorBool<Destination>
set_final_data(Destination FinalData) {
MUploadDataFunctor = [this, FinalData]() {
using DestinationValueT = iterator_value_type_t<Destination>;
// TODO if Destination is ContiguousIterator then don't create
// ContiguousStorage. updateHostMemory works only with pointer to
// continuous data.
const size_t Size = MSizeInBytes / sizeof(DestinationValueT);
std::unique_ptr<bool[]> ContiguousStorage(new bool[Size]);
updateHostMemory(ContiguousStorage.get());
std::copy(ContiguousStorage.get(), ContiguousStorage.get() + Size,
FinalData);
};
}

protected:
void updateHostMemory(void *const Ptr);

Expand Down