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.

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

[cl] Permit size 0 in clEnqueue(Read|Write|Copy|Fill)Buffer #387

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

frasercrmck
Copy link
Contributor

@frasercrmck frasercrmck commented Mar 4, 2024

This is not explicitly forbidden by the current unified OpenCL specification for clEnqueueReadBuffer, clEnqueueWriteBuffer, or clEnqueueCopyBuffer; though it was forbidden up to and including OpenCL 2.0.

Therefore, make the checks for size 0 dependent on the OpenCL version being built.

A size of zero for clEnqueueFillBuffer has always been valid, so explicitly test that. The muxCommandFillBuffer function forbids a size of 0, so it was a bug to call that function when size was 0.

This is not explicitly forbidden by the current unified OpenCL
specification for clEnqueueReadBuffer, clEnqueueWriteBuffer, or
clEnqueueCopyBuffer; though it *was* forbidden up to and including
OpenCL 2.0.

Therefore, make the checks for size 0 dependent on the OpenCL version
being built.

A size of zero for clEnqueueFillBuffer has always been valid, so
explicitly test that. The muxCommandFillBuffer function forbids a size
of 0, so it was a bug to call that function when size was 0.

This requires updating the mux specification to allow sizes of 0 in all
of these functions. It also implicitly allows the same in the HAL,
though this is not as tightly specified.
inBuffer, 0, nullptr, nullptr));
TEST_F(clEnqueueReadBufferTest, SizeZero) {
// An error when size == 0 was removed starting with OpenCL 2.1.
if (UCL::isDeviceVersionAtLeast({2, 1})) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we test OpenCL <3.0 anywhere?

}
if (!mem_write(dst, temp.data(), size, locker)) {
return false;
if (size) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm assuming this is because mem_read and mem_write still require size to be greater than zero? Are they used anywhere directly where a size of zero should be allowed?

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.

2 participants