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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions doc/modules/mux/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ version increases mean backward compatible bug fixes have been applied.
Versions prior to 1.0.0 may contain breaking changes in minor
versions as the API is still under development.

0.81.0
------

* ``muxCommandReadBuffer``, ``muxCommandWriteBuffer``,
``muxCommandCopyBuffer``, and ``muxCommandFillBuffer`` now accept a ``size``
of zero.

0.80.0
------

Expand Down
2 changes: 1 addition & 1 deletion doc/specifications/mux-compiler-spec.rst
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
ComputeMux Compiler Specification
=================================

This is version 0.80.0 of the specification.
This is version 0.81.0 of the specification.

ComputeMux is Codeplay’s proprietary API for executing compute workloads across
heterogeneous devices. ComputeMux is an extremely lightweight,
Expand Down
2 changes: 1 addition & 1 deletion doc/specifications/mux-runtime-spec.rst
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
ComputeMux Runtime Specification
================================

This is version 0.80.0 of the specification.
This is version 0.81.0 of the specification.

ComputeMux is Codeplay’s proprietary API for executing compute workloads across
heterogeneous devices. ComputeMux is an extremely lightweight,
Expand Down
14 changes: 8 additions & 6 deletions examples/refsi/hal_refsi/source/refsi_hal_g1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,12 +273,14 @@ bool refsi_g1_hal_device::mem_copy(hal::hal_addr_t dst, hal::hal_addr_t src,
dst, src, size);
}

std::vector<uint8_t> temp(size);
if (!mem_read(temp.data(), src, size, locker)) {
return false;
}
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?

std::vector<uint8_t> temp(size);
if (!mem_read(temp.data(), src, size, locker)) {
return false;
}
if (!mem_write(dst, temp.data(), size, locker)) {
return false;
}
}

return true;
Expand Down
4 changes: 4 additions & 0 deletions examples/refsi/hal_refsi/source/refsi_hal_m1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,10 @@ bool refsi_m1_hal_device::mem_copy(hal::hal_addr_t dst, hal::hal_addr_t src,
dst, src, size);
}

if (!size) {
return true;
}

refsi_command_buffer cb;

// Start a 1D DMA transfer to copy data from one buffer to another.
Expand Down
2 changes: 1 addition & 1 deletion modules/mux/include/mux/mux.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ extern "C" {
/// @brief Mux major version number.
#define MUX_MAJOR_VERSION 0
/// @brief Mux minor version number.
#define MUX_MINOR_VERSION 80
#define MUX_MINOR_VERSION 81
/// @brief Mux patch version number.
#define MUX_PATCH_VERSION 0
/// @brief Mux combined version number.
Expand Down
16 changes: 0 additions & 16 deletions modules/mux/source/command_buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,6 @@ mux_result_t muxCommandCopyBuffer(mux_command_buffer_t command_buffer,
return mux_error_invalid_value;
}

if (0 == size) {
return mux_error_invalid_value;
}

if ((size + src_offset) > src_buffer->memory_requirements.size) {
return mux_error_invalid_value;
}
Expand Down Expand Up @@ -350,10 +346,6 @@ mux_result_t muxCommandFillBuffer(mux_command_buffer_t command_buffer,
return mux_error_invalid_value;
}

if (0 == size) {
return mux_error_invalid_value;
}

if ((size + offset) > buffer->memory_requirements.size) {
return mux_error_invalid_value;
}
Expand Down Expand Up @@ -397,10 +389,6 @@ mux_result_t muxCommandReadBuffer(mux_command_buffer_t command_buffer,
return mux_error_invalid_value;
}

if (0 == size) {
return mux_error_invalid_value;
}

if ((size + offset) > buffer->memory_requirements.size) {
return mux_error_invalid_value;
}
Expand Down Expand Up @@ -552,10 +540,6 @@ mux_result_t muxCommandWriteBuffer(mux_command_buffer_t command_buffer,
return mux_error_invalid_value;
}

if (0 == size) {
return mux_error_invalid_value;
}

if ((size + offset) > buffer->memory_requirements.size) {
return mux_error_invalid_value;
}
Expand Down
2 changes: 1 addition & 1 deletion modules/mux/targets/host/include/host/host.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ extern "C" {
/// @brief Host major version number.
#define HOST_MAJOR_VERSION 0
/// @brief Host minor version number.
#define HOST_MINOR_VERSION 80
#define HOST_MINOR_VERSION 81
/// @brief Host patch version number.
#define HOST_PATCH_VERSION 0
/// @brief Host combined version number.
Expand Down
4 changes: 4 additions & 0 deletions modules/mux/targets/host/source/queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ void commandWriteBuffer(host::command_info_s *info) {
void commandFillBuffer(host::command_info_s *info) {
host::command_info_fill_buffer_s *const fill = &(info->fill_command);

if (!fill->size) {
return;
}

auto buffer = static_cast<host::buffer_s *>(fill->buffer);

size_t size = fill->pattern_size;
Expand Down
2 changes: 1 addition & 1 deletion modules/mux/targets/riscv/include/riscv/riscv.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ extern "C" {
/// @brief Riscv major version number.
#define RISCV_MAJOR_VERSION 0
/// @brief Riscv minor version number.
#define RISCV_MINOR_VERSION 80
#define RISCV_MINOR_VERSION 81
/// @brief Riscv patch version number.
#define RISCV_PATCH_VERSION 0
/// @brief Riscv combined version number.
Expand Down
2 changes: 1 addition & 1 deletion modules/mux/tools/api/mux.xml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception</comment>
<block>
<define priority="high">${FUNCTION_PREFIX}_MAJOR_VERSION<value>0</value>
<doxygen><brief>${Function_Prefix} major version number.</brief></doxygen></define>
<define priority="high">${FUNCTION_PREFIX}_MINOR_VERSION<value>80</value>
<define priority="high">${FUNCTION_PREFIX}_MINOR_VERSION<value>81</value>
<doxygen><brief>${Function_Prefix} minor version number.</brief></doxygen></define>
<define priority="high">${FUNCTION_PREFIX}_PATCH_VERSION<value>0</value>
<doxygen><brief>${Function_Prefix} patch version number.</brief></doxygen></define>
Expand Down
4 changes: 4 additions & 0 deletions source/cl/source/buffer.cpp
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 @@ -928,7 +930,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
2 changes: 2 additions & 0 deletions source/cl/source/validate.cpp
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
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
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
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})) {
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?

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
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
Loading