From 0709c0f6b40b1e365104b248464ffefa746b5052 Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Thu, 31 Oct 2019 14:09:39 -0500 Subject: [PATCH] anv: Flatten descriptor bindings in anv_nir_apply_pipeline_layout This lets us stop tracking the pipeline layout. It also means less indirection on a very hot path. As an extra bonus, we can make some of our data structures smaller. No measurable CPU overhead improvement. Reviewed-by: Lionel Landwerlin --- src/intel/vulkan/anv_cmd_buffer.c | 8 ---- .../vulkan/anv_nir_apply_pipeline_layout.c | 39 ++++++++++------ src/intel/vulkan/anv_pipeline.c | 3 -- src/intel/vulkan/anv_private.h | 34 ++++++++------ src/intel/vulkan/genX_cmd_buffer.c | 45 ++++--------------- src/intel/vulkan/genX_pipeline.c | 1 - 6 files changed, 54 insertions(+), 76 deletions(-) diff --git a/src/intel/vulkan/anv_cmd_buffer.c b/src/intel/vulkan/anv_cmd_buffer.c index 59a281d6053..4bf2725dccc 100644 --- a/src/intel/vulkan/anv_cmd_buffer.c +++ b/src/intel/vulkan/anv_cmd_buffer.c @@ -614,14 +614,6 @@ anv_cmd_buffer_bind_descriptor_set(struct anv_cmd_buffer *cmd_buffer, cmd_buffer->state.descriptors_dirty |= set_layout->shader_stages & VK_SHADER_STAGE_ALL_GRAPHICS; } - - /* Pipeline layout objects are required to live at least while any command - * buffers that use them are in recording state. We need to grab a reference - * to the pipeline layout being bound here so we can compute correct dynamic - * offsets for VK_DESCRIPTOR_TYPE_*_DYNAMIC in dynamic_offset_for_binding() - * when we record draw commands that come after this. - */ - pipe_state->layout = layout; } void anv_CmdBindDescriptorSets( diff --git a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c index 1d13aa60492..c4717ea7665 100644 --- a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c +++ b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c @@ -1137,7 +1137,7 @@ anv_nir_apply_pipeline_layout(const struct anv_physical_device *pdevice, map->surface_to_descriptor[map->surface_count] = (struct anv_pipeline_binding) { .set = ANV_DESCRIPTOR_SET_DESCRIPTORS, - .binding = s, + .index = s, }; state.set[s].desc_offset = map->surface_count; map->surface_count++; @@ -1226,16 +1226,28 @@ anv_nir_apply_pipeline_layout(const struct anv_physical_device *pdevice, state.set[set].surface_offsets[b] = BINDLESS_OFFSET; } else { state.set[set].surface_offsets[b] = map->surface_count; - struct anv_sampler **samplers = binding->immutable_samplers; - for (unsigned i = 0; i < binding->array_size; i++) { - uint8_t planes = samplers ? samplers[i]->n_planes : 1; - for (uint8_t p = 0; p < planes; p++) { + if (binding->dynamic_offset_index < 0) { + struct anv_sampler **samplers = binding->immutable_samplers; + for (unsigned i = 0; i < binding->array_size; i++) { + uint8_t planes = samplers ? samplers[i]->n_planes : 1; + for (uint8_t p = 0; p < planes; p++) { + map->surface_to_descriptor[map->surface_count++] = + (struct anv_pipeline_binding) { + .set = set, + .index = binding->descriptor_index + i, + .plane = p, + }; + } + } + } else { + for (unsigned i = 0; i < binding->array_size; i++) { map->surface_to_descriptor[map->surface_count++] = (struct anv_pipeline_binding) { .set = set, - .binding = b, - .index = i, - .plane = p, + .index = binding->descriptor_index + i, + .dynamic_offset_index = + layout->set[set].dynamic_offset_start + + binding->dynamic_offset_index + i, }; } } @@ -1264,8 +1276,7 @@ anv_nir_apply_pipeline_layout(const struct anv_physical_device *pdevice, map->sampler_to_descriptor[map->sampler_count++] = (struct anv_pipeline_binding) { .set = set, - .binding = b, - .index = i, + .index = binding->descriptor_index + i, .plane = p, }; } @@ -1294,8 +1305,9 @@ anv_nir_apply_pipeline_layout(const struct anv_physical_device *pdevice, const uint32_t set = var->data.descriptor_set; const uint32_t binding = var->data.binding; - const uint32_t array_size = - layout->set[set].layout->binding[binding].array_size; + struct anv_descriptor_set_binding_layout *bind_layout = + &layout->set[set].layout->binding[binding]; + const uint32_t array_size = bind_layout->array_size; if (state.set[set].use_count[binding] == 0) continue; @@ -1307,8 +1319,7 @@ anv_nir_apply_pipeline_layout(const struct anv_physical_device *pdevice, &map->surface_to_descriptor[state.set[set].surface_offsets[binding]]; for (unsigned i = 0; i < array_size; i++) { assert(pipe_binding[i].set == set); - assert(pipe_binding[i].binding == binding); - assert(pipe_binding[i].index == i); + assert(pipe_binding[i].index == bind_layout->descriptor_index + i); if (dim == GLSL_SAMPLER_DIM_SUBPASS || dim == GLSL_SAMPLER_DIM_SUBPASS_MS) diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c index 7e8a825a7d6..0f582e2801e 100644 --- a/src/intel/vulkan/anv_pipeline.c +++ b/src/intel/vulkan/anv_pipeline.c @@ -926,14 +926,12 @@ anv_pipeline_link_fs(const struct brw_compiler *compiler, if (stage->key.wm.color_outputs_valid & BITFIELD_BIT(rt)) { rt_bindings[rt] = (struct anv_pipeline_binding) { .set = ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS, - .binding = 0, .index = rt, }; } else { /* Setup a null render target */ rt_bindings[rt] = (struct anv_pipeline_binding) { .set = ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS, - .binding = 0, .index = UINT32_MAX, }; } @@ -943,7 +941,6 @@ anv_pipeline_link_fs(const struct brw_compiler *compiler, /* Setup a null render target */ rt_bindings[0] = (struct anv_pipeline_binding) { .set = ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS, - .binding = 0, .index = UINT32_MAX, }; num_rt_bindings = 1; diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 0b1e97c64b6..5b8ac491ce5 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -2012,25 +2012,32 @@ anv_descriptor_set_destroy(struct anv_device *device, #define ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS UINT8_MAX struct anv_pipeline_binding { - /* The descriptor set this surface corresponds to. The special value of - * ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS indicates that the offset refers - * to a color attachment and not a regular descriptor. + /** Index in the descriptor set + * + * This is a flattened index; the descriptor set layout is already taken + * into account. */ - uint8_t set; + uint32_t index; - /* Binding in the descriptor set */ - uint32_t binding; + /** The descriptor set this surface corresponds to. + * + * The special ANV_DESCRIPTOR_SET_* values above indicates that this + * binding is not a normal descriptor set but something else. + */ + uint8_t set; - /* Index in the binding */ - uint32_t index; + union { + /** Plane in the binding index for images */ + uint8_t plane; - /* Plane in the binding index */ - uint8_t plane; + /** Input attachment index (relative to the subpass) */ + uint8_t input_attachment_index; - /* Input attachment index (relative to the subpass) */ - uint8_t input_attachment_index; + /** Dynamic offset index (for dynamic UBOs and SSBOs) */ + uint8_t dynamic_offset_index; + }; - /* For a storage image, whether it is write-only */ + /** For a storage image, whether it is write-only */ bool write_only; }; @@ -2468,7 +2475,6 @@ struct anv_attachment_state { */ struct anv_cmd_pipeline_state { struct anv_pipeline *pipeline; - struct anv_pipeline_layout *layout; struct anv_descriptor_set *descriptors[MAX_SETS]; uint32_t dynamic_offsets[MAX_DYNAMIC_BUFFERS]; diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c index 67ba0307195..b52aabdd984 100644 --- a/src/intel/vulkan/genX_cmd_buffer.c +++ b/src/intel/vulkan/genX_cmd_buffer.c @@ -2072,34 +2072,6 @@ cmd_buffer_alloc_push_constants(struct anv_cmd_buffer *cmd_buffer) cmd_buffer->state.push_constants_dirty |= VK_SHADER_STAGE_ALL_GRAPHICS; } -static const struct anv_descriptor * -anv_descriptor_for_binding(const struct anv_cmd_pipeline_state *pipe_state, - const struct anv_pipeline_binding *binding) -{ - assert(binding->set < MAX_SETS); - const struct anv_descriptor_set *set = - pipe_state->descriptors[binding->set]; - const uint32_t offset = - set->layout->binding[binding->binding].descriptor_index; - return &set->descriptors[offset + binding->index]; -} - -static uint32_t -dynamic_offset_for_binding(const struct anv_cmd_pipeline_state *pipe_state, - const struct anv_pipeline_binding *binding) -{ - assert(binding->set < MAX_SETS); - const struct anv_descriptor_set *set = - pipe_state->descriptors[binding->set]; - - uint32_t dynamic_offset_idx = - pipe_state->layout->set[binding->set].dynamic_offset_start + - set->layout->binding[binding->binding].dynamic_offset_index + - binding->index; - - return pipe_state->dynamic_offsets[dynamic_offset_idx]; -} - static struct anv_address anv_descriptor_set_address(struct anv_cmd_buffer *cmd_buffer, struct anv_descriptor_set *set) @@ -2179,7 +2151,6 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer, if (binding->set == ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS) { /* Color attachment binding */ assert(stage == MESA_SHADER_FRAGMENT); - assert(binding->binding == 0); if (binding->index < subpass->color_count) { const unsigned att = subpass->color_attachments[binding->index].attachment; @@ -2247,7 +2218,7 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer, * given by binding->binding. (Yes, that's confusing.) */ struct anv_descriptor_set *set = - pipe_state->descriptors[binding->binding]; + pipe_state->descriptors[binding->index]; assert(set->desc_mem.alloc_size); assert(set->desc_surface_state.alloc_size); bt_map[s] = set->desc_surface_state.offset + state_offset; @@ -2257,7 +2228,7 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer, } const struct anv_descriptor *desc = - anv_descriptor_for_binding(pipe_state, binding); + &pipe_state->descriptors[binding->set]->descriptors[binding->index]; switch (desc->type) { case VK_DESCRIPTOR_TYPE_SAMPLER: @@ -2339,7 +2310,7 @@ emit_binding_table(struct anv_cmd_buffer *cmd_buffer, case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC: { /* Compute the offset within the buffer */ uint32_t dynamic_offset = - dynamic_offset_for_binding(pipe_state, binding); + pipe_state->dynamic_offsets[binding->dynamic_offset_index]; uint64_t offset = desc->offset + dynamic_offset; /* Clamp to the buffer size */ offset = MIN2(offset, desc->buffer->size); @@ -2413,7 +2384,7 @@ emit_samplers(struct anv_cmd_buffer *cmd_buffer, for (uint32_t s = 0; s < map->sampler_count; s++) { struct anv_pipeline_binding *binding = &map->sampler_to_descriptor[s]; const struct anv_descriptor *desc = - anv_descriptor_for_binding(pipe_state, binding); + &pipe_state->descriptors[binding->set]->descriptors[binding->index]; if (desc->type != VK_DESCRIPTOR_TYPE_SAMPLER && desc->type != VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER) @@ -2608,7 +2579,7 @@ cmd_buffer_flush_push_constants(struct anv_cmd_buffer *cmd_buffer, * confusing.) */ struct anv_descriptor_set *set = - gfx_state->base.descriptors[binding->binding]; + gfx_state->base.descriptors[binding->index]; struct anv_address desc_buffer_addr = anv_descriptor_set_address(cmd_buffer, set); const unsigned desc_buffer_size = set->desc_mem.alloc_size; @@ -2618,8 +2589,10 @@ cmd_buffer_flush_push_constants(struct anv_cmd_buffer *cmd_buffer, read_addr = anv_address_add(desc_buffer_addr, range->start * 32); } else { + struct anv_descriptor_set *set = + gfx_state->base.descriptors[binding->set]; const struct anv_descriptor *desc = - anv_descriptor_for_binding(&gfx_state->base, binding); + &set->descriptors[binding->index]; if (desc->type == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER) { read_len = MIN2(range->length, @@ -2630,7 +2603,7 @@ cmd_buffer_flush_push_constants(struct anv_cmd_buffer *cmd_buffer, assert(desc->type == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC); uint32_t dynamic_offset = - dynamic_offset_for_binding(&gfx_state->base, binding); + gfx_state->base.dynamic_offsets[binding->dynamic_offset_index]; uint32_t buf_offset = MIN2(desc->offset + dynamic_offset, desc->buffer->size); uint32_t buf_range = diff --git a/src/intel/vulkan/genX_pipeline.c b/src/intel/vulkan/genX_pipeline.c index 5594726e4a2..1d35c99001b 100644 --- a/src/intel/vulkan/genX_pipeline.c +++ b/src/intel/vulkan/genX_pipeline.c @@ -1111,7 +1111,6 @@ emit_cb_state(struct anv_pipeline *pipeline, continue; } - assert(binding->binding == 0); const VkPipelineColorBlendAttachmentState *a = &info->pAttachments[binding->index]; -- 2.30.2