spirv: Handle patch decorations up-front
authorJason Ekstrand <jason.ekstrand@intel.com>
Thu, 12 Jan 2017 02:04:57 +0000 (18:04 -0800)
committerJason Ekstrand <jason.ekstrand@intel.com>
Thu, 12 Jan 2017 18:41:34 +0000 (10:41 -0800)
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 <kenneth@whitecape.org>
src/compiler/spirv/vtn_variables.c

index 1cc1402f72d9817b0695e7b389a272616fce6339..61a3701e435e9ea982f4c56337d8be8f159e4f92 100644 (file)
@@ -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