From 27a1c7ffbdd7d7a8fd1240413446c1172752d93b Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Wed, 11 Jan 2017 18:04:57 -0800 Subject: [PATCH] spirv: Handle patch decorations up-front Once again, SPIR-V is insane... It allows you to place "patch" decorations on structure members. Presumably, this is so that you can do something such as out struct S { layout(location = 0) patch vec4 thing1; layout(location = 0) vec4 thing2; } str; And have your I/O "nicely" organized. While this is a bit silly, it's allowed and well-defined so whatever. Where it really gets interesting is when you have an array of struct. SPIR-V says nothing about not allowing you to have those qualifiers on the members of a struct that's inside an array and GLSLang does this. Specifically, if you have layout(location = 0) out patch struct S { vec4 thing1; vec4 thing2; } str[2]; then GLSLang will place the "patch" decorations on the struct members. This is ridiculous there is no way that having some of them be patch and some not would be well-defined given that patch and non-patch outputs are in effectively different storage classes. This commit moves around the way we handle the "patch" decoration so that we can detect even the crazy cases and handle them. Fixes: dEQP-VK.tessellation.user_defined_io.per_patch_block_array.* Reviewed-by: Kenneth Graunke --- src/compiler/spirv/vtn_variables.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_variables.c index 1cc1402f72d..61a3701e435 100644 --- a/src/compiler/spirv/vtn_variables.c +++ b/src/compiler/spirv/vtn_variables.c @@ -1361,8 +1361,29 @@ vtn_handle_variables(struct vtn_builder *b, SpvOp opcode, case vtn_variable_mode_input: case vtn_variable_mode_output: { + /* In order to know whether or not we're a per-vertex inout, we need + * the patch qualifier. This means walking the variable decorations + * early before we actually create any variables. Not a big deal. + * + * GLSLang really likes to place decorations in the most interior + * thing it possibly can. In particular, if you have a struct, it + * will place the patch decorations on the struct members. This + * should be handled by the variable splitting below just fine. + * + * If you have an array-of-struct, things get even more weird as it + * will place the patch decorations on the struct even though it's + * inside an array and some of the members being patch and others not + * makes no sense whatsoever. Since the only sensible thing is for + * it to be all or nothing, we'll call it patch if any of the members + * are declared patch. + */ var->patch = false; vtn_foreach_decoration(b, val, var_is_patch_cb, &var->patch); + if (glsl_type_is_array(var->type->type) && + glsl_type_is_struct(without_array->type)) { + vtn_foreach_decoration(b, without_array->val, + var_is_patch_cb, &var->patch); + } /* For inputs and outputs, we immediately split structures. This * is for a couple of reasons. For one, builtins may all come in @@ -1402,6 +1423,7 @@ vtn_handle_variables(struct vtn_builder *b, SpvOp opcode, var->members[i]->interface_type = interface_type->members[i]->type; var->members[i]->data.mode = nir_mode; + var->members[i]->data.patch = var->patch; } } else { var->var = rzalloc(b->shader, nir_variable); @@ -1409,6 +1431,7 @@ vtn_handle_variables(struct vtn_builder *b, SpvOp opcode, var->var->type = var->type->type; var->var->interface_type = interface_type->type; var->var->data.mode = nir_mode; + var->var->data.patch = var->patch; } /* For inputs and outputs, we need to grab locations and builtin