On 22/10/18 11:24 pm, Alejandro Piñeiro wrote:
vnt_variables uses interface_type on several use cases, but on nir
variable it is more limited. From nir.h:

    /**
      * For variables that are in an interface block or are an instance of an
      * interface block, this is the \c GLSL_TYPE_INTERFACE type for that block.
      *
      * \sa ir_variable::location
      */

But interface blocks expects the type to be an struct, so those cases
should not be filled. For example, glsl checks if a variable is in an
uniform block if it is an uniform and has an interface type.

One example of why this is needed: gl_PatchVerticesIn is lowered to an
uniform. Without this change, it would include a interface_type. Then,
we would try to initialize the uniform block, and find that it doesn't
have any component.
---
  src/compiler/spirv/vtn_variables.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/src/compiler/spirv/vtn_variables.c 
b/src/compiler/spirv/vtn_variables.c
index 957ef0610b7..9a4ddeaa822 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -1818,6 +1818,8 @@ vtn_create_variable(struct vtn_builder *b, struct 
vtn_value *val,
              var->var->members[i].mode = nir_mode;
              var->var->members[i].patch = var->patch;
           }
+      } else {
+         var->var->interface_type = NULL;
        }
/* For inputs and outputs, we need to grab locations and builtin


Please don't do these bandaid style changes, they make the code hard to follow and error prone. We should instead change the code so that interface_type is never set unless we are processing a struct:


      var->var = rzalloc(b->shader, nir_variable);
      var->var->name = ralloc_strdup(var->var, val->name);
      var->var->type = var->type->type;
      var->var->data.mode = nir_mode;
      var->var->data.patch = var->patch;

      /* For inputs and outputs, we immediately split structures.  This
       * is for a couple of reasons.  For one, builtins may all come in
       * a struct and we really want those split out into separate
       * variables.  For another, interpolation qualifiers can be
       * applied to members of the top-level struct ane we need to be
       * able to preserve that information.
       */

      struct vtn_type *interface_type = var->type;
      if (is_per_vertex_inout(var, b->shader->info.stage)) {
         /* In Geometry shaders (and some tessellation), inputs come
          * in per-vertex arrays.  However, some builtins come in
          * non-per-vertex, hence the need for the is_array check.  In
          * any case, there are no non-builtin arrays allowed so this
          * check should be sufficient.
          */
         interface_type = var->type->array_element;
      }

      if (glsl_type_is_struct(interface_type->type)) {
         var->var->interface_type = interface_type->type;

         /* It's a struct.  Set it up as per-member. */
         var->var->num_members = glsl_get_length(interface_type->type);
var->var->members = rzalloc_array(var->var, struct nir_variable_data,
                                           var->var->num_members);

         for (unsigned i = 0; i < var->var->num_members; i++) {
            var->var->members[i].mode = nir_mode;
            var->var->members[i].patch = var->patch;
         }
      }

With that change this is:

Reviewed-by: Timothy Arceri <tarc...@itsqueeze.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to