From dfc3a7cb3c06c2e0d57e96ad15f7126461540994 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Fri, 27 Jul 2018 16:05:05 +0200 Subject: [PATCH] spirv/nir: handle location decorations on block interface members MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Previously the code was taking any location decoration on the block and using that to calculate the member locations for all of the members. I think this was assuming that there would only be one location decoration for the entire block. According to the Vulkan spec it is possible to add location decorations to individual members: “If the structure type is a Block but without a Location, then each of its members must have a Location decoration. If it is a Block with a Location decoration, then its members are assigned consecutive locations in declaration order, starting from the first member which is initially the Block. Any member with its own Location decoration is assigned that location. Each remaining member is assigned the location after the immediately preceding member in declaration order.” This patch makes it instead keep track of which members have been assigned an explicit location. It also has a space to store the location for the struct as a whole. Once all the decorations have been processed it iterates over each member to fill in the missing locations using the rules described above. So, this commit is needed to get working a case like this, on both Vulkan and OpenGL using SPIR-V (ARB_gl_spirv): out block { layout(location = 2) vec4 c; layout(location = 3) vec4 d; layout(location = 0) vec4 a; layout(location = 1) vec4 b; } name; v2: (changes made by Alejandro Piñeiro) * Update after introducing struct member splitting (See commit b0c643d) * Update after only exposing interface_type for blocks, not to any struct * Update after last changes done for xfb support v3: use "assign" instead of "add" on the new method added (Tapani) Signed-off-by: Neil Roberts Signed-off-by: Alejandro Piñeiro Reviewed-by: Tapani Pälli --- src/compiler/spirv/vtn_private.h | 6 +++ src/compiler/spirv/vtn_variables.c | 69 ++++++++++++++++++++++++++---- 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h index cba2bd665fa..748a2a2e742 100644 --- a/src/compiler/spirv/vtn_private.h +++ b/src/compiler/spirv/vtn_private.h @@ -475,6 +475,12 @@ struct vtn_variable { nir_variable *var; + /* If the variable is a struct with a location set on it then this will be + * stored here. This will be used to calculate locations for members that + * don’t have their own explicit location. + */ + int base_location; + int shared_location; /** diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c index b1ac0b99784..91b24241fd3 100644 --- a/src/compiler/spirv/vtn_variables.c +++ b/src/compiler/spirv/vtn_variables.c @@ -1597,13 +1597,11 @@ var_decoration_cb(struct vtn_builder *b, struct vtn_value *val, int member, */ if (dec->decoration == SpvDecorationLocation) { unsigned location = dec->literals[0]; - bool is_vertex_input = false; if (b->shader->info.stage == MESA_SHADER_FRAGMENT && vtn_var->mode == vtn_variable_mode_output) { location += FRAG_RESULT_DATA0; } else if (b->shader->info.stage == MESA_SHADER_VERTEX && vtn_var->mode == vtn_variable_mode_input) { - is_vertex_input = true; location += VERT_ATTRIB_GENERIC0; } else if (vtn_var->mode == vtn_variable_mode_input || vtn_var->mode == vtn_variable_mode_output) { @@ -1620,14 +1618,13 @@ var_decoration_cb(struct vtn_builder *b, struct vtn_value *val, int member, } else { /* This handles the structure member case */ assert(vtn_var->var->members); - for (unsigned i = 0; i < vtn_var->var->num_members; i++) { - vtn_var->var->members[i].location = location; - const struct glsl_type *member_type = - glsl_get_struct_field(vtn_var->var->interface_type, i); - location += glsl_count_attribute_slots(member_type, - is_vertex_input); - } + + if (member == -1) + vtn_var->base_location = location; + else + vtn_var->var->members[member].location = location; } + return; } else { if (vtn_var->var) { @@ -1914,6 +1911,50 @@ is_per_vertex_inout(const struct vtn_variable *var, gl_shader_stage stage) return false; } +static void +assign_missing_member_locations(struct vtn_variable *var, + bool is_vertex_input) +{ + unsigned length = + glsl_get_length(glsl_without_array(var->type->type)); + int location = var->base_location; + + for (unsigned i = 0; i < length; i++) { + /* From the Vulkan spec: + * + * “If the structure type is a Block but without a Location, then each + * of its members must have a Location decoration.” + * + */ + if (var->type->block) { + assert(var->base_location != -1 || + var->var->members[i].location != -1); + } + + /* From the Vulkan spec: + * + * “Any member with its own Location decoration is assigned that + * location. Each remaining member is assigned the location after the + * immediately preceding member in declaration order.” + */ + if (var->var->members[i].location != -1) + location = var->var->members[i].location; + else + var->var->members[i].location = location; + + /* Below we use type instead of interface_type, because interface_type + * is only available when it is a Block. This code also supports + * input/outputs that are just structs + */ + const struct glsl_type *member_type = + glsl_get_struct_field(glsl_without_array(var->type->type), i); + + location += + glsl_count_attribute_slots(member_type, is_vertex_input); + } +} + + static void vtn_create_variable(struct vtn_builder *b, struct vtn_value *val, struct vtn_type *ptr_type, SpvStorageClass storage_class, @@ -1977,6 +2018,7 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val, struct vtn_variable *var = rzalloc(b, struct vtn_variable); var->type = type; var->mode = mode; + var->base_location = -1; vtn_assert(val->value_type == vtn_value_type_pointer); val->pointer = vtn_pointer_for_variable(b, var, ptr_type); @@ -2105,6 +2147,7 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val, for (unsigned i = 0; i < var->var->num_members; i++) { var->var->members[i].mode = nir_mode; var->var->members[i].patch = var->patch; + var->var->members[i].location = -1; } } @@ -2135,6 +2178,14 @@ vtn_create_variable(struct vtn_builder *b, struct vtn_value *val, vtn_foreach_decoration(b, val, var_decoration_cb, var); + if ((var->mode == vtn_variable_mode_input || + var->mode == vtn_variable_mode_output) && + var->var->members) { + bool is_vertex_input = (b->shader->info.stage == MESA_SHADER_VERTEX && + var->mode == vtn_variable_mode_input); + assign_missing_member_locations(var, is_vertex_input); + } + if (var->mode == vtn_variable_mode_uniform) { /* XXX: We still need the binding information in the nir_variable * for these. We should fix that. -- 2.30.2