Skip to content

Commit

Permalink
fix support for read of 3D images with unnormalised sampler
Browse files Browse the repository at this point in the history 
This PR depends on google/clspv#1234

We parse the new NormalizedSamplerMaskPushConstant non-semantic
instruction from KhronosGroup/SPIRV-Headers#377.

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.
  • Loading branch information
@rjodinchr
rjodinchr committed Sep 20, 2023
1 parent e16b5df commit 77c3c0e
Show file tree
Hide file tree
Showing 8 changed files with 219 additions and 31 deletions.
42 changes: 41 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,26 @@ 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);
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 @@ -123,6 +149,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 @@ -264,7 +294,17 @@ 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 sampler_norm = false;
if (auto md = m_entry_point->sampler_metadata()) {
sampler_norm = md->find(i) != md->end();
}
auto sampler = sampler_norm && !clsampler->normalized_coords()
? clsampler->vulkan_sampler_norm()
: 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
7 changes: 4 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 norm) {
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 || norm) {
unnormalized_coordinates = VK_FALSE;
} else {
unnormalized_coordinates = VK_TRUE;
Expand Down Expand Up @@ -235,7 +235,8 @@ bool cvk_sampler::init() {
unnormalized_coordinates, // unnormalizedCoordinates
};

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

return (res == VK_SUCCESS);
}
Expand Down
18 changes: 16 additions & 2 deletions src/memory.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,13 +428,18 @@ 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() {
if (m_sampler != VK_NULL_HANDLE) {
auto vkdev = context()->device()->vulkan_device();
vkDestroySampler(vkdev, m_sampler, nullptr);
}
if (m_sampler_norm != VK_NULL_HANDLE) {
auto vkdev = context()->device()->vulkan_device();
vkDestroySampler(vkdev, m_sampler_norm, nullptr);
}
}

static cvk_sampler* create(cvk_context* context, bool normalized_coords,
Expand All @@ -453,17 +458,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 vulkan_sampler_norm() {
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 norm = 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 @@ -151,6 +151,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 @@ -208,6 +210,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 @@ -1668,8 +1681,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 @@ -1883,6 +1897,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 @@ -1963,32 +1982,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 77c3c0e

Please sign in to comment.