Skip to content

Commit

Permalink
[cl] Permit sizes of 0 in clEnqueueXXXBuffer; mux; HAL
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.

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.
  • Loading branch information
frasercrmck committed Mar 4, 2024
1 parent 7cd729b commit 677b9bb
Show file tree
Hide file tree
Showing 17 changed files with 72 additions and 39 deletions.
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) {
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})) {
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

0 comments on commit 677b9bb

Please sign in to comment.