Skip to content

Conversation

jszuppe
Copy link
Contributor

@jszuppe jszuppe commented May 5, 2016

This PR resolves issues with Boost.Compute running on OSX.

Tests for detail::inplace_reduce, radix_sort, radix_sort_by_key and matrix transpose example are disabled for CPU devices on Apple platform because for CPU devices on Apple platform when local memory is used in a kernel local work group size must be [1;1;1]. Those algorithms and matrix example are designed for GPUs, so from practical perspective we do not lose any functionality.

I've also fixed boost::compute::discrete_distribution constructor. There was a read from uninitialized memory m_probabilities[i-1] (with i = 0).

I've found out that there's a bug in clCreateBuffer() on Apple platform. clCreatBuffer() does not return NULL and does not set error to CL_INVALID_BUFFER_SIZE when size of the buffer memory object is greater than CL_DEVICE_MAX_MEM_ALLOC_SIZE. You can see bug example here - 42949672960 is the size of allocated memory where CL_DEVICE_MAX_MEM_ALLOC_SIZE is only 1073741824 bytes.

After this PR, Travis-CI OSX build is not allowed to fail.

jszuppe added 8 commits May 5, 2016 14:21
Not every device allows for local work group size to be
greater than [1;1;1].
Tests for inplace_reduce, radix_sort and radix_sort_by_key are disabled
for CPU devices on Apple platform because for CPU devices on Apple
platform when local memory is used in a kernel, local work group size
must be [1;1;1]. Those algorithms are designed for GPU devices,
therefore we do no lose any functionality.
This commit adds asserts that check if used template
type is correct (is integral, is a floating point type).
From tests on Travis-CI it seems that on Apple OpenCL platform
clCreatBuffer() does not return NULL and does not set error to
CL_INVALID_BUFFER_SIZE when size of the buffer memory object is
greater than CL_DEVICE_MAX_MEM_ALLOC_SIZE.
@coveralls
Copy link

coveralls commented May 5, 2016

Coverage Status

Coverage decreased (-0.07%) to 88.8% when pulling 13e173c on haahh:pr_osx into dc7bfa0 on boostorg:develop.

: m_n(std::distance(first, last)),
m_probabilities(std::distance(first, last))
: m_n((std::max)(size_t(1), static_cast<size_t>(std::distance(first, last)))),
m_probabilities((std::max)(size_t(1), static_cast<size_t>(std::distance(first, last))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we still need to handle the case where first == last better. Looking at the documentation for std::discrete_distribution, when the input range is empty (or calling the default constructor), we should just initialize with a single probability of 1. But this is currently not implemented, and we don't have to do it in this pull request. I'll open a separate bug for this.

@kylelutz kylelutz merged commit d172c80 into boostorg:develop May 10, 2016
@kylelutz
Copy link
Collaborator

Thanks for all the fixes!

@jszuppe jszuppe deleted the pr_osx branch May 31, 2016 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants