[PATCH vkd3d 1/5] vkd3d: Index the "vk_uav_counter_views" array to match the "uav_counters" array.
That is, index elements of vk_uav_counter_views by their index in uav_counters, rather than by their register index. This is necessary because multiple UAVs can share the same register index but differ in register space. Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d/command.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c index 8a7ff66..7c6b758 100644 --- a/libs/vkd3d/command.c +++ b/libs/vkd3d/command.c @@ -2653,9 +2653,10 @@ static void d3d12_command_list_update_descriptor_table(struct d3d12_command_list const struct vkd3d_vk_device_procs *vk_procs = &list->device->vk_procs; struct VkDescriptorImageInfo image_infos[24], *current_image_info; const struct d3d12_root_descriptor_table *descriptor_table; + const struct d3d12_pipeline_state *state = list->state; const struct d3d12_root_descriptor_table_range *range; VkDevice vk_device = list->device->vk_device; - unsigned int i, j, descriptor_count; + unsigned int i, j, k, descriptor_count; struct d3d12_desc *descriptor; descriptor_table = root_signature_get_descriptor_table(root_signature, index); @@ -2678,14 +2679,20 @@ static void d3d12_command_list_update_descriptor_table(struct d3d12_command_list unsigned int register_idx = range->base_register_idx + j; /* Track UAV counters. */ - if (range->descriptor_magic == VKD3D_DESCRIPTOR_MAGIC_UAV - && register_idx < ARRAY_SIZE(bindings->vk_uav_counter_views)) + if (range->descriptor_magic == VKD3D_DESCRIPTOR_MAGIC_UAV) { - VkBufferView vk_counter_view = descriptor->magic == VKD3D_DESCRIPTOR_MAGIC_UAV - ? descriptor->u.view->vk_counter_view : VK_NULL_HANDLE; - if (bindings->vk_uav_counter_views[register_idx] != vk_counter_view) - bindings->uav_counter_dirty_mask |= 1u << register_idx; - bindings->vk_uav_counter_views[register_idx] = vk_counter_view; + for (k = 0; k < vkd3d_popcount(state->uav_counter_mask); ++k) + { + if (state->uav_counters[k].register_index == register_idx) + { + VkBufferView vk_counter_view = descriptor->magic == VKD3D_DESCRIPTOR_MAGIC_UAV + ? descriptor->u.view->vk_counter_view : VK_NULL_HANDLE; + if (bindings->vk_uav_counter_views[k] != vk_counter_view) + bindings->uav_counter_dirty_mask |= 1u << k; + bindings->vk_uav_counter_views[k] = vk_counter_view; + break; + } + } } if (!vk_write_descriptor_set_from_d3d12_desc(current_descriptor_write, @@ -2826,7 +2833,7 @@ static void d3d12_command_list_update_uav_counter_descriptors(struct d3d12_comma unsigned int uav_counter_count; unsigned int i; - if (!state || !(state->uav_counter_mask & bindings->uav_counter_dirty_mask)) + if (!state || !bindings->uav_counter_dirty_mask) return; uav_counter_count = vkd3d_popcount(state->uav_counter_mask); @@ -2841,7 +2848,7 @@ static void d3d12_command_list_update_uav_counter_descriptors(struct d3d12_comma const struct vkd3d_shader_uav_counter_binding *uav_counter = &state->uav_counters[i]; const VkBufferView *vk_uav_counter_views = bindings->vk_uav_counter_views; - assert(vk_uav_counter_views[uav_counter->register_index]); + assert(vk_uav_counter_views[i]); vk_descriptor_writes[i].sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET; vk_descriptor_writes[i].pNext = NULL; @@ -2852,7 +2859,7 @@ static void d3d12_command_list_update_uav_counter_descriptors(struct d3d12_comma vk_descriptor_writes[i].descriptorType = VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER; vk_descriptor_writes[i].pImageInfo = NULL; vk_descriptor_writes[i].pBufferInfo = NULL; - vk_descriptor_writes[i].pTexelBufferView = &vk_uav_counter_views[uav_counter->register_index]; + vk_descriptor_writes[i].pTexelBufferView = &vk_uav_counter_views[i]; } VK_CALL(vkUpdateDescriptorSets(vk_device, uav_counter_count, vk_descriptor_writes, 0, NULL)); -- 2.26.2
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d/command.c | 6 +++--- libs/vkd3d/state.c | 8 ++++---- libs/vkd3d/vkd3d_private.h | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c index 7c6b758..0903a7f 100644 --- a/libs/vkd3d/command.c +++ b/libs/vkd3d/command.c @@ -1865,7 +1865,7 @@ static void d3d12_command_list_invalidate_bindings(struct d3d12_command_list *li if (!state) return; - if (state->uav_counter_mask) + if (state->uav_counter_count) { struct vkd3d_pipeline_bindings *bindings = &list->pipeline_bindings[state->vk_bind_point]; bindings->uav_counter_dirty_mask = ~(uint8_t)0; @@ -2681,7 +2681,7 @@ static void d3d12_command_list_update_descriptor_table(struct d3d12_command_list /* Track UAV counters. */ if (range->descriptor_magic == VKD3D_DESCRIPTOR_MAGIC_UAV) { - for (k = 0; k < vkd3d_popcount(state->uav_counter_mask); ++k) + for (k = 0; k < state->uav_counter_count; ++k) { if (state->uav_counters[k].register_index == register_idx) { @@ -2836,7 +2836,7 @@ static void d3d12_command_list_update_uav_counter_descriptors(struct d3d12_comma if (!state || !bindings->uav_counter_dirty_mask) return; - uav_counter_count = vkd3d_popcount(state->uav_counter_mask); + uav_counter_count = state->uav_counter_count; assert(uav_counter_count <= ARRAY_SIZE(vk_descriptor_writes)); vk_descriptor_set = d3d12_command_allocator_allocate_descriptor_set(list->allocator, state->vk_set_layout); diff --git a/libs/vkd3d/state.c b/libs/vkd3d/state.c index a7b6251..44891a1 100644 --- a/libs/vkd3d/state.c +++ b/libs/vkd3d/state.c @@ -1418,7 +1418,7 @@ static HRESULT d3d12_pipeline_state_init_compute_uav_counters(struct d3d12_pipel vkd3d_free(binding_desc); return E_OUTOFMEMORY; } - state->uav_counter_mask = shader_info->uav_counter_mask; + state->uav_counter_count = uav_counter_count; memset(&context, 0, sizeof(context)); if (root_signature->vk_push_set_layout) @@ -1495,7 +1495,7 @@ static HRESULT d3d12_pipeline_state_init_compute(struct d3d12_pipeline_state *st state->vk_pipeline_layout = VK_NULL_HANDLE; state->vk_set_layout = VK_NULL_HANDLE; state->uav_counters = NULL; - state->uav_counter_mask = 0; + state->uav_counter_count = 0; if (!(root_signature = unsafe_impl_from_ID3D12RootSignature(desc->pRootSignature))) { @@ -1529,7 +1529,7 @@ static HRESULT d3d12_pipeline_state_init_compute(struct d3d12_pipeline_state *st shader_interface.combined_samplers = NULL; shader_interface.combined_sampler_count = 0; shader_interface.uav_counters = state->uav_counters; - shader_interface.uav_counter_count = vkd3d_popcount(state->uav_counter_mask); + shader_interface.uav_counter_count = state->uav_counter_count; vk_pipeline_layout = state->vk_pipeline_layout ? state->vk_pipeline_layout : root_signature->vk_pipeline_layout; @@ -2041,7 +2041,7 @@ static HRESULT d3d12_pipeline_state_init_graphics(struct d3d12_pipeline_state *s state->vk_pipeline_layout = VK_NULL_HANDLE; state->vk_set_layout = VK_NULL_HANDLE; state->uav_counters = NULL; - state->uav_counter_mask = 0; + state->uav_counter_count = 0; graphics->stage_count = 0; memset(&input_signature, 0, sizeof(input_signature)); diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index bb53e48..0afc38d 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -797,7 +797,7 @@ struct d3d12_pipeline_state uint32_t set_index; struct vkd3d_shader_uav_counter_binding *uav_counters; - uint8_t uav_counter_mask; + unsigned int uav_counter_count; struct d3d12_device *device; -- 2.26.2
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d/command.c | 3 ++- libs/vkd3d/state.c | 1 + libs/vkd3d/vkd3d_private.h | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c index 0903a7f..90227ef 100644 --- a/libs/vkd3d/command.c +++ b/libs/vkd3d/command.c @@ -2683,7 +2683,8 @@ static void d3d12_command_list_update_descriptor_table(struct d3d12_command_list { for (k = 0; k < state->uav_counter_count; ++k) { - if (state->uav_counters[k].register_index == register_idx) + if (state->uav_counters[k].register_space == range->register_space + && state->uav_counters[k].register_index == register_idx) { VkBufferView vk_counter_view = descriptor->magic == VKD3D_DESCRIPTOR_MAGIC_UAV ? descriptor->u.view->vk_counter_view : VK_NULL_HANDLE; diff --git a/libs/vkd3d/state.c b/libs/vkd3d/state.c index 44891a1..11db8b5 100644 --- a/libs/vkd3d/state.c +++ b/libs/vkd3d/state.c @@ -646,6 +646,7 @@ static HRESULT d3d12_root_signature_init_root_descriptor_tables(struct d3d12_roo table->ranges[j].descriptor_count = range->NumDescriptors; table->ranges[j].binding = vk_binding; table->ranges[j].descriptor_magic = vkd3d_descriptor_magic_from_d3d12(range->RangeType); + table->ranges[j].register_space = range->RegisterSpace; table->ranges[j].base_register_idx = range->BaseShaderRegister; } } diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index 0afc38d..abb8b66 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -658,6 +658,7 @@ struct d3d12_root_descriptor_table_range uint32_t binding; uint32_t descriptor_magic; + unsigned int register_space; unsigned int base_register_idx; }; -- 2.26.2
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- include/vkd3d_shader.h | 10 ++++- libs/vkd3d-shader/spirv.c | 24 +++++++--- libs/vkd3d-shader/vkd3d_shader_main.c | 65 ++++++++++++++++++++++++--- libs/vkd3d/state.c | 26 +++++++---- 4 files changed, 102 insertions(+), 23 deletions(-) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index 67d4562..23c2cb7 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -539,13 +539,19 @@ struct vkd3d_versioned_root_signature_desc /* FIXME: Add support for 64 UAV bind slots. */ #define VKD3D_SHADER_MAX_UNORDERED_ACCESS_VIEWS 8 +struct vkd3d_uav_scan_info +{ + unsigned int register_space, register_index, id; + bool counter, read; +}; + struct vkd3d_shader_scan_info { enum vkd3d_shader_structure_type type; void *next; - unsigned int uav_read_mask; /* VKD3D_SHADER_MAX_UNORDERED_ACCESS_VIEWS */ - unsigned int uav_counter_mask; /* VKD3D_SHADER_MAX_UNORDERED_ACCESS_VIEWS */ + struct vkd3d_uav_scan_info uavs[VKD3D_SHADER_MAX_UNORDERED_ACCESS_VIEWS]; + unsigned int uav_count; unsigned int sampler_comparison_mode_mask; /* 16 */ bool use_vocp; }; diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 5a37b8f..ecca74c 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -5106,6 +5106,19 @@ static const struct vkd3d_spirv_resource_type *vkd3d_dxbc_compiler_enable_resour return resource_type_info; } +static const struct vkd3d_uav_scan_info *find_uav_scan_info( + const struct vkd3d_shader_scan_info *scan_info, unsigned int id) +{ + unsigned int i; + + for (i = 0; i < scan_info->uav_count; ++i) + { + if (scan_info->uavs[i].id == id) + return &scan_info->uavs[i]; + } + return NULL; +} + static SpvImageFormat image_format_for_image_read(enum vkd3d_component_type data_type) { /* The following formats are supported by Direct3D 11 hardware for UAV @@ -5130,14 +5143,13 @@ static uint32_t vkd3d_dxbc_compiler_get_image_type_id(struct vkd3d_dxbc_compiler const struct vkd3d_shader_register *reg, const struct vkd3d_spirv_resource_type *resource_type_info, enum vkd3d_component_type data_type, bool raw_structured, uint32_t depth) { - const struct vkd3d_shader_scan_info *scan_info = compiler->scan_info; + const struct vkd3d_uav_scan_info *uav_info = find_uav_scan_info(compiler->scan_info, reg->idx[0].offset); struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; uint32_t sampled_type_id; SpvImageFormat format; format = SpvImageFormatUnknown; - if (reg->type == VKD3DSPR_UAV - && (raw_structured || (scan_info->uav_read_mask & (1u << reg->idx[0].offset)))) + if (reg->type == VKD3DSPR_UAV && (raw_structured || uav_info->read)) format = image_format_for_image_read(data_type); sampled_type_id = vkd3d_spirv_get_type_id(builder, data_type, 1); @@ -5214,8 +5226,8 @@ static void vkd3d_dxbc_compiler_emit_resource_declaration(struct vkd3d_dxbc_comp enum vkd3d_shader_resource_type resource_type, enum vkd3d_data_type resource_data_type, unsigned int structure_stride, bool raw) { + const struct vkd3d_uav_scan_info *uav_info = find_uav_scan_info(compiler->scan_info, reg->idx[0].offset); uint32_t counter_type_id, type_id, ptr_type_id, var_id, counter_var_id = 0; - const struct vkd3d_shader_scan_info *scan_info = compiler->scan_info; struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; SpvStorageClass storage_class = SpvStorageClassUniformConstant; const struct vkd3d_spirv_resource_type *resource_type_info; @@ -5251,10 +5263,10 @@ static void vkd3d_dxbc_compiler_emit_resource_declaration(struct vkd3d_dxbc_comp vkd3d_dxbc_compiler_emit_register_debug_name(builder, var_id, reg); - if (is_uav && !(scan_info->uav_read_mask & (1u << reg->idx[0].offset))) + if (is_uav && !uav_info->read) vkd3d_spirv_build_op_decorate(builder, var_id, SpvDecorationNonReadable, NULL, 0); - if (is_uav && (scan_info->uav_counter_mask & (1u << reg->idx[0].offset))) + if (is_uav && uav_info->counter) { assert(structure_stride); /* counters are valid only for structured buffers */ diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index aa486cc..9ef84d4 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -22,9 +22,6 @@ VKD3D_DEBUG_ENV_NAME("VKD3D_SHADER_DEBUG"); -STATIC_ASSERT(MEMBER_SIZE(struct vkd3d_shader_scan_info, uav_counter_mask) * CHAR_BIT >= VKD3D_SHADER_MAX_UNORDERED_ACCESS_VIEWS); -STATIC_ASSERT(MEMBER_SIZE(struct vkd3d_shader_scan_info, uav_read_mask) * CHAR_BIT >= VKD3D_SHADER_MAX_UNORDERED_ACCESS_VIEWS); - static void vkd3d_shader_dump_blob(const char *path, const char *prefix, const void *data, size_t size) { static int shader_id = 0; @@ -205,11 +202,25 @@ static bool vkd3d_shader_instruction_is_uav_read(const struct vkd3d_shader_instr || (handler_idx == VKD3DSIH_LD_STRUCTURED && instruction->src[2].reg.type == VKD3DSPR_UAV); } +static struct vkd3d_uav_scan_info *find_uav_scan_info(struct vkd3d_shader_scan_info *scan_info, unsigned int id) +{ + unsigned int i; + + for (i = 0; i < scan_info->uav_count; ++i) + { + if (scan_info->uavs[i].id == id) + return &scan_info->uavs[i]; + } + return NULL; +} + static void vkd3d_shader_scan_record_uav_read(struct vkd3d_shader_scan_info *scan_info, const struct vkd3d_shader_register *reg) { - assert(reg->idx[0].offset < VKD3D_SHADER_MAX_UNORDERED_ACCESS_VIEWS); - scan_info->uav_read_mask |= 1u << reg->idx[0].offset; + struct vkd3d_uav_scan_info *uav_info = find_uav_scan_info(scan_info, reg->idx[0].offset); + + assert(uav_info); + uav_info->read = true; } static bool vkd3d_shader_instruction_is_uav_counter(const struct vkd3d_shader_instruction *instruction) @@ -222,8 +233,10 @@ static bool vkd3d_shader_instruction_is_uav_counter(const struct vkd3d_shader_in static void vkd3d_shader_scan_record_uav_counter(struct vkd3d_shader_scan_info *scan_info, const struct vkd3d_shader_register *reg) { - assert(reg->idx[0].offset < VKD3D_SHADER_MAX_UNORDERED_ACCESS_VIEWS); - scan_info->uav_counter_mask |= 1u << reg->idx[0].offset; + struct vkd3d_uav_scan_info *uav_info = find_uav_scan_info(scan_info, reg->idx[0].offset); + + assert(uav_info); + uav_info->counter = true; } static void vkd3d_shader_scan_input_declaration(struct vkd3d_shader_scan_info *scan_info, @@ -246,6 +259,39 @@ static void vkd3d_shader_scan_sampler_declaration(struct vkd3d_shader_scan_info } } +static void vkd3d_shader_scan_uav_declaration(struct vkd3d_shader_scan_info *scan_info, + const struct vkd3d_shader_instruction *instruction) +{ + unsigned int register_space, register_index, id; + struct vkd3d_uav_scan_info *uav_info; + + assert(scan_info->uav_count < VKD3D_SHADER_MAX_UNORDERED_ACCESS_VIEWS); + + if (instruction->handler_idx == VKD3DSIH_DCL_UAV_RAW) + { + register_space = instruction->declaration.raw_resource.register_space; + register_index = instruction->declaration.raw_resource.register_index; + id = instruction->declaration.raw_resource.dst.reg.idx[0].offset; + } + else if (instruction->handler_idx == VKD3DSIH_DCL_UAV_STRUCTURED) + { + register_space = instruction->declaration.structured_resource.register_space; + register_index = instruction->declaration.structured_resource.register_index; + id = instruction->declaration.structured_resource.reg.reg.idx[0].offset; + } + else /* VKD3DSIH_DCL_UAV_TYPED */ + { + register_space = instruction->declaration.semantic.register_space; + register_index = instruction->declaration.semantic.register_index; + id = instruction->declaration.semantic.reg.reg.idx[0].offset; + } + + uav_info = &scan_info->uavs[scan_info->uav_count++]; + uav_info->register_space = register_space; + uav_info->register_index = register_index; + uav_info->id = id; +} + static void vkd3d_shader_scan_instruction(struct vkd3d_shader_scan_info *scan_info, const struct vkd3d_shader_instruction *instruction) { @@ -259,6 +305,11 @@ static void vkd3d_shader_scan_instruction(struct vkd3d_shader_scan_info *scan_in case VKD3DSIH_DCL_SAMPLER: vkd3d_shader_scan_sampler_declaration(scan_info, instruction); break; + case VKD3DSIH_DCL_UAV_RAW: + case VKD3DSIH_DCL_UAV_STRUCTURED: + case VKD3DSIH_DCL_UAV_TYPED: + vkd3d_shader_scan_uav_declaration(scan_info, instruction); + break; default: break; } diff --git a/libs/vkd3d/state.c b/libs/vkd3d/state.c index 11db8b5..4bd4ef7 100644 --- a/libs/vkd3d/state.c +++ b/libs/vkd3d/state.c @@ -1405,11 +1405,17 @@ static HRESULT d3d12_pipeline_state_init_compute_uav_counters(struct d3d12_pipel struct vkd3d_descriptor_set_context context; VkDescriptorSetLayoutBinding *binding_desc; VkDescriptorSetLayout set_layouts[3]; - unsigned int uav_counter_count; + unsigned int uav_counter_count = 0; unsigned int i, j; HRESULT hr; - if (!(uav_counter_count = vkd3d_popcount(shader_info->uav_counter_mask))) + for (i = 0; i < shader_info->uav_count; ++i) + { + if (shader_info->uavs[i].counter) + ++uav_counter_count; + } + + if (!uav_counter_count) return S_OK; if (!(binding_desc = vkd3d_calloc(uav_counter_count, sizeof(*binding_desc)))) @@ -1427,13 +1433,13 @@ static HRESULT d3d12_pipeline_state_init_compute_uav_counters(struct d3d12_pipel if (root_signature->vk_set_layout) set_layouts[context.set_index++] = root_signature->vk_set_layout; - for (i = 0, j = 0; i < VKD3D_SHADER_MAX_UNORDERED_ACCESS_VIEWS; ++i) + for (i = 0, j = 0; i < shader_info->uav_count; ++i) { - if (!(shader_info->uav_counter_mask & (1u << i))) + if (!shader_info->uavs[i].counter) continue; - state->uav_counters[j].register_space = 0; - state->uav_counters[j].register_index = i; + state->uav_counters[j].register_space = shader_info->uavs[i].register_space; + state->uav_counters[j].register_index = shader_info->uavs[i].register_index; state->uav_counters[j].shader_visibility = VKD3D_SHADER_VISIBILITY_COMPUTE; state->uav_counters[j].binding.set = context.set_index; state->uav_counters[j].binding.binding = context.descriptor_binding; @@ -2256,8 +2262,12 @@ static HRESULT d3d12_pipeline_state_init_graphics(struct d3d12_pipeline_state *s hr = hresult_from_vkd3d_result(ret); goto fail; } - if (shader_info.uav_counter_mask) - FIXME("UAV counters not implemented for graphics pipelines.\n"); + + for (j = 0; j < shader_info.uav_count; ++j) + { + if (shader_info.uavs[j].counter) + FIXME("UAV counters not implemented for graphics pipelines.\n"); + } compile_args = NULL; switch (shader_stages[i].stage) -- 2.26.2
On Thu, 4 Jun 2020 at 06:20, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
+struct vkd3d_uav_scan_info +{ + unsigned int register_space, register_index, id; + bool counter, read; +}; + It seems tempting to store the "counter" and "read" fields as a "flags" field instead. For what it's worth, note that I also have patches in this area in my own tree; we'll need to store information about the other descriptor types in the vkd3d_shader_scan_info at some point as well.
I'm not sure whether there's a use for the "id" field in the public API. I'm inclined to say lookup by ID should go through the "symbol_table" tree. Perhaps more important is the question how this would work with ranges larger than a single descriptor. I.e., how would for example a "dcl_uav_raw u0[1:3], ..." declaration be represented in the vkd3d_shader_scan_info structure?
On 6/4/20 3:48 PM, Henri Verbeet wrote:
On Thu, 4 Jun 2020 at 06:20, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
+struct vkd3d_uav_scan_info +{ + unsigned int register_space, register_index, id; + bool counter, read; +}; + It seems tempting to store the "counter" and "read" fields as a "flags" field instead. For what it's worth, note that I also have patches in this area in my own tree; we'll need to store information about the other descriptor types in the vkd3d_shader_scan_info at some point as well.
Sure, that makes sense.
I'm not sure whether there's a use for the "id" field in the public API. I'm inclined to say lookup by ID should go through the "symbol_table" tree.
Yes, I don't see a reason to expose "id". Is the idea then to introduce a new symbol table in struct vkd3d_shader_parser? It seems a bit unfortunate that the table would be duplicated, but after all I guess it's not a lot of duplication...
Perhaps more important is the question how this would work with ranges larger than a single descriptor. I.e., how would for example a "dcl_uav_raw u0[1:3], ..." declaration be represented in the vkd3d_shader_scan_info structure?
It's hard for me to answer that without a full perspective as to how the indices are or might be used, but my best guess is that, if descriptor ranges are uploaded all at once, we'd want to track a single used/counter flag for the whole range.
On Fri, 5 Jun 2020 at 01:39, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
On 6/4/20 3:48 PM, Henri Verbeet wrote:
On Thu, 4 Jun 2020 at 06:20, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
+struct vkd3d_uav_scan_info +{ + unsigned int register_space, register_index, id; + bool counter, read; +}; + It seems tempting to store the "counter" and "read" fields as a "flags" field instead. For what it's worth, note that I also have patches in this area in my own tree; we'll need to store information about the other descriptor types in the vkd3d_shader_scan_info at some point as well.
Sure, that makes sense.
I'm not sure whether there's a use for the "id" field in the public API. I'm inclined to say lookup by ID should go through the "symbol_table" tree.
Yes, I don't see a reason to expose "id".
Is the idea then to introduce a new symbol table in struct vkd3d_shader_parser? It seems a bit unfortunate that the table would be duplicated, but after all I guess it's not a lot of duplication...
No, the idea would be that in spirv.c you'd use the symbol table to lookup the range information for the ID, and then use that information to lookup the counter/read information. E.g.: dcl_uav_typed u0[1:3], ..., space=4 ... store_uav_typed u0[1], ... The store_uav_typed handler would lookup u0 in the symbol table and find that it corresponds to [1:3] in register space 4. Index 1 in that range would then be descriptor 2 in that register space 4. It would then lookup the information for descriptor 2 in register space 4 in the vkd3d_shader_scan_info to check whether that's a readonly UAV. (There are of course variants on that, like doing the lookup when inserting u0 in the symbol table, etc. To some extent it also depends on what we do with the issue below.)
Perhaps more important is the question how this would work with ranges larger than a single descriptor. I.e., how would for example a "dcl_uav_raw u0[1:3], ..." declaration be represented in the vkd3d_shader_scan_info structure?
It's hard for me to answer that without a full perspective as to how the indices are or might be used, but my best guess is that, if descriptor ranges are uploaded all at once, we'd want to track a single used/counter flag for the whole range.
It probably needs tests to determine whether it's allowed in practice, but the bytecode seems to allow for overlapping ranges. E.g.: dcl_uav_raw u0[1:4], ..., space=0 dcl_uav_raw u1[3:6], ..., space=0 would make both u0[2] and u1[0] refer to descriptor 3 in register space 0. But even without aliasing, it's not clear to me that e.g. using the counter for u0[0] necessarily implies that all of u0 needs one.
On 6/4/20 5:04 PM, Henri Verbeet wrote:
On Fri, 5 Jun 2020 at 01:39, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
On 6/4/20 3:48 PM, Henri Verbeet wrote:
On Thu, 4 Jun 2020 at 06:20, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
+struct vkd3d_uav_scan_info +{ + unsigned int register_space, register_index, id; + bool counter, read; +}; + It seems tempting to store the "counter" and "read" fields as a "flags" field instead. For what it's worth, note that I also have patches in this area in my own tree; we'll need to store information about the other descriptor types in the vkd3d_shader_scan_info at some point as well.
Sure, that makes sense.
I'm not sure whether there's a use for the "id" field in the public API. I'm inclined to say lookup by ID should go through the "symbol_table" tree.
Yes, I don't see a reason to expose "id".
Is the idea then to introduce a new symbol table in struct vkd3d_shader_parser? It seems a bit unfortunate that the table would be duplicated, but after all I guess it's not a lot of duplication...
No, the idea would be that in spirv.c you'd use the symbol table to lookup the range information for the ID, and then use that information to lookup the counter/read information. E.g.:
dcl_uav_typed u0[1:3], ..., space=4 ... store_uav_typed u0[1], ...
The store_uav_typed handler would lookup u0 in the symbol table and find that it corresponds to [1:3] in register space 4. Index 1 in that range would then be descriptor 2 in that register space 4. It would then lookup the information for descriptor 2 in register space 4 in the vkd3d_shader_scan_info to check whether that's a readonly UAV. (There are of course variants on that, like doing the lookup when inserting u0 in the symbol table, etc. To some extent it also depends on what we do with the issue below.)
I'm probably misunderstanding something, but how would we find the register space and index in vkd3d_shader_scan_record_uav_read()?
Perhaps more important is the question how this would work with ranges larger than a single descriptor. I.e., how would for example a "dcl_uav_raw u0[1:3], ..." declaration be represented in the vkd3d_shader_scan_info structure?
It's hard for me to answer that without a full perspective as to how the indices are or might be used, but my best guess is that, if descriptor ranges are uploaded all at once, we'd want to track a single used/counter flag for the whole range.
It probably needs tests to determine whether it's allowed in practice, but the bytecode seems to allow for overlapping ranges. E.g.:
dcl_uav_raw u0[1:4], ..., space=0 dcl_uav_raw u1[3:6], ..., space=0
would make both u0[2] and u1[0] refer to descriptor 3 in register space 0.
In which case I guess we'd mark both ranges as read if descriptor 3 is read from.
But even without aliasing, it's not clear to me that e.g. using the counter for u0[0] necessarily implies that all of u0 needs one.
Yes, certainly, it's more conservative than is probably desirable, especially as I now examine spirv.c a little closer. Of course, I also don't know how much of that is redundancy and how much is a performance concern. While using e.g. a bitmask may be reasonable, I'm not sure how to reconcile that with unbounded or even particularly large descriptor arrays. Perhaps cutting off the bitmask size at a point makes sense (and treating anything further as "always used"?), but I don't have the experience to guess what a reasonable cutoff point is.
On Fri, 5 Jun 2020 at 02:55, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
Is the idea then to introduce a new symbol table in struct vkd3d_shader_parser? It seems a bit unfortunate that the table would be duplicated, but after all I guess it's not a lot of duplication...
No, the idea would be that in spirv.c you'd use the symbol table to lookup the range information for the ID, and then use that information to lookup the counter/read information. E.g.:
dcl_uav_typed u0[1:3], ..., space=4 ... store_uav_typed u0[1], ...
The store_uav_typed handler would lookup u0 in the symbol table and find that it corresponds to [1:3] in register space 4. Index 1 in that range would then be descriptor 2 in that register space 4. It would then lookup the information for descriptor 2 in register space 4 in the vkd3d_shader_scan_info to check whether that's a readonly UAV. (There are of course variants on that, like doing the lookup when inserting u0 in the symbol table, etc. To some extent it also depends on what we do with the issue below.)
I'm probably misunderstanding something, but how would we find the register space and index in vkd3d_shader_scan_record_uav_read()?
It would need some kind of temporary lookup structure in vkd3d_shader_scan_dxbc(), but I don't think that needs to be a duplicate of the symbol table.
Perhaps more important is the question how this would work with ranges larger than a single descriptor. I.e., how would for example a "dcl_uav_raw u0[1:3], ..." declaration be represented in the vkd3d_shader_scan_info structure?
It's hard for me to answer that without a full perspective as to how the indices are or might be used, but my best guess is that, if descriptor ranges are uploaded all at once, we'd want to track a single used/counter flag for the whole range.
It probably needs tests to determine whether it's allowed in practice, but the bytecode seems to allow for overlapping ranges. E.g.:
dcl_uav_raw u0[1:4], ..., space=0 dcl_uav_raw u1[3:6], ..., space=0
would make both u0[2] and u1[0] refer to descriptor 3 in register space 0.
In which case I guess we'd mark both ranges as read if descriptor 3 is read from.
But even without aliasing, it's not clear to me that e.g. using the counter for u0[0] necessarily implies that all of u0 needs one.
Yes, certainly, it's more conservative than is probably desirable, especially as I now examine spirv.c a little closer. Of course, I also don't know how much of that is redundancy and how much is a performance concern.
While using e.g. a bitmask may be reasonable, I'm not sure how to reconcile that with unbounded or even particularly large descriptor arrays. Perhaps cutting off the bitmask size at a point makes sense (and treating anything further as "always used"?), but I don't have the experience to guess what a reasonable cutoff point is.
My main concern would be that if we require an unnecessary UAV counter descriptor that the application doesn't actually have, we'd fail to compile the shader. Perhaps that's impossible, but we'd want to be sure of that before designing the API in a way that makes resolving that issue impossible.
On 6/5/20 6:47 AM, Henri Verbeet wrote:
Perhaps more important is the question how this would work with ranges larger than a single descriptor. I.e., how would for example a "dcl_uav_raw u0[1:3], ..." declaration be represented in the vkd3d_shader_scan_info structure?
It's hard for me to answer that without a full perspective as to how the indices are or might be used, but my best guess is that, if descriptor ranges are uploaded all at once, we'd want to track a single used/counter flag for the whole range.
It probably needs tests to determine whether it's allowed in practice, but the bytecode seems to allow for overlapping ranges. E.g.:
dcl_uav_raw u0[1:4], ..., space=0 dcl_uav_raw u1[3:6], ..., space=0
would make both u0[2] and u1[0] refer to descriptor 3 in register space 0.
In which case I guess we'd mark both ranges as read if descriptor 3 is read from.
But even without aliasing, it's not clear to me that e.g. using the counter for u0[0] necessarily implies that all of u0 needs one.
Yes, certainly, it's more conservative than is probably desirable, especially as I now examine spirv.c a little closer. Of course, I also don't know how much of that is redundancy and how much is a performance concern.
While using e.g. a bitmask may be reasonable, I'm not sure how to reconcile that with unbounded or even particularly large descriptor arrays. Perhaps cutting off the bitmask size at a point makes sense (and treating anything further as "always used"?), but I don't have the experience to guess what a reasonable cutoff point is.
My main concern would be that if we require an unnecessary UAV counter descriptor that the application doesn't actually have, we'd fail to compile the shader. Perhaps that's impossible, but we'd want to be sure of that before designing the API in a way that makes resolving that issue impossible.
If we extend the above proposal, perhaps something like: #define VKD3D_ARRAY_COUNT_UNBOUNDED (~0) struct vkd3d_uav_scan_info { unsigned int register_space, register_index, array_count; bool counter, read; } array_count doesn't necessarily have to reflect the actual declared array count (unless there's a reason to do that I'm not thinking of?) Thus if the array isn't indexed dynamically, we can fill one vkd3d_uav_scan_info entry for each access, each with an array_count of 1. If it is, we add one entry with the declared array count or VKD3D_ARRAY_COUNT_UNBOUNDED (if it's indexed dynamically). Does that work?
On Fri, 5 Jun 2020 at 20:04, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
If we extend the above proposal, perhaps something like:
#define VKD3D_ARRAY_COUNT_UNBOUNDED (~0)
struct vkd3d_uav_scan_info { unsigned int register_space, register_index, array_count; bool counter, read; }
array_count doesn't necessarily have to reflect the actual declared array count (unless there's a reason to do that I'm not thinking of?)
Thus if the array isn't indexed dynamically, we can fill one vkd3d_uav_scan_info entry for each access, each with an array_count of 1. If it is, we add one entry with the declared array count or VKD3D_ARRAY_COUNT_UNBOUNDED (if it's indexed dynamically). Does that work?
Probably something along those lines, yeah. Although perhaps it makes more sense to store "unbounded" as a flag, like "counter" and "read", and then just set "count" to 0.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- tests/d3d12.c | 48 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/tests/d3d12.c b/tests/d3d12.c index c4ec41a..ec7be9d 100644 --- a/tests/d3d12.c +++ b/tests/d3d12.c @@ -32825,7 +32825,7 @@ static void test_write_buffer_immediate(void) static void test_register_space(void) { - ID3D12Resource *input_buffers[6], *output_buffers[8]; + ID3D12Resource *input_buffers[6], *output_buffers[8], *counter_buffers[2]; D3D12_ROOT_SIGNATURE_DESC root_signature_desc; D3D12_UNORDERED_ACCESS_VIEW_DESC uav_desc; D3D12_SHADER_RESOURCE_VIEW_DESC srv_desc; @@ -32834,6 +32834,7 @@ static void test_register_space(void) ID3D12DescriptorHeap *heap; ID3D12CommandQueue *queue; ID3D12Device *device; + uint32_t counter; unsigned int i; HRESULT hr; @@ -32907,11 +32908,13 @@ static void test_register_space(void) u6[0] = t6.Load(0); u7[0] = c1; u8[0] = c2; + u7.IncrementCounter(); + u8.DecrementCounter(); } #endif - 0x43425844, 0x073b3de1, 0x527d26fc, 0xa46f417c, 0x4d9f338c, 0x00000001, 0x00000444, 0x00000003, + 0x43425844, 0x2a87323a, 0x4f83d345, 0x707e5d14, 0xdf41ce4a, 0x00000001, 0x00000474, 0x00000003, 0x0000002c, 0x0000003c, 0x0000004c, 0x4e475349, 0x00000008, 0x00000000, 0x00000008, 0x4e47534f, - 0x00000008, 0x00000000, 0x00000008, 0x58454853, 0x000003f0, 0x00050051, 0x000000fc, 0x0100086a, + 0x00000008, 0x00000000, 0x00000008, 0x58454853, 0x00000420, 0x00050051, 0x00000108, 0x0100086a, 0x07000059, 0x00308e46, 0x00000000, 0x00000001, 0x00000001, 0x00000001, 0x00000001, 0x07000059, 0x00308e46, 0x00000001, 0x00000001, 0x00000001, 0x00000001, 0x00000002, 0x060000a1, 0x00307e46, 0x00000000, 0x00000001, 0x00000001, 0x00000001, 0x07000858, 0x00307e46, 0x00000001, 0x00000002, @@ -32923,8 +32926,8 @@ static void test_register_space(void) 0x00000002, 0x00000001, 0x0700089c, 0x0031ee46, 0x00000002, 0x00000003, 0x00000003, 0x00004444, 0x00000001, 0x0600009d, 0x0031ee46, 0x00000003, 0x00000001, 0x00000001, 0x00000002, 0x0600009d, 0x0031ee46, 0x00000004, 0x00000002, 0x00000002, 0x00000002, 0x0700089c, 0x0031ee46, 0x00000005, - 0x00000003, 0x00000003, 0x00004444, 0x00000002, 0x0700009e, 0x0031ee46, 0x00000006, 0x00000003, - 0x00000003, 0x00000004, 0x00000003, 0x0700009e, 0x0031ee46, 0x00000007, 0x00000003, 0x00000003, + 0x00000003, 0x00000003, 0x00004444, 0x00000002, 0x0780009e, 0x0031ee46, 0x00000006, 0x00000003, + 0x00000003, 0x00000004, 0x00000003, 0x0780009e, 0x0031ee46, 0x00000007, 0x00000003, 0x00000003, 0x00000004, 0x00000004, 0x02000068, 0x00000001, 0x0400009b, 0x00000001, 0x00000001, 0x00000001, 0x080000a5, 0x00100012, 0x00000000, 0x00004001, 0x00000000, 0x00207006, 0x00000003, 0x00000001, 0x080000a6, 0x0021e012, 0x00000000, 0x00000001, 0x00004001, 0x00000000, 0x0010000a, 0x00000000, @@ -32943,7 +32946,8 @@ static void test_register_space(void) 0x0c0000a8, 0x0021e012, 0x00000007, 0x00000003, 0x00004001, 0x00000000, 0x00004001, 0x00000000, 0x0030800a, 0x00000000, 0x00000001, 0x00000000, 0x0c0000a8, 0x0021e012, 0x00000006, 0x00000003, 0x00004001, 0x00000000, 0x00004001, 0x00000000, 0x0030800a, 0x00000001, 0x00000001, 0x00000000, - 0x0100003e, + 0x060000b2, 0x00100012, 0x00000000, 0x0021e000, 0x00000007, 0x00000003, 0x060000b3, 0x00100012, + 0x00000000, 0x0021e000, 0x00000006, 0x00000003, 0x0100003e, }; static const uint32_t srv_data[] = {100, 200, 300, 400, 500, 600}; @@ -32980,18 +32984,37 @@ static void test_register_space(void) get_cpu_descriptor_handle(&context, heap, i)); } + for (i = 0; i < ARRAY_SIZE(counter_buffers); ++i) + { + counter_buffers[i] = create_default_buffer(device, sizeof(uint32_t), + D3D12_RESOURCE_FLAG_ALLOW_UNORDERED_ACCESS, D3D12_RESOURCE_STATE_COPY_DEST); + counter = 2; + upload_buffer_data(counter_buffers[i], 0, sizeof(counter), &counter, queue, command_list); + reset_command_list(command_list, context.allocator); + transition_sub_resource_state(command_list, counter_buffers[i], 0, + D3D12_RESOURCE_STATE_COPY_DEST, D3D12_RESOURCE_STATE_UNORDERED_ACCESS); + } + for (i = 0; i < ARRAY_SIZE(output_buffers); ++i) { output_buffers[i] = create_default_buffer(device, sizeof(uint32_t), D3D12_RESOURCE_FLAG_ALLOW_UNORDERED_ACCESS, D3D12_RESOURCE_STATE_UNORDERED_ACCESS); memset(&uav_desc, 0, sizeof(uav_desc)); - uav_desc.Format = DXGI_FORMAT_R32_UINT; uav_desc.ViewDimension = D3D12_UAV_DIMENSION_BUFFER; uav_desc.Buffer.FirstElement = 0; uav_desc.Buffer.NumElements = 1; - ID3D12Device_CreateUnorderedAccessView(device, output_buffers[i], NULL, &uav_desc, - get_cpu_descriptor_handle(&context, heap, ARRAY_SIZE(input_buffers) + i)); + if (i < 6) + { + uav_desc.Format = DXGI_FORMAT_R32_UINT; + } + else + { + uav_desc.Format = DXGI_FORMAT_UNKNOWN; + uav_desc.Buffer.StructureByteStride = sizeof(uint32_t); + } + ID3D12Device_CreateUnorderedAccessView(device, output_buffers[i], i >= 6 ? counter_buffers[i - 6] : NULL, + &uav_desc, get_cpu_descriptor_handle(&context, heap, ARRAY_SIZE(input_buffers) + i)); } context.pipeline_state = create_compute_pipeline_state(device, context.root_signature, @@ -33014,10 +33037,17 @@ static void test_register_space(void) reset_command_list(command_list, context.allocator); } + counter = read_uav_counter(&context, counter_buffers[0], 0); + ok(counter == 3, "Got unexpected value %d.\n", counter); + counter = read_uav_counter(&context, counter_buffers[1], 0); + ok(counter == 1, "Got unexpected value %d.\n", counter); + for (i = 0; i < ARRAY_SIZE(input_buffers); ++i) ID3D12Resource_Release(input_buffers[i]); for (i = 0; i < ARRAY_SIZE(output_buffers); ++i) ID3D12Resource_Release(output_buffers[i]); + for (i = 0; i < ARRAY_SIZE(counter_buffers); ++i) + ID3D12Resource_Release(counter_buffers[i]); ID3D12DescriptorHeap_Release(heap); destroy_test_context(&context); } -- 2.26.2
On Thu, 4 Jun 2020 at 06:19, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
@@ -2678,14 +2679,20 @@ static void d3d12_command_list_update_descriptor_table(struct d3d12_command_list unsigned int register_idx = range->base_register_idx + j;
/* Track UAV counters. */ - if (range->descriptor_magic == VKD3D_DESCRIPTOR_MAGIC_UAV - && register_idx < ARRAY_SIZE(bindings->vk_uav_counter_views)) + if (range->descriptor_magic == VKD3D_DESCRIPTOR_MAGIC_UAV) { - VkBufferView vk_counter_view = descriptor->magic == VKD3D_DESCRIPTOR_MAGIC_UAV - ? descriptor->u.view->vk_counter_view : VK_NULL_HANDLE; - if (bindings->vk_uav_counter_views[register_idx] != vk_counter_view) - bindings->uav_counter_dirty_mask |= 1u << register_idx; - bindings->vk_uav_counter_views[register_idx] = vk_counter_view; + for (k = 0; k < vkd3d_popcount(state->uav_counter_mask); ++k) + { + if (state->uav_counters[k].register_index == register_idx) + { + VkBufferView vk_counter_view = descriptor->magic == VKD3D_DESCRIPTOR_MAGIC_UAV + ? descriptor->u.view->vk_counter_view : VK_NULL_HANDLE; + if (bindings->vk_uav_counter_views[k] != vk_counter_view) + bindings->uav_counter_dirty_mask |= 1u << k;
Note that after this patch, "uav_counter_dirty_mask" isn't really being used as a mask anymore.
On 6/4/20 3:48 PM, Henri Verbeet wrote:
On Thu, 4 Jun 2020 at 06:19, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
@@ -2678,14 +2679,20 @@ static void d3d12_command_list_update_descriptor_table(struct d3d12_command_list unsigned int register_idx = range->base_register_idx + j;
/* Track UAV counters. */ - if (range->descriptor_magic == VKD3D_DESCRIPTOR_MAGIC_UAV - && register_idx < ARRAY_SIZE(bindings->vk_uav_counter_views)) + if (range->descriptor_magic == VKD3D_DESCRIPTOR_MAGIC_UAV) { - VkBufferView vk_counter_view = descriptor->magic == VKD3D_DESCRIPTOR_MAGIC_UAV - ? descriptor->u.view->vk_counter_view : VK_NULL_HANDLE; - if (bindings->vk_uav_counter_views[register_idx] != vk_counter_view) - bindings->uav_counter_dirty_mask |= 1u << register_idx; - bindings->vk_uav_counter_views[register_idx] = vk_counter_view; + for (k = 0; k < vkd3d_popcount(state->uav_counter_mask); ++k) + { + if (state->uav_counters[k].register_index == register_idx) + { + VkBufferView vk_counter_view = descriptor->magic == VKD3D_DESCRIPTOR_MAGIC_UAV + ? descriptor->u.view->vk_counter_view : VK_NULL_HANDLE; + if (bindings->vk_uav_counter_views[k] != vk_counter_view) + bindings->uav_counter_dirty_mask |= 1u << k;
Note that after this patch, "uav_counter_dirty_mask" isn't really being used as a mask anymore.
Well, unless I've made a mistake, it's still a mask; it's just masking indices to the uav_counters/vk_uav_counter_views array instead of register indices... At any rate I don't see a more appropriate name, myself...
On Fri, 5 Jun 2020 at 01:30, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
On 6/4/20 3:48 PM, Henri Verbeet wrote:
On Thu, 4 Jun 2020 at 06:19, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
@@ -2678,14 +2679,20 @@ static void d3d12_command_list_update_descriptor_table(struct d3d12_command_list unsigned int register_idx = range->base_register_idx + j;
/* Track UAV counters. */ - if (range->descriptor_magic == VKD3D_DESCRIPTOR_MAGIC_UAV - && register_idx < ARRAY_SIZE(bindings->vk_uav_counter_views)) + if (range->descriptor_magic == VKD3D_DESCRIPTOR_MAGIC_UAV) { - VkBufferView vk_counter_view = descriptor->magic == VKD3D_DESCRIPTOR_MAGIC_UAV - ? descriptor->u.view->vk_counter_view : VK_NULL_HANDLE; - if (bindings->vk_uav_counter_views[register_idx] != vk_counter_view) - bindings->uav_counter_dirty_mask |= 1u << register_idx; - bindings->vk_uav_counter_views[register_idx] = vk_counter_view; + for (k = 0; k < vkd3d_popcount(state->uav_counter_mask); ++k) + { + if (state->uav_counters[k].register_index == register_idx) + { + VkBufferView vk_counter_view = descriptor->magic == VKD3D_DESCRIPTOR_MAGIC_UAV + ? descriptor->u.view->vk_counter_view : VK_NULL_HANDLE; + if (bindings->vk_uav_counter_views[k] != vk_counter_view) + bindings->uav_counter_dirty_mask |= 1u << k;
Note that after this patch, "uav_counter_dirty_mask" isn't really being used as a mask anymore.
Well, unless I've made a mistake, it's still a mask; it's just masking indices to the uav_counters/vk_uav_counter_views array instead of register indices...
Yes, but after
- if (!state || !(state->uav_counter_mask & bindings->uav_counter_dirty_mask)) + if (!state || !bindings->uav_counter_dirty_mask) we never check or clear individual indices.
On 6/4/20 4:17 PM, Henri Verbeet wrote:
On Fri, 5 Jun 2020 at 01:30, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
On 6/4/20 3:48 PM, Henri Verbeet wrote:
On Thu, 4 Jun 2020 at 06:19, Zebediah Figura <zfigura(a)codeweavers.com> wrote:
@@ -2678,14 +2679,20 @@ static void d3d12_command_list_update_descriptor_table(struct d3d12_command_list unsigned int register_idx = range->base_register_idx + j;
/* Track UAV counters. */ - if (range->descriptor_magic == VKD3D_DESCRIPTOR_MAGIC_UAV - && register_idx < ARRAY_SIZE(bindings->vk_uav_counter_views)) + if (range->descriptor_magic == VKD3D_DESCRIPTOR_MAGIC_UAV) { - VkBufferView vk_counter_view = descriptor->magic == VKD3D_DESCRIPTOR_MAGIC_UAV - ? descriptor->u.view->vk_counter_view : VK_NULL_HANDLE; - if (bindings->vk_uav_counter_views[register_idx] != vk_counter_view) - bindings->uav_counter_dirty_mask |= 1u << register_idx; - bindings->vk_uav_counter_views[register_idx] = vk_counter_view; + for (k = 0; k < vkd3d_popcount(state->uav_counter_mask); ++k) + { + if (state->uav_counters[k].register_index == register_idx) + { + VkBufferView vk_counter_view = descriptor->magic == VKD3D_DESCRIPTOR_MAGIC_UAV + ? descriptor->u.view->vk_counter_view : VK_NULL_HANDLE; + if (bindings->vk_uav_counter_views[k] != vk_counter_view) + bindings->uav_counter_dirty_mask |= 1u << k;
Note that after this patch, "uav_counter_dirty_mask" isn't really being used as a mask anymore.
Well, unless I've made a mistake, it's still a mask; it's just masking indices to the uav_counters/vk_uav_counter_views array instead of register indices...
Yes, but after
- if (!state || !(state->uav_counter_mask & bindings->uav_counter_dirty_mask)) + if (!state || !bindings->uav_counter_dirty_mask) we never check or clear individual indices.
Ah, right. I'll flatten it down to a single flag, then.
participants (2)
-
Henri Verbeet -
Zebediah Figura