From 074f4f6de74d1664bee98d8dc15d38a234299d51 Mon Sep 17 00:00:00 2001 From: Romaric Jodin Date: Mon, 2 Sep 2024 15:17:21 +0200 Subject: [PATCH] init image at creation time fix #719 --- README.md | 6 ++++++ src/CMakeLists.txt | 1 + src/config.def | 2 ++ src/context.cpp | 34 ++++++++++++++++++++++++++++++ src/context.hpp | 9 ++++++++ src/memory.cpp | 20 ++++++++++++++++++ src/queue.cpp | 2 ++ src/queue.hpp | 3 +++ tests/api/images.cpp | 49 ++++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 126 insertions(+) create mode 100644 src/context.cpp diff --git a/README.md b/README.md index 6fdf753a..c9d1a2c7 100644 --- a/README.md +++ b/README.md @@ -543,6 +543,12 @@ using the name of the corresponding environment variable. can be a work-around when having issues with clang compiling in the application thread. +* `CLVK_INIT_IMAGE_AT_CREATION` force to initialize OpenCL images created with + `CL_MEM_COPY_HOST_PTR` or `CL_MEM_USE_HOST_PTR` at creation time instead of + initializing them during first use of the image (default: false). It reduces + the memory footprint as clvk needs to keep a buffer with the data to + initialize at first use. + # Limitations * Only one device per CL context diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index d848e02a..f68a6d5d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -56,6 +56,7 @@ endif() add_library(OpenCL-objects OBJECT api.cpp config.cpp + context.cpp device.cpp device_properties.cpp event.cpp diff --git a/src/config.def b/src/config.def index 005c06b6..e7ef2ffd 100644 --- a/src/config.def +++ b/src/config.def @@ -31,6 +31,8 @@ OPTION(uint32_t, opencl_version, (uint32_t)CL_MAKE_VERSION(3, 0, 0)) OPTION(bool, build_in_separate_thread, false) +OPTION(bool, init_image_at_creation, false) + #if COMPILER_AVAILABLE OPTION(std::string, clspv_options, "") #if !CLSPV_ONLINE_COMPILER diff --git a/src/context.cpp b/src/context.cpp new file mode 100644 index 00000000..6a1ca473 --- /dev/null +++ b/src/context.cpp @@ -0,0 +1,34 @@ +// Copyright 2024 The clvk authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "context.hpp" +#include "queue.hpp" + +cvk_command_queue* cvk_context::get_or_create_image_init_command_queue() { + std::unique_lock lock(m_queue_image_init_lock); + if (m_queue_image_init != nullptr) { + return m_queue_image_init; + } + std::vector properties_array; + m_queue_image_init = + new cvk_command_queue(this, m_device, 0, std::move(properties_array)); + cl_int ret = m_queue_image_init->init(); + if (ret != CL_SUCCESS) { + return nullptr; + } + m_queue_image_init->detach_from_context(); + return m_queue_image_init; +} + +void cvk_context::free_image_init_command_queue() { delete m_queue_image_init; } diff --git a/src/context.hpp b/src/context.hpp index de3314e6..8a6cf3a5 100644 --- a/src/context.hpp +++ b/src/context.hpp @@ -29,6 +29,8 @@ struct cvk_context_callback { void* data; }; +struct cvk_command_queue; + struct cvk_context : public _cl_context, refcounted, object_magic_header { @@ -73,6 +75,7 @@ struct cvk_context : public _cl_context, auto cb = *cbi; cb.pointer(this, cb.data); } + free_image_init_command_queue(); } const std::vector& properties() const { @@ -116,6 +119,9 @@ struct cvk_context : public _cl_context, cvk_printf_callback_t get_printf_callback() { return m_printf_callback; } void* get_printf_userdata() { return m_user_data; } + cvk_command_queue* get_or_create_image_init_command_queue(); + void free_image_init_command_queue(); + private: cvk_device* m_device; std::mutex m_callbacks_lock; @@ -124,6 +130,9 @@ struct cvk_context : public _cl_context, size_t m_printf_buffersize; cvk_printf_callback_t m_printf_callback; void* m_user_data; + + std::mutex m_queue_image_init_lock; + cvk_command_queue* m_queue_image_init = nullptr; }; static inline cvk_context* icd_downcast(cl_context context) { diff --git a/src/memory.cpp b/src/memory.cpp index 36230533..dad59d5e 100644 --- a/src/memory.cpp +++ b/src/memory.cpp @@ -508,6 +508,26 @@ bool cvk_image::init_vulkan_image() { "Could not copy image host_ptr data to the staging buffer"); return false; } + + if (config.init_image_at_creation()) { + auto queue = m_context->get_or_create_image_init_command_queue(); + if (queue == nullptr) { + return false; + } + + auto initimage = new cvk_command_image_init(queue, this); + ret = queue->enqueue_command_with_deps(initimage, 0, nullptr, + nullptr); + if (ret != CL_SUCCESS) { + return false; + } + ret = queue->finish(); + if (ret != CL_SUCCESS) { + return false; + } + std::lock_guard lock(m_init_tracker.mutex()); + m_init_tracker.set_state(cvk_mem_init_state::completed); + } } return true; diff --git a/src/queue.cpp b/src/queue.cpp index 861370a1..4ebfe4b8 100644 --- a/src/queue.cpp +++ b/src/queue.cpp @@ -80,6 +80,8 @@ cvk_command_queue::~cvk_command_queue() { } } +void cvk_command_queue::detach_from_context() { m_context.reset(nullptr); } + cl_int cvk_command_queue::satisfy_data_dependencies(cvk_command* cmd) { if (cmd->is_data_movement()) { return CL_SUCCESS; diff --git a/src/queue.hpp b/src/queue.hpp index d3bc593c..e63db68f 100644 --- a/src/queue.hpp +++ b/src/queue.hpp @@ -175,6 +175,7 @@ struct cvk_command_queue : public _cl_command_queue, } cvk_buffer* get_or_create_printf_buffer() { + CVK_ASSERT(m_context != nullptr); if (!m_printf_buffer) { cl_int status; m_printf_buffer = cvk_buffer::create( @@ -237,6 +238,8 @@ struct cvk_command_queue : public _cl_command_queue, cl_int execute_cmds_required_by_no_lock(cl_uint num_events, _cl_event* const* event_list); + void detach_from_context(); + private: CHECK_RETURN cl_int satisfy_data_dependencies(cvk_command* cmd); void enqueue_command(cvk_command* cmd); diff --git a/tests/api/images.cpp b/tests/api/images.cpp index 306e2997..8d960ff3 100644 --- a/tests/api/images.cpp +++ b/tests/api/images.cpp @@ -15,6 +15,7 @@ #define CL_USE_DEPRECATED_OPENCL_1_1_APIS #include "testcl.hpp" +#include "utils.hpp" TEST_F(WithContext, CreateImageLegacy) { cl_image_format format = {CL_RGBA, CL_UNORM_INT8}; @@ -804,3 +805,51 @@ TEST_F(WithCommandQueue, 1DBufferImageReleaseAfterUnmap) { Finish(); EXPECT_TRUE(destructor_called); } + +#ifdef CLVK_UNIT_TESTING_ENABLED + +TEST_F(WithCommandQueue, ImageInitAtCreation) { + auto cfg = + CLVK_CONFIG_SCOPED_OVERRIDE(init_image_at_creation, bool, true, true); + + const size_t IMAGE_WIDTH = 16; + const cl_uchar init_value = 0xAB; + std::vector host_data(IMAGE_WIDTH, init_value); + std::vector read_data(IMAGE_WIDTH, 0); + + cl_image_format format = {CL_R, CL_UNSIGNED_INT8}; + cl_image_desc desc = { + CL_MEM_OBJECT_IMAGE1D, // image_type + IMAGE_WIDTH, // image_width + 1, // image_height + 1, // image_depth + 0, // image_array_size + 0, // image_row_pitch + 0, // image_slice_pitch + 0, // num_mip_levels + 0, // num_samples + nullptr, // buffer + }; + auto image = CreateImage(CL_MEM_READ_WRITE | CL_MEM_COPY_HOST_PTR, &format, + &desc, host_data.data()); + + // Read the data back from the image. + size_t origin[3] = {0, 0, 0}; + size_t region[3] = {IMAGE_WIDTH, 1, 1}; + EnqueueReadImage(image, CL_FALSE, origin, region, 0, 0, read_data.data()); + Finish(); + + // Check that we got the values copied from the initial host pointer. + bool success = true; + for (int i = 0; i < IMAGE_WIDTH; ++i) { + auto val = read_data[i]; + if (val != init_value) { + printf("Failed comparison at data[%d]: expected %d but got %d\n", i, + init_value, val); + success = false; + } + } + EXPECT_TRUE(success); +} + +#endif