[Mesa-dev] [PATCH] spirv: Handle patch decorations up-front

Jason Ekstrand jason at jlekstrand.net
Thu Jan 12 02:32:34 UTC 2017


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.*
---
 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 3ecb54f..91e5b13 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -1359,8 +1359,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 wierd 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
@@ -1400,6 +1421,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);
@@ -1407,6 +1429,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
-- 
2.5.0.400.gff86faf



More information about the mesa-dev mailing list