Skip to content

Commit

Permalink
fix support for read of 3D images with unnormalised sampler (#614)
Browse files Browse the repository at this point in the history 
Set the sampler mask in the push constant, but also create a new
sampler with normalised coordinate if not already the case or already
created.

Co-authored-by: Kévin Petit <[email protected]>
  • Loading branch information
@rjodinchr @kpet
rjodinchr and kpet authored Oct 29, 2023
1 parent e88369c commit b65b365
Show file tree
Hide file tree
Showing 8 changed files with 242 additions and 32 deletions.
64 changes: 63 additions & 1 deletion src/kernel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

#include <algorithm>

#include "clspv/Sampler.h"

#include "kernel.hpp"
#include "memory.hpp"

Expand Down Expand Up @@ -44,6 +46,10 @@ cl_int cvk_kernel::init() {
m_image_metadata = md;
}

if (const auto* md = m_entry_point->sampler_metadata()) {
m_sampler_metadata = md;
}

// Init argument values
m_argument_values = cvk_kernel_argument_values::create(m_entry_point);
if (m_argument_values == nullptr) {
Expand Down Expand Up @@ -100,6 +106,45 @@ void cvk_kernel::set_image_metadata(cl_uint index, const void* image) {
}
}

void cvk_kernel::set_sampler_metadata(cl_uint index, const void* sampler) {
if (!m_sampler_metadata) {
return;
}
auto md = m_sampler_metadata->find(index);
if (md != m_sampler_metadata->end()) {
auto apisampler = *reinterpret_cast<const cl_sampler*>(sampler);
auto offset = md->second;
auto sampler = icd_downcast(apisampler);
uint32_t sampler_mask = (sampler->normalized_coords()
? clspv::CLK_NORMALIZED_COORDS_TRUE
: clspv::CLK_NORMALIZED_COORDS_FALSE) |
(sampler->filter_mode() == CL_FILTER_NEAREST
? clspv::CLK_FILTER_NEAREST
: clspv::CLK_FILTER_LINEAR);
switch (sampler->addressing_mode()) {
case CL_ADDRESS_NONE:
sampler_mask |= clspv::CLK_ADDRESS_NONE;
break;
case CL_ADDRESS_CLAMP:
sampler_mask |= clspv::CLK_ADDRESS_CLAMP;
break;
case CL_ADDRESS_REPEAT:
sampler_mask |= clspv::CLK_ADDRESS_REPEAT;
break;
case CL_ADDRESS_CLAMP_TO_EDGE:
sampler_mask |= clspv::CLK_ADDRESS_CLAMP_TO_EDGE;
break;
case CL_ADDRESS_MIRRORED_REPEAT:
sampler_mask |= clspv::CLK_ADDRESS_MIRRORED_REPEAT;
break;
default:
break;
}
m_argument_values->set_pod_data(offset, sizeof(sampler_mask),
&sampler_mask);
}
}

cl_int cvk_kernel::set_arg(cl_uint index, size_t size, const void* value) {
std::lock_guard<std::mutex> lock(m_lock);

Expand All @@ -125,6 +170,10 @@ cl_int cvk_kernel::set_arg(cl_uint index, size_t size, const void* value) {
set_image_metadata(index, value);
}

if (arg.kind == kernel_argument_kind::sampler) {
set_sampler_metadata(index, value);
}

return ret;
}

Expand Down Expand Up @@ -268,7 +317,20 @@ bool cvk_kernel_argument_values::setup_descriptor_sets() {
}
case kernel_argument_kind::sampler: {
auto clsampler = static_cast<cvk_sampler*>(get_arg_value(arg));
auto sampler = clsampler->vulkan_sampler();
bool normalized_coord_sampler_required = false;
if (auto md = m_entry_point->sampler_metadata()) {
normalized_coord_sampler_required = md->find(i) != md->end();
}
auto sampler =
normalized_coord_sampler_required &&
!clsampler->normalized_coords()
? clsampler
->get_or_create_vulkan_sampler_with_normalized_coords()
: clsampler->vulkan_sampler();
if (sampler == VK_NULL_HANDLE) {
cvk_error_fn("Could not set descriptor for sampler");
return false;
}

cvk_debug_fn("sampler %p @ set = %u, binding = %u", sampler,
arg.descriptorSet, arg.binding);
Expand Down
13 changes: 11 additions & 2 deletions src/kernel.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ struct cvk_kernel : public _cl_kernel, api_object<object_magic::kernel> {

cvk_kernel(cvk_program* program, const char* name)
: api_object(program->context()), m_program(program),
m_entry_point(nullptr), m_name(name), m_image_metadata(nullptr) {}
m_entry_point(nullptr), m_name(name), m_sampler_metadata(nullptr),
m_image_metadata(nullptr) {}

CHECK_RETURN cl_int init();
std::unique_ptr<cvk_kernel> clone(cl_int* errcode_ret) const;
Expand All @@ -42,10 +43,16 @@ struct cvk_kernel : public _cl_kernel, api_object<object_magic::kernel> {
return m_argument_values;
}

const kernel_sampler_metadata_map* get_sampler_metadata() const {
return m_sampler_metadata;
}

const kernel_image_metadata_map* get_image_metadata() const {
return m_image_metadata;
}

void set_sampler_metadata(cl_uint index, const void* sampler);

void set_image_metadata(cl_uint index, const void* image);

CHECK_RETURN cl_int set_arg(cl_uint index, size_t size, const void* value);
Expand Down Expand Up @@ -158,6 +165,7 @@ struct cvk_kernel : public _cl_kernel, api_object<object_magic::kernel> {
std::string m_name;
std::vector<kernel_argument> m_args;
std::shared_ptr<cvk_kernel_argument_values> m_argument_values;
const kernel_sampler_metadata_map* m_sampler_metadata;
const kernel_image_metadata_map* m_image_metadata;
};

Expand Down Expand Up @@ -237,7 +245,8 @@ struct cvk_kernel_argument_values {
}

if (m_entry_point->has_pod_arguments() ||
m_entry_point->has_image_metadata()) {
m_entry_point->has_image_metadata() ||
m_entry_point->has_sampler_metadata()) {
// TODO(#101): host out-of-memory errors are currently unhandled.
auto buffer = std::make_unique<std::vector<uint8_t>>(
m_entry_point->pod_buffer_size());
Expand Down
8 changes: 5 additions & 3 deletions src/memory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ cvk_sampler::create(cvk_context* context, bool normalized_coords,
return sampler.release();
}

bool cvk_sampler::init() {
bool cvk_sampler::init(bool force_normalized_coordinates) {
auto vkdev = context()->device()->vulkan_device();

// Translate addressing mode
Expand Down Expand Up @@ -199,7 +199,7 @@ bool cvk_sampler::init() {

// Translate coordinate type
VkBool32 unnormalized_coordinates;
if (m_normalized_coords) {
if (m_normalized_coords || force_normalized_coordinates) {
unnormalized_coordinates = VK_FALSE;
} else {
unnormalized_coordinates = VK_TRUE;
Expand Down Expand Up @@ -235,7 +235,9 @@ bool cvk_sampler::init() {
unnormalized_coordinates, // unnormalizedCoordinates
};

auto res = vkCreateSampler(vkdev, &create_info, nullptr, &m_sampler);
VkSampler* sampler =
force_normalized_coordinates ? &m_sampler_norm : &m_sampler;
auto res = vkCreateSampler(vkdev, &create_info, nullptr, sampler);

return (res == VK_SUCCESS);
}
Expand Down
19 changes: 16 additions & 3 deletions src/memory.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -431,13 +431,17 @@ struct cvk_sampler : public _cl_sampler, api_object<object_magic::sampler> {
std::vector<cl_sampler_properties>&& properties)
: api_object(context), m_normalized_coords(normalized_coords),
m_addressing_mode(addressing_mode), m_filter_mode(filter_mode),
m_properties(std::move(properties)), m_sampler(VK_NULL_HANDLE) {}
m_properties(std::move(properties)), m_sampler(VK_NULL_HANDLE),
m_sampler_norm(VK_NULL_HANDLE) {}

~cvk_sampler() {
auto vkdev = context()->device()->vulkan_device();
if (m_sampler != VK_NULL_HANDLE) {
auto vkdev = context()->device()->vulkan_device();
vkDestroySampler(vkdev, m_sampler, nullptr);
}
if (m_sampler_norm != VK_NULL_HANDLE) {
vkDestroySampler(vkdev, m_sampler_norm, nullptr);
}
}

static cvk_sampler* create(cvk_context* context, bool normalized_coords,
Expand All @@ -456,17 +460,26 @@ struct cvk_sampler : public _cl_sampler, api_object<object_magic::sampler> {
cl_addressing_mode addressing_mode() const { return m_addressing_mode; }
cl_filter_mode filter_mode() const { return m_filter_mode; }
VkSampler vulkan_sampler() const { return m_sampler; }
VkSampler get_or_create_vulkan_sampler_with_normalized_coords() {
if (m_sampler_norm == VK_NULL_HANDLE) {
if (!init(true)) {
return VK_NULL_HANDLE;
}
}
return m_sampler_norm;
}
const std::vector<cl_sampler_properties>& properties() const {
return m_properties;
}

private:
bool init();
bool init(bool force_normalized_coordinates = false);
bool m_normalized_coords;
cl_addressing_mode m_addressing_mode;
cl_filter_mode m_filter_mode;
const std::vector<cl_sampler_properties> m_properties;
VkSampler m_sampler;
VkSampler m_sampler_norm;
};

static inline cvk_sampler* icd_downcast(cl_sampler sampler) {
Expand Down
82 changes: 59 additions & 23 deletions src/program.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ spv_result_t parse_reflection(void* user_data,
return pushconstant::module_constants_pointer;
case NonSemanticClspvReflectionPrintfBufferPointerPushConstant:
return pushconstant::printf_buffer_pointer;
case NonSemanticClspvReflectionNormalizedSamplerMaskPushConstant:
return pushconstant::normalized_sampler_mask;
default:
cvk_error_fn("Unhandled reflection instruction for push constant");
break;
Expand Down Expand Up @@ -212,6 +214,17 @@ spv_result_t parse_reflection(void* user_data,
parse_data->arg_infos[inst->result_id] = info;
break;
}
case NonSemanticClspvReflectionNormalizedSamplerMaskPushConstant: {
auto kernel = parse_data->strings[inst->words[5]];
auto ordinal = parse_data->constants[inst->words[6]];
auto offset = parse_data->constants[inst->words[7]];
auto size = parse_data->constants[inst->words[8]];
parse_data->binary->add_sampler_metadata(kernel, ordinal,
offset);
auto pc = inst_to_push_constant(ext_inst);
parse_data->binary->add_push_constant(pc, {offset, size});
break;
}
case NonSemanticClspvReflectionImageArgumentInfoChannelOrderPushConstant: {
auto kernel = parse_data->strings[inst->words[5]];
auto ordinal = parse_data->constants[inst->words[6]];
Expand Down Expand Up @@ -1674,8 +1687,9 @@ cvk_entry_point::cvk_entry_point(VkDevice dev, cvk_program* program,
: m_device(dev), m_context(program->context()), m_program(program),
m_name(name), m_pod_descriptor_type(VK_DESCRIPTOR_TYPE_MAX_ENUM),
m_pod_buffer_size(0u), m_has_pod_arguments(false),
m_has_pod_buffer_arguments(false), m_image_metadata(nullptr),
m_descriptor_pool(VK_NULL_HANDLE), m_pipeline_layout(VK_NULL_HANDLE) {}
m_has_pod_buffer_arguments(false), m_sampler_metadata(nullptr),
m_image_metadata(nullptr), m_descriptor_pool(VK_NULL_HANDLE),
m_pipeline_layout(VK_NULL_HANDLE) {}

cvk_entry_point* cvk_program::get_entry_point(std::string& name,
cl_int* errcode_ret) {
Expand Down Expand Up @@ -1895,6 +1909,11 @@ cl_int cvk_entry_point::init() {
m_image_metadata = md;
}

// Get the sampler metadata for this entry point
if (auto* md = m_program->sampler_metadata(m_name)) {
m_sampler_metadata = md;
}

// Get a pointer to the arguments from the program
auto args = m_program->args_for_kernel(m_name);

Expand Down Expand Up @@ -1975,32 +1994,49 @@ cl_int cvk_entry_point::init() {
m_pod_buffer_size = round_up(m_pod_buffer_size, 4);
}

// Take the size of image metadata into account for the pod buffer size
if (m_image_metadata) {
// Find how big the POD buffer should be
// Take the size of image & sampler metadata into account for the pod buffer
// size
{
uint32_t max_offset = 0;
for (const auto& md : *m_image_metadata) {
auto order_offset = md.second.order_offset;
auto data_type_offset = md.second.data_type_offset;
if (md.second.has_valid_order()) {
max_offset = std::max(order_offset, max_offset);
push_constant_range.offset =
std::min(order_offset, push_constant_range.offset);
if (order_offset + sizeof(uint32_t) >
push_constant_range.offset + push_constant_range.size) {
push_constant_range.size = order_offset + sizeof(uint32_t) -
push_constant_range.offset;
if (m_image_metadata) {
// Find how big the POD buffer should be
for (const auto& md : *m_image_metadata) {
auto order_offset = md.second.order_offset;
auto data_type_offset = md.second.data_type_offset;
if (md.second.has_valid_order()) {
max_offset = std::max(order_offset, max_offset);
push_constant_range.offset =
std::min(order_offset, push_constant_range.offset);
if (order_offset + sizeof(uint32_t) >
push_constant_range.offset + push_constant_range.size) {
push_constant_range.size = order_offset +
sizeof(uint32_t) -
push_constant_range.offset;
}
}
if (md.second.has_valid_data_type()) {
max_offset = std::max(data_type_offset, max_offset);
push_constant_range.offset =
std::min(data_type_offset, push_constant_range.offset);
if (data_type_offset + sizeof(uint32_t) >
push_constant_range.offset + push_constant_range.size) {
push_constant_range.size = data_type_offset +
sizeof(uint32_t) -
push_constant_range.offset;
}
}
}
if (md.second.has_valid_data_type()) {
max_offset = std::max(data_type_offset, max_offset);
}
if (m_sampler_metadata) {
for (const auto& md : *m_sampler_metadata) {
auto offset = md.second;
max_offset = std::max(offset, max_offset);
push_constant_range.offset =
std::min(data_type_offset, push_constant_range.offset);
if (data_type_offset + sizeof(uint32_t) >
std::min(offset, push_constant_range.offset);
if (offset + sizeof(uint32_t) >
push_constant_range.offset + push_constant_range.size) {
push_constant_range.size = data_type_offset +
sizeof(uint32_t) -
push_constant_range.offset;
push_constant_range.size =
offset + sizeof(uint32_t) - push_constant_range.offset;
}
}
}
Expand Down
Loading

0 comments on commit b65b365

Please sign in to comment.