Skip to content
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

Direct memory copies from host to device location #1557

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
+54 −40
Draft

Direct memory copies from host to device location #1557

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cf2d7ed
change old part
michael-smirnov Apr 8, 2021
15be47b
change new part
michael-smirnov Apr 8, 2021
9efb5ec
clang-format fix
michael-smirnov Apr 8, 2021
3c1fcd8
Merge branch 'master' into dev/remove-copy-workaround
michael-smirnov May 11, 2021
ef1ce63
Merge branch 'master' into dev/remove-copy-workaround
michael-smirnov Jul 28, 2021
174231d
remove additional usm host arrays
michael-smirnov Jul 28, 2021
a3c7373
remove asserts
michael-smirnov Jul 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
23 changes: 11 additions & 12 deletions cpp/daal/include/services/internal/buffer_impl_sycl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,14 @@ class UsmBuffer : public Base, public UsmBufferIface<T>
}
else if (_allocType == alloc::device)
{
auto host_ptr = SharedPtr<T>(cl::sycl::malloc_host<T>(_size, _queue), // TODO: use daal_malloc
auto host_ptr = SharedPtr<T>((T *)daal_malloc(_size * sizeof(T)),
[q = this->_queue, data = this->_data, size = this->_size, needSynchronize](const void * hostData) mutable {
if (needSynchronize)
{
auto event = q.memcpy(data.get(), hostData, size * sizeof(T));
event.wait_and_throw();
}
cl::sycl::free(const_cast<void *>(hostData), q);
daal_free(const_cast<void *>(hostData));
});
if (!host_ptr)
{
Expand Down Expand Up @@ -335,8 +335,8 @@ class ConvertToUsm : public BufferVisitor<T>
Status makeCopyToUSM(const SharedPtr<T> & hostData, size_t count)
{
Status st;
// TODO: use malloc_device and queue.memcpy()
auto usmData = cl::sycl::malloc_shared<T>(count, _q);

auto usmData = cl::sycl::malloc_device<T>(count, _q);
if (usmData == nullptr)
{
return services::ErrorMemoryAllocationFailed;
Expand All @@ -347,17 +347,16 @@ class ConvertToUsm : public BufferVisitor<T>

if (_rwFlag & data_management::readOnly)
{
int result = daal_memcpy_s(usmData, size, hostData.get(), size);
if (result)
{
return services::ErrorMemoryCopyFailedInternal;
}
st |= internal::sycl::catchSyclExceptions([&, q = this->_q]() mutable {
auto event = q.memcpy(usmData, hostData.get(), size);
event.wait_and_throw();
});
}

_data = SharedPtr<T>(usmData, [q = this->_q, rwFlag = this->_rwFlag, hostData, size](const void * data) mutable {
if (rwFlag & data_management::writeOnly)
_data = SharedPtr<T>(usmData, [q = this->_q, rwFlag = this->_rwFlag, hostData, size, st](const void * data) mutable {
if ((rwFlag & data_management::writeOnly) && st)
{
daal_memcpy_s(hostData.get(), size, data, size);
q.memcpy(hostData.get(), data, size).wait_and_throw();
}
cl::sycl::free(const_cast<void *>(data), q);
});
Expand Down
12 changes: 5 additions & 7 deletions cpp/daal/include/services/internal/sycl/buffer_utils_sycl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,20 +194,18 @@ class ArrayCopier
DAAL_CHECK_STATUS_VAR(status);

{
// TODO: change to use toUSM() and queue.memcpy()
auto dst = sub.toHost(data_management::writeOnly, status);
auto dst = sub.toUSM(queue, data_management::writeOnly, status);
DAAL_CHECK_STATUS_VAR(status);

auto dst_raw = dst.get();

const size_t size = sizeof(T) * count;
DAAL_ASSERT(size >= count);

int result = daal_memcpy_s(dst_raw, size, src, size);
if (result)
{
return services::ErrorMemoryCopyFailedInternal;
}
status |= catchSyclExceptions([&, q = this->queue]() mutable {
auto event = q.memcpy(dst_raw, src, size);
event.wait_and_throw();
});
}
return status;
}
Expand Down
3 changes: 1 addition & 2 deletions cpp/oneapi/dal/algo/svm/backend/utils.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ inline array<Float> convert_labels(sycl::queue& queue,
using error_msg = dal::detail::error_messages;
const std::int64_t count = arr_label.get_count();

// TODO: Replace allocation type once bug with `memcpy` and host memory is fixed
auto new_label_arr = array<Float>::empty(queue, count, sycl::usm::alloc::host);
auto new_label_arr = array<Float>::empty(count);
auto new_label_data = new_label_arr.get_mutable_data();

const auto arr_label_host = dal::backend::to_host_sync(arr_label);
Expand Down
29 changes: 29 additions & 0 deletions cpp/oneapi/dal/backend/memory.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ inline bool is_device_friendly_usm(const sycl::queue& queue, const void* pointer
(pointer_type == sycl::usm::alloc::shared);
}

inline bool is_host_friendly_usm(const sycl::queue& queue, const void* pointer) {
const auto pointer_type = sycl::get_pointer_type(pointer, queue.get_context());
return (pointer_type == sycl::usm::alloc::host) || //
(pointer_type == sycl::usm::alloc::shared);
}

inline bool is_known_usm(const sycl::queue& queue, const void* pointer) {
const auto pointer_type = sycl::get_pointer_type(pointer, queue.get_context());
return pointer_type != sycl::usm::alloc::unknown;
Expand Down Expand Up @@ -238,11 +244,34 @@ inline bool is_device_friendly_usm(const array<T>& ary) {
(pointer_type == sycl::usm::alloc::shared);
}

template <typename T>
inline bool is_host_friendly_usm(const array<T>& ary) {
const auto pointer_type = get_usm_type(ary);
return (pointer_type == sycl::usm::alloc::host) || //
(pointer_type == sycl::usm::alloc::shared);
Comment on lines +289 to +290
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return (pointer_type == sycl::usm::alloc::host) || //
(pointer_type == sycl::usm::alloc::shared);
return (pointer_type == sycl::usm::alloc::host) ||
(pointer_type == sycl::usm::alloc::shared);

}

template <typename T>
inline bool is_known_usm(const array<T>& ary) {
return get_usm_type(ary) != sycl::usm::alloc::unknown;
}

#endif

template <typename T>
using unique_host_ptr = std::unique_ptr<T, detail::default_delete<T, detail::default_host_policy>>;

inline unique_host_ptr<void> make_unique_host(std::int64_t size) {
const detail::default_host_policy host_policy;
return unique_host_ptr<void>{ detail::malloc(host_policy, size),
detail::make_default_delete<void>(host_policy) };
}

template <typename T>
inline unique_host_ptr<T> make_unique_host(std::int64_t count) {
const detail::default_host_policy host_policy;
return unique_host_ptr<T>{ detail::malloc<T>(host_policy, count),
detail::make_default_delete<T>(host_policy) };
}

} // namespace oneapi::dal::backend
6 changes: 2 additions & 4 deletions cpp/oneapi/dal/backend/transfer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,14 @@ template <typename T>
inline std::tuple<array<T>, sycl::event> to_host(const array<T>& ary) {
ONEDAL_ASSERT(ary.get_count() > 0);

if (!ary.get_queue().has_value()) {
if (!ary.get_queue().has_value() || is_host_friendly_usm(ary)) {
return { ary, sycl::event{} };
}

ONEDAL_ASSERT(ary.get_queue().has_value());
auto q = ary.get_queue().value();

// TODO: Change allocation kind to normal host memory once
// bug in `copy` with the host memory is fixed
const auto ary_host = array<T>::empty(q, ary.get_count(), sycl::usm::alloc::host);
const auto ary_host = array<T>::empty(ary.get_count());
const auto event = copy<T>(q, ary_host.get_mutable_data(), ary.get_data(), ary.get_count());
return { ary_host, event };
}
Expand Down
8 changes: 4 additions & 4 deletions cpp/oneapi/dal/table/backend/convert.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ sycl::event convert_vector_device2host(sycl::queue& q,
const std::int64_t src_stride_in_bytes =
dal::detail::check_mul_overflow(element_size_in_bytes, src_stride);

const auto tmp_host_unique = make_unique_usm_host(q, src_size_in_bytes);
const auto tmp_host_unique = make_unique_host(src_size_in_bytes);

auto gather_event = gather_device2host(q,
tmp_host_unique.get(),
Expand All @@ -188,7 +188,7 @@ sycl::event convert_vector_device2host(sycl::queue& q,
deps);
gather_event.wait_and_throw();

convert_vector(dal::detail::default_host_policy{},
convert_vector(detail::default_host_policy{},
tmp_host_unique.get(),
dst_host,
src_type,
Expand Down Expand Up @@ -225,9 +225,9 @@ sycl::event convert_vector_host2device(sycl::queue& q,
const std::int64_t dst_stride_in_bytes =
dal::detail::check_mul_overflow(element_size_in_bytes, dst_stride);

const auto tmp_host_unique = make_unique_usm_host(q, dst_size_in_bytes);
const auto tmp_host_unique = make_unique_host(dst_size_in_bytes);

convert_vector(dal::detail::default_host_policy{},
convert_vector(detail::default_host_policy{},
src_host,
tmp_host_unique.get(),
src_type,
Expand Down