On Wednesday, January 11, 2017 6:32:34 PM PST Jason Ekstrand wrote:
> 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
weird
> + * 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.
> + */
I wonder if we could emit a warning if they don't match. That would be
nice, although not essential...
It would be nice if SPIR-V on glslang were less crazy in this regard
but I don't expect either of those to realistically happen. I think
your approach is reasonable.
Thank you so much for fixing this for me.
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
> 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
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
