From 677b9bb6cffcaf85b4055c89ba9b71bef5b4804e Mon Sep 17 00:00:00 2001 From: Fraser Cormack Date: Mon, 4 Mar 2024 10:18:14 +0000 Subject: [PATCH] [cl] Permit sizes of 0 in clEnqueueXXXBuffer; mux; HAL 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. --- doc/modules/mux/changes.rst | 7 +++++++ doc/specifications/mux-compiler-spec.rst | 2 +- doc/specifications/mux-runtime-spec.rst | 2 +- examples/refsi/hal_refsi/source/refsi_hal_g1.cpp | 14 ++++++++------ examples/refsi/hal_refsi/source/refsi_hal_m1.cpp | 4 ++++ modules/mux/include/mux/mux.h | 2 +- modules/mux/source/command_buffer.cpp | 16 ---------------- modules/mux/targets/host/include/host/host.h | 2 +- modules/mux/targets/host/source/queue.cpp | 4 ++++ modules/mux/targets/riscv/include/riscv/riscv.h | 2 +- modules/mux/tools/api/mux.xml | 2 +- source/cl/source/buffer.cpp | 4 ++++ source/cl/source/validate.cpp | 2 ++ .../test/UnitCL/source/clEnqueueCopyBuffer.cpp | 13 ++++++++++--- .../test/UnitCL/source/clEnqueueFillBuffer.cpp | 7 +++++++ .../test/UnitCL/source/clEnqueueReadBuffer.cpp | 14 ++++++++++---- .../test/UnitCL/source/clEnqueueWriteBuffer.cpp | 14 ++++++++++---- 17 files changed, 72 insertions(+), 39 deletions(-) diff --git a/doc/modules/mux/changes.rst b/doc/modules/mux/changes.rst index 23321c7a7..ccd3afc8d 100644 --- a/doc/modules/mux/changes.rst +++ b/doc/modules/mux/changes.rst @@ -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 ------ diff --git a/doc/specifications/mux-compiler-spec.rst b/doc/specifications/mux-compiler-spec.rst index b73fcd5ea..1ee63e993 100644 --- a/doc/specifications/mux-compiler-spec.rst +++ b/doc/specifications/mux-compiler-spec.rst @@ -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, diff --git a/doc/specifications/mux-runtime-spec.rst b/doc/specifications/mux-runtime-spec.rst index 10b78d526..a5b848ca5 100644 --- a/doc/specifications/mux-runtime-spec.rst +++ b/doc/specifications/mux-runtime-spec.rst @@ -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, diff --git a/examples/refsi/hal_refsi/source/refsi_hal_g1.cpp b/examples/refsi/hal_refsi/source/refsi_hal_g1.cpp index d1e398d60..14665fc2d 100644 --- a/examples/refsi/hal_refsi/source/refsi_hal_g1.cpp +++ b/examples/refsi/hal_refsi/source/refsi_hal_g1.cpp @@ -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 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 temp(size); + if (!mem_read(temp.data(), src, size, locker)) { + return false; + } + if (!mem_write(dst, temp.data(), size, locker)) { + return false; + } } return true; diff --git a/examples/refsi/hal_refsi/source/refsi_hal_m1.cpp b/examples/refsi/hal_refsi/source/refsi_hal_m1.cpp index 5c00862f6..380d6488e 100644 --- a/examples/refsi/hal_refsi/source/refsi_hal_m1.cpp +++ b/examples/refsi/hal_refsi/source/refsi_hal_m1.cpp @@ -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. diff --git a/modules/mux/include/mux/mux.h b/modules/mux/include/mux/mux.h index 1a7f680ba..6bb2b8c36 100644 --- a/modules/mux/include/mux/mux.h +++ b/modules/mux/include/mux/mux.h @@ -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. diff --git a/modules/mux/source/command_buffer.cpp b/modules/mux/source/command_buffer.cpp index 92f961fb6..153f3d022 100644 --- a/modules/mux/source/command_buffer.cpp +++ b/modules/mux/source/command_buffer.cpp @@ -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; } @@ -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; } @@ -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; } @@ -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; } diff --git a/modules/mux/targets/host/include/host/host.h b/modules/mux/targets/host/include/host/host.h index 6dfe93ac8..b517ae074 100644 --- a/modules/mux/targets/host/include/host/host.h +++ b/modules/mux/targets/host/include/host/host.h @@ -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. diff --git a/modules/mux/targets/host/source/queue.cpp b/modules/mux/targets/host/source/queue.cpp index 5355485b1..abcb0c9fc 100644 --- a/modules/mux/targets/host/source/queue.cpp +++ b/modules/mux/targets/host/source/queue.cpp @@ -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(fill->buffer); size_t size = fill->pattern_size; diff --git a/modules/mux/targets/riscv/include/riscv/riscv.h b/modules/mux/targets/riscv/include/riscv/riscv.h index 609701abb..0f8daa39a 100644 --- a/modules/mux/targets/riscv/include/riscv/riscv.h +++ b/modules/mux/targets/riscv/include/riscv/riscv.h @@ -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. diff --git a/modules/mux/tools/api/mux.xml b/modules/mux/tools/api/mux.xml index 1121eb0dd..c746e9f21 100644 --- a/modules/mux/tools/api/mux.xml +++ b/modules/mux/tools/api/mux.xml @@ -39,7 +39,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception ${FUNCTION_PREFIX}_MAJOR_VERSION0 ${Function_Prefix} major version number. - ${FUNCTION_PREFIX}_MINOR_VERSION80 + ${FUNCTION_PREFIX}_MINOR_VERSION81 ${Function_Prefix} minor version number. ${FUNCTION_PREFIX}_PATCH_VERSION0 ${Function_Prefix} patch version number. diff --git a/source/cl/source/buffer.cpp b/source/cl/source/buffer.cpp index ea73982a2..2667b34e9 100644 --- a/source/cl/source/buffer.cpp +++ b/source/cl/source/buffer.cpp @@ -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), @@ -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), diff --git a/source/cl/source/validate.cpp b/source/cl/source/validate.cpp index 5c31ab049..e64bd2c6a 100644 --- a/source/cl/source/validate.cpp +++ b/source/cl/source/validate.cpp @@ -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; diff --git a/source/cl/test/UnitCL/source/clEnqueueCopyBuffer.cpp b/source/cl/test/UnitCL/source/clEnqueueCopyBuffer.cpp index ad5c47dca..68585d760 100644 --- a/source/cl/test/UnitCL/source/clEnqueueCopyBuffer.cpp +++ b/source/cl/test/UnitCL/source/clEnqueueCopyBuffer.cpp @@ -16,6 +16,7 @@ #include "Common.h" #include "EventWaitList.h" +#include "ucl/checks.h" class clEnqueueCopyBufferCheckTest : public ucl::CommandQueueTest { protected: @@ -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) { diff --git a/source/cl/test/UnitCL/source/clEnqueueFillBuffer.cpp b/source/cl/test/UnitCL/source/clEnqueueFillBuffer.cpp index 11a838a49..ff970c63b 100644 --- a/source/cl/test/UnitCL/source/clEnqueueFillBuffer.cpp +++ b/source/cl/test/UnitCL/source/clEnqueueFillBuffer.cpp @@ -14,6 +14,8 @@ // // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +#include + #include #include "Common.h" @@ -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, diff --git a/source/cl/test/UnitCL/source/clEnqueueReadBuffer.cpp b/source/cl/test/UnitCL/source/clEnqueueReadBuffer.cpp index 31fece0fb..ea6a9fcec 100644 --- a/source/cl/test/UnitCL/source/clEnqueueReadBuffer.cpp +++ b/source/cl/test/UnitCL/source/clEnqueueReadBuffer.cpp @@ -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) { diff --git a/source/cl/test/UnitCL/source/clEnqueueWriteBuffer.cpp b/source/cl/test/UnitCL/source/clEnqueueWriteBuffer.cpp index 4ebb6550c..0ab32b850 100644 --- a/source/cl/test/UnitCL/source/clEnqueueWriteBuffer.cpp +++ b/source/cl/test/UnitCL/source/clEnqueueWriteBuffer.cpp @@ -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) {