Type-safe copying #616

jszuppe · 2016-05-26T21:34:52Z

Resolves #363.

Sync. copying to/from device

For both coping data to and from device there are 3 dispatch_copy() functions and each function works in different scenario:

Copying to device when value types (of InputIterator and OutputIterator) match and host iterator is contiguous
Copying to device when value types mismatch and host iterator is contiguous
Copying to device when host iterator is not contiguous (values types does not have to match)

1st option

It's straightforward copying using OpenCL functions for reading and writing a device memory.

2nd option has three sub-options

Maping device memory to host and performing std::copy (casting/converting and copying on host)
Using std::vector<DeviceIterator::value_type> as a intermediate memory (casting/converting on host, copying using OpenCL function)
Maping host memory to the device and using copy kernel to both convert and copy the data.

3rd option has two sub-options

Maping device memory to host and performing std::copy (casting/converting and copying on host)
Using std::vector<DeviceIterator::value_type> as a intermediate memory (casting/converting on host, copying using OpenCL function)

Additional info

When copying to host memory we treat host iterators whose value type is bool as if they were non-contiguous.

Async. copying to/from device

There is asynchronous copying with mismatching types works too. In both cases (to/from device) I used maping host memory to the device and using copy kernel to both convert and copy the data.

Various fixes

Fix types in perf_sort_by_key.cpp
Fix return values in async coping svm_ptr<> from/to device
Fix test in test_functional_as.cpp

When InputIterator (host) is a non-contiguous iterator we don't need a separate algorithm for cases when value_types of InputIterator and OutputIterator (device) do not match and cases when they do match.

Type-safe copying from device to host. Seperate copying algorithm device -> host for non-contiguous OutputIterator (host).

At the end of test we should read from input vector (not output) in order to check if transform() with as<int>() was performed correctly.

coveralls · 2016-05-26T22:55:59Z

Coverage increased (+0.3%) to 80.828% when pulling c3f32be on haahh:pr_typesafe_copy into d41ff00 on boostorg:develop.

This commit modifies svm_ptr<T> to keep its context. It is convenient for the users and enables creating svm_ptr_index_expr<T, IndexExpr> class.

Tests for copying SVM memory to/from/on device when value_types of InputIterator and OutputIterator mismatch.

kylelutz · 2016-05-28T16:24:36Z

example/opencv_histogram.cpp

-#define BOOST_COMPUTE_DEBUG_KERNEL_COMPILATION
+#ifndef BOOST_COMPUTE_DEBUG_KERNEL_COMPILATION
+    #define BOOST_COMPUTE_DEBUG_KERNEL_COMPILATION
+#endif


We should probably just remove this define from the file. It should only really be defined by the user when they are debugging.

Will fix that later today.

coveralls · 2016-05-28T16:41:13Z

Coverage increased (+0.3%) to 80.857% when pulling 69e09f2 on haahh:pr_typesafe_copy into d41ff00 on boostorg:develop.

kylelutz · 2016-05-28T16:41:14Z

include/boost/compute/algorithm/copy.hpp

+        make_buffer_iterator<output_type>(mapped_host),
+        queue
+    );
+    // update host memory asynchronously by maping and unmaping memory


Shouldn't asynchronously here be synchronously? Also, I think an alternative would be instead of mapping and unmapping the buffer, replace the call above with copy_on_device_async() and wait on the returned event/future.

Yes, indeed, it should be synchronously since later wait() is called for event returned by unmap operation.

And yes, those lines can be replace by just calling copy_on_device_async() as it does the same (create buffer for mapping host memory, perform copying on device and map/unmap buffer).

Fix: dispatch_copy_async(), not copy_on_device_async(). We have to map and unmap at the end to force OpenCL driver to update host memory (mapping is a synchronization point).

kylelutz · 2016-05-28T16:59:41Z

Looks good! Left a few comments. I'll finish up reviewing/testing this out today.

jszuppe · 2016-05-28T19:42:41Z

Thanks! Just fyi, I ran all the tests with SVM on Windows with the latest AMD drivers with fixed clEnqueueSVMMemcpy() (you can see that there are tests with svm_ptr<> that are skipped for AMD, this is the reason).

jszuppe · 2016-05-28T20:30:11Z

I've just found that that there is an inconsistency in what async. copy functions return when first == last. In one function it's future object with result iterator and an empty event (event()), in other functions it's empty future object... However, from user perspective it does not matter because it's impossible to get the value in both of those cases (user gets Invalid event error or sth like this when try to get() the value).

coveralls · 2016-05-28T22:24:10Z

Coverage increased (+0.3%) to 80.861% when pulling 8f4db3d on haahh:pr_typesafe_copy into d41ff00 on boostorg:develop.

kylelutz · 2016-06-01T04:26:42Z

Awesome! Merged!

jszuppe added 10 commits May 21, 2016 16:44

Fix type-safety issues when copying host->device

a830531

Add tests for copying host->device with mismatched types

bba6c61

Fix type, long_ and long might be different types

3493b75

Fix type-safety issues for async copying host->device

d12e07c

Add tests for coping on device with mismatched types

7c3d0c2

Coping from host for non-contiguous iterators (host)

ce8230b

When InputIterator (host) is a non-contiguous iterator we don't need a separate algorithm for cases when value_types of InputIterator and OutputIterator (device) do not match and cases when they do match.

Type-safe copying from device to host

1419b7d

Type-safe copying from device to host. Seperate copying algorithm device -> host for non-contiguous OutputIterator (host).

Make sure there's no macro redefinition

2187821

Fix test in test_functional_as.cpp

55ac7da

At the end of test we should read from input vector (not output) in order to check if transform() with as<int>() was performed correctly.

Add test for async coping with svm_ptr<>

7461c92

jszuppe added 8 commits May 28, 2016 13:15

Fix async coping svm_ptr<> from/to/on device

3dcbd4c

Type-safe async coping from device to host

aaaaca4

Better names for tests

933ff15

Now svm_ptr<T> keeps its context

4280024

This commit modifies svm_ptr<T> to keep its context. It is convenient for the users and enables creating svm_ptr_index_expr<T, IndexExpr> class.

Support for svm_ptr<T> in meta_kernel

753f883

Coping SVM to/from host by mapping SVM memory

5c29b50

Tests for copying SVM memory when types mismatch

ba0da3f

Tests for copying SVM memory to/from/on device when value_types of InputIterator and OutputIterator mismatch.

Minor fixes in test_copy.cpp

69e09f2

jszuppe force-pushed the pr_typesafe_copy branch from c3f32be to 69e09f2 Compare May 28, 2016 14:47

kylelutz reviewed May 28, 2016
View reviewed changes

Remove debug macro, fix names of event variables

c9a0aba

Fix async copying when first == last (input is empty)

bffc9f7

Reuse dispatch_copy_async() in dispatch_copy()

8f4db3d

kylelutz merged commit 4c31d07 into boostorg:develop Jun 1, 2016

kylelutz mentioned this pull request Jun 1, 2016

Fix type-safety issues when copying to/from host-memory #363

Closed

jszuppe mentioned this pull request Jun 1, 2016

compute::copy returns incorrect values when template parameter mismatchs #600

Closed

jszuppe deleted the pr_typesafe_copy branch June 1, 2016 16:37

Type-safe copying #616

Type-safe copying #616

Uh oh!

Conversation

jszuppe commented May 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sync. copying to/from device

1st option

2nd option has three sub-options

3rd option has two sub-options

Additional info

Async. copying to/from device

Various fixes

Uh oh!

coveralls commented May 26, 2016

Uh oh!

kylelutz May 28, 2016

Choose a reason for hiding this comment

Uh oh!

jszuppe May 28, 2016

Choose a reason for hiding this comment

Uh oh!

coveralls commented May 28, 2016

Uh oh!

kylelutz May 28, 2016

Choose a reason for hiding this comment

Uh oh!

jszuppe May 28, 2016

Choose a reason for hiding this comment

Uh oh!

jszuppe May 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kylelutz commented May 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jszuppe commented May 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jszuppe commented May 28, 2016

Uh oh!

coveralls commented May 28, 2016

Uh oh!

kylelutz commented Jun 1, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jszuppe commented May 26, 2016 •

edited

Loading

jszuppe May 28, 2016 •

edited

Loading

kylelutz commented May 28, 2016 •

edited

Loading

jszuppe commented May 28, 2016 •

edited

Loading