From 1c8e80a07b8f59deebd2f6d045b2a8049ebdbab4 Mon Sep 17 00:00:00 2001 From: Mike Blumenkrantz Date: Fri, 26 Jun 2020 15:49:03 -0400 Subject: [PATCH] zink: correctly set up ubo bindings and buffer indices var->data.binding is only set for vulkan drivers (though it also will get incremented after nir_lower_uniforms_to_ubo), so we have to generate our own values here. to do this, we iterate backwards over the ubos to account for the "first" ubos being at the end of the list, and we must also ensure that we remap the buffer index correctly based on whether we're running our nir_lower_uniforms_to_ubo pass note that running nir_lower_uniforms_to_ubo unconditionally would require us to add a number of checks in this patch for !shader->num_uniforms in order to properly adjust to the altered instructions which become 1-indexed instead of 0-indexed when this pass is run with no uniforms present Reviewed-by: Erik Faye-Lund Part-of: --- .../drivers/zink/nir_to_spirv/nir_to_spirv.c | 11 ++- src/gallium/drivers/zink/zink_compiler.c | 79 +++++++++++-------- 2 files changed, 53 insertions(+), 37 deletions(-) diff --git a/src/gallium/drivers/zink/nir_to_spirv/nir_to_spirv.c b/src/gallium/drivers/zink/nir_to_spirv/nir_to_spirv.c index ae45cbc5424..98a1937ad6e 100644 --- a/src/gallium/drivers/zink/nir_to_spirv/nir_to_spirv.c +++ b/src/gallium/drivers/zink/nir_to_spirv/nir_to_spirv.c @@ -2333,12 +2333,15 @@ nir_to_spirv(struct nir_shader *s, const struct zink_so_info *so_info) nir_foreach_shader_out_variable(var, s) emit_output(&ctx, var); + if (so_info) emit_so_info(&ctx, util_last_bit64(s->info.outputs_written), so_info); - nir_foreach_variable_with_modes(var, s, nir_var_uniform | - nir_var_mem_ubo | - nir_var_mem_ssbo) - emit_uniform(&ctx, var); + /* we have to reverse iterate to match what's done in zink_compiler.c */ + foreach_list_typed_reverse(nir_variable, var, node, &s->variables) + if (_nir_shader_variable_has_mode(var, nir_var_uniform | + nir_var_mem_ubo | + nir_var_mem_ssbo)) + emit_uniform(&ctx, var); if (s->info.stage == MESA_SHADER_FRAGMENT) { spirv_builder_emit_exec_mode(&ctx.builder, entry_point, diff --git a/src/gallium/drivers/zink/zink_compiler.c b/src/gallium/drivers/zink/zink_compiler.c index eca835c0daf..9c7bdce341f 100644 --- a/src/gallium/drivers/zink/zink_compiler.c +++ b/src/gallium/drivers/zink/zink_compiler.c @@ -281,46 +281,59 @@ zink_shader_create(struct zink_screen *screen, struct nir_shader *nir, } ret->num_bindings = 0; - nir_foreach_variable_with_modes(var, nir, nir_var_uniform | - nir_var_mem_ubo) { - if (var->data.mode == nir_var_mem_ubo) { - /* ignore variables being accessed if they aren't the base of the UBO */ - if (var->data.location) - continue; - int binding = zink_binding(nir->info.stage, - VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, - var->data.binding); - ret->bindings[ret->num_bindings].index = var->data.binding; - ret->bindings[ret->num_bindings].binding = binding; - ret->bindings[ret->num_bindings].type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; - ret->num_bindings++; - } else { - assert(var->data.mode == nir_var_uniform); - if (glsl_type_is_sampler(var->type)) { - int binding = zink_binding(nir->info.stage, - VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, - var->data.binding); - ret->bindings[ret->num_bindings].index = var->data.binding; - ret->bindings[ret->num_bindings].binding = binding; - ret->bindings[ret->num_bindings].type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER; - ret->num_bindings++; - } else if (glsl_type_is_array(var->type)) { - /* need to unroll possible arrays of arrays before checking type - * in order to handle ARB_arrays_of_arrays extension - */ - const struct glsl_type *type = glsl_without_array(var->type); - if (!glsl_type_is_sampler(type)) + uint32_t cur_ubo = 0; + /* UBO buffers are zero-indexed, but buffer 0 is always the one created by nir_lower_uniforms_to_ubo, + * which means there is no buffer 0 if there are no uniforms + */ + int ubo_index = !nir->num_uniforms; + /* need to set up var->data.binding for UBOs, which means we need to start at + * the "first" UBO, which is at the end of the list + */ + foreach_list_typed_reverse(nir_variable, var, node, &nir->variables) { + if (_nir_shader_variable_has_mode(var, nir_var_uniform | + nir_var_mem_ubo | + nir_var_mem_ssbo)) { + if (var->data.mode == nir_var_mem_ubo) { + /* ignore variables being accessed if they aren't the base of the UBO */ + if (var->data.location) continue; + var->data.binding = cur_ubo++; - unsigned size = glsl_get_aoa_size(var->type); - for (int i = 0; i < size; ++i) { + int binding = zink_binding(nir->info.stage, + VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, + var->data.binding); + ret->bindings[ret->num_bindings].index = ubo_index++; + ret->bindings[ret->num_bindings].binding = binding; + ret->bindings[ret->num_bindings].type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; + ret->num_bindings++; + } else { + assert(var->data.mode == nir_var_uniform); + if (glsl_type_is_sampler(var->type)) { int binding = zink_binding(nir->info.stage, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, - var->data.binding + i); - ret->bindings[ret->num_bindings].index = var->data.binding + i; + var->data.binding); + ret->bindings[ret->num_bindings].index = var->data.binding; ret->bindings[ret->num_bindings].binding = binding; ret->bindings[ret->num_bindings].type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER; ret->num_bindings++; + } else if (glsl_type_is_array(var->type)) { + /* need to unroll possible arrays of arrays before checking type + * in order to handle ARB_arrays_of_arrays extension + */ + const struct glsl_type *type = glsl_without_array(var->type); + if (!glsl_type_is_sampler(type)) + continue; + + unsigned size = glsl_get_aoa_size(var->type); + for (int i = 0; i < size; ++i) { + int binding = zink_binding(nir->info.stage, + VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, + var->data.binding + i); + ret->bindings[ret->num_bindings].index = var->data.binding + i; + ret->bindings[ret->num_bindings].binding = binding; + ret->bindings[ret->num_bindings].type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER; + ret->num_bindings++; + } } } }