Skip to content

Commit

Permalink
[cl] Permit size 0 in clEnqueue(Read|Write|Copy|Fill)Buffer
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@frasercrmck
frasercrmck committed Mar 4, 2024
1 parent 7cd729b commit 679de07
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 54 deletions.
98 changes: 55 additions & 43 deletions source/cl/source/buffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,9 @@ cl::EnqueueWriteBuffer(cl_command_queue command_queue, cl_mem buffer,
return CL_INVALID_CONTEXT);
OCL_CHECK(buffer->size < (offset + size), return CL_INVALID_VALUE);
OCL_CHECK(!ptr, return CL_INVALID_VALUE);
#if CL_TARGET_OPENCL_VERSION < 210
OCL_CHECK(0 == size, return CL_INVALID_VALUE);
#endif
OCL_CHECK(cl::validate::IsInBitSet(buffer->flags, CL_MEM_HOST_READ_ONLY),
return CL_INVALID_OPERATION);
OCL_CHECK(cl::validate::IsInBitSet(buffer->flags, CL_MEM_HOST_NO_ACCESS),
Expand Down Expand Up @@ -873,18 +875,20 @@ cl::EnqueueWriteBuffer(cl_command_queue command_queue, cl_mem buffer,
return CL_OUT_OF_RESOURCES;
}

auto device_index = command_queue->getDeviceIndex();
auto mux_buffer =
static_cast<cl_mem_buffer>(buffer)->mux_buffers[device_index];
auto mux_error =
muxCommandWriteBuffer(*mux_command_buffer, mux_buffer, offset, ptr,
size, 0, nullptr, nullptr);
if (mux_error) {
auto error = cl::getErrorFrom(mux_error);
if (event_release_guard) {
event_release_guard->complete(error);
if (0 != size) {
auto device_index = command_queue->getDeviceIndex();
auto mux_buffer =
static_cast<cl_mem_buffer>(buffer)->mux_buffers[device_index];
auto mux_error =
muxCommandWriteBuffer(*mux_command_buffer, mux_buffer, offset, ptr,
size, 0, nullptr, nullptr);
if (mux_error) {
auto error = cl::getErrorFrom(mux_error);
if (event_release_guard) {
event_release_guard->complete(error);
}
return error;
}
return error;
}

cl::retainInternal(buffer);
Expand Down Expand Up @@ -928,7 +932,9 @@ CL_API_ENTRY cl_int CL_API_CALL cl::EnqueueReadBuffer(
return CL_INVALID_CONTEXT);
OCL_CHECK(buffer->size < (offset + size), return CL_INVALID_VALUE);
OCL_CHECK(!ptr, return CL_INVALID_VALUE);
#if CL_TARGET_OPENCL_VERSION < 210
OCL_CHECK(0 == size, return CL_INVALID_VALUE);
#endif
OCL_CHECK(cl::validate::IsInBitSet(buffer->flags, CL_MEM_HOST_WRITE_ONLY),
return CL_INVALID_OPERATION);
OCL_CHECK(cl::validate::IsInBitSet(buffer->flags, CL_MEM_HOST_NO_ACCESS),
Expand Down Expand Up @@ -960,14 +966,16 @@ CL_API_ENTRY cl_int CL_API_CALL cl::EnqueueReadBuffer(
return CL_OUT_OF_RESOURCES;
}

auto device_index = command_queue->getDeviceIndex();
auto mux_buffer =
static_cast<cl_mem_buffer>(buffer)->mux_buffers[device_index];
auto mux_error =
muxCommandReadBuffer(*mux_command_buffer, mux_buffer, offset, ptr, size,
0, nullptr, nullptr);
if (mux_error) {
return cl::getErrorFrom(mux_error);
if (0 != size) {
auto device_index = command_queue->getDeviceIndex();
auto mux_buffer =
static_cast<cl_mem_buffer>(buffer)->mux_buffers[device_index];
auto mux_error =
muxCommandReadBuffer(*mux_command_buffer, mux_buffer, offset, ptr,
size, 0, nullptr, nullptr);
if (mux_error) {
return cl::getErrorFrom(mux_error);
}
}

cl::retainInternal(buffer);
Expand Down Expand Up @@ -1038,20 +1046,22 @@ cl::EnqueueCopyBuffer(cl_command_queue command_queue, cl_mem src_buffer,
return CL_OUT_OF_RESOURCES;
}

auto device_index = command_queue->getDeviceIndex();
auto mux_src_buffer =
static_cast<cl_mem_buffer>(src_buffer)->mux_buffers[device_index];
auto mux_dst_buffer =
static_cast<cl_mem_buffer>(dst_buffer)->mux_buffers[device_index];
auto mux_error = muxCommandCopyBuffer(*mux_command_buffer, mux_src_buffer,
src_offset, mux_dst_buffer, dst_offset,
size, 0, nullptr, nullptr);
if (mux_error) {
auto error = cl::getErrorFrom(mux_error);
if (return_event) {
return_event->complete(error);
if (0 != size) {
auto device_index = command_queue->getDeviceIndex();
auto mux_src_buffer =
static_cast<cl_mem_buffer>(src_buffer)->mux_buffers[device_index];
auto mux_dst_buffer =
static_cast<cl_mem_buffer>(dst_buffer)->mux_buffers[device_index];
auto mux_error = muxCommandCopyBuffer(
*mux_command_buffer, mux_src_buffer, src_offset, mux_dst_buffer,
dst_offset, size, 0, nullptr, nullptr);
if (mux_error) {
auto error = cl::getErrorFrom(mux_error);
if (return_event) {
return_event->complete(error);
}
return error;
}
return error;
}

cl::retainInternal(src_buffer);
Expand Down Expand Up @@ -1104,18 +1114,20 @@ cl::EnqueueFillBuffer(cl_command_queue command_queue, cl_mem buffer,
return CL_OUT_OF_RESOURCES;
}

auto device_index = command_queue->getDeviceIndex();
auto mux_buffer =
static_cast<cl_mem_buffer>(buffer)->mux_buffers[device_index];
auto mux_error =
muxCommandFillBuffer(*mux_command_buffer, mux_buffer, offset, size,
pattern, pattern_size, 0, nullptr, nullptr);
if (mux_error) {
auto error = cl::getErrorFrom(mux_error);
if (return_event) {
return_event->complete(error);
if (0 != size) {
auto device_index = command_queue->getDeviceIndex();
auto mux_buffer =
static_cast<cl_mem_buffer>(buffer)->mux_buffers[device_index];
auto mux_error =
muxCommandFillBuffer(*mux_command_buffer, mux_buffer, offset, size,
pattern, pattern_size, 0, nullptr, nullptr);
if (mux_error) {
auto error = cl::getErrorFrom(mux_error);
if (return_event) {
return_event->complete(error);
}
return error;
}
return error;
}

cl::retainInternal(buffer);
Expand Down
2 changes: 2 additions & 0 deletions source/cl/source/validate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,9 @@ cl_int CopyBufferArguments(cl_command_queue command_queue, cl_mem src_buffer,
return CL_INVALID_VALUE);
OCL_CHECK(src_offset + size > src_buffer->size, return CL_INVALID_VALUE);
OCL_CHECK(dst_offset + size > dst_buffer->size, return CL_INVALID_VALUE);
#if CL_TARGET_OPENCL_VERSION < 210
OCL_CHECK(size == 0, return CL_INVALID_VALUE);
#endif
size_t src_start = src_offset;
size_t dst_start = dst_offset;
size_t src_buffer_size = 0;
Expand Down
13 changes: 10 additions & 3 deletions source/cl/test/UnitCL/source/clEnqueueCopyBuffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include "Common.h"
#include "EventWaitList.h"
#include "ucl/checks.h"

class clEnqueueCopyBufferCheckTest : public ucl::CommandQueueTest {
protected:
Expand Down Expand Up @@ -296,9 +297,15 @@ TEST_F(clEnqueueCopyBufferCheckTest, dst_bufferSizePlusOffsetTooLarge) {
}

TEST_F(clEnqueueCopyBufferCheckTest, BufferSizeZero) {
ASSERT_EQ_ERRCODE(CL_INVALID_VALUE,
clEnqueueCopyBuffer(command_queue, src_buffer, src_buffer,
0, 0, 0, 0, nullptr, nullptr));
// An error when size == 0 was removed starting with OpenCL 2.1.
if (UCL::isDeviceVersionAtLeast({2, 1})) {
ASSERT_SUCCESS(clEnqueueCopyBuffer(command_queue, src_buffer, src_buffer, 0,
0, 0, 0, nullptr, nullptr));
} else {
ASSERT_EQ_ERRCODE(CL_INVALID_VALUE,
clEnqueueCopyBuffer(command_queue, src_buffer, src_buffer,
0, 0, 0, 0, nullptr, nullptr));
}
}

TEST_F(clEnqueueCopyBufferTest, CopyBufferNoEvents) {
Expand Down
7 changes: 7 additions & 0 deletions source/cl/test/UnitCL/source/clEnqueueFillBuffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
//
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include <CL/cl.h>

#include <array>

#include "Common.h"
Expand Down Expand Up @@ -122,6 +124,11 @@ TEST_F(clEnqueueFillBufferTest, ZeroPatternSize) {
size, 0, nullptr, nullptr));
}

TEST_F(clEnqueueFillBufferTest, SizeZero) {
ASSERT_SUCCESS(clEnqueueFillBuffer(command_queue, buffer, &pattern,
pattern_size, 0, 0, 0, nullptr, nullptr));
}

TEST_F(clEnqueueFillBufferTest, BadPatternSizes) {
EXPECT_EQ_ERRCODE(CL_INVALID_VALUE,
clEnqueueFillBuffer(command_queue, buffer, &pattern, 3, 0,
Expand Down
14 changes: 10 additions & 4 deletions source/cl/test/UnitCL/source/clEnqueueReadBuffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,16 @@ TEST_F(clEnqueueReadBufferTest, InvalidBuffer) {
0, nullptr, nullptr));
}

TEST_F(clEnqueueReadBufferTest, InvalidSize) {
ASSERT_EQ_ERRCODE(CL_INVALID_VALUE,
clEnqueueReadBuffer(command_queue, inMem, CL_TRUE, 0, 0,
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})) {
ASSERT_SUCCESS(clEnqueueReadBuffer(command_queue, inMem, CL_TRUE, 0, 0,
inBuffer, 0, nullptr, nullptr));
} else {
ASSERT_EQ_ERRCODE(CL_INVALID_VALUE,
clEnqueueReadBuffer(command_queue, inMem, CL_TRUE, 0, 0,
inBuffer, 0, nullptr, nullptr));
}
}

TEST_F(clEnqueueReadBufferTest, NullWaitList) {
Expand Down
14 changes: 10 additions & 4 deletions source/cl/test/UnitCL/source/clEnqueueWriteBuffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,16 @@ TEST_F(clEnqueueWriteBufferTest, InvalidBuffer) {
nullptr, 0, nullptr, nullptr));
}

TEST_F(clEnqueueWriteBufferTest, InvalidSize) {
ASSERT_EQ_ERRCODE(CL_INVALID_VALUE,
clEnqueueWriteBuffer(command_queue, mem, true, 0, 0,
&buffer[0], 0, nullptr, nullptr));
TEST_F(clEnqueueWriteBufferTest, SizeZero) {
// An error when size == 0 was removed starting with OpenCL 2.1.
if (UCL::isDeviceVersionAtLeast({2, 1})) {
ASSERT_SUCCESS(clEnqueueWriteBuffer(command_queue, mem, true, 0, 0,
&buffer[0], 0, nullptr, nullptr));
} else {
ASSERT_EQ_ERRCODE(CL_INVALID_VALUE,
clEnqueueWriteBuffer(command_queue, mem, true, 0, 0,
&buffer[0], 0, nullptr, nullptr));
}
}

TEST_F(clEnqueueWriteBufferTest, WriteToReadOnly) {
Expand Down

0 comments on commit 679de07

Please sign in to comment.