[Mesa-dev] [PATCH v2 03/11] mesa: Strip arrayness from interface block names in some IO validation

[Ilia Mirkin]

On Thu, Jun 16, 2016 at 3:06 PM, Ian Romanick <idr at freedesktop.org> wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> Outputs from the vertex shader need to be able to match
> per-vertex-arrayed inputs of later stages.  Acomplish this by stripping
> one level of arrayness from the names and types of outputs going to a
> per-vertex-arrayed stage.
>
> v2: Add missing checks for TESS_EVAL->GEOMETRY.  Noticed by Timothy
> Arceri.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96358
> Cc: "12.0" <mesa-stable at lists.freedesktop.org>
> Cc: Gregory Hainaut <gregory.hainaut at gmail.com>
> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>  src/mesa/main/shader_query.cpp | 100 +++++++++++++++++++++++++++++++++++++----
>  1 file changed, 92 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 5956ce4..a6a0a2f 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -1385,13 +1385,26 @@ _mesa_get_program_resourceiv(struct gl_shader_program *shProg,
>
>  static bool
>  validate_io(struct gl_shader_program *producer,
> -            struct gl_shader_program *consumer)
> +            struct gl_shader_program *consumer,
> +            gl_shader_stage producer_stage,
> +            gl_shader_stage consumer_stage)
>  {
>     if (producer == consumer)
>        return true;
>
> +   const bool nonarray_stage_to_array_stage =
> +      (producer_stage == MESA_SHADER_VERTEX &&
> +       (consumer_stage == MESA_SHADER_GEOMETRY ||
> +        consumer_stage == MESA_SHADER_TESS_CTRL ||
> +        consumer_stage == MESA_SHADER_TESS_EVAL)) ||
> +      (producer_stage == MESA_SHADER_TESS_EVAL &&
> +       consumer_stage == MESA_SHADER_GEOMETRY);

Since TCS -> GEOM is not possible, isn't the only way that array ->
array can happen is TCS -> TES? IOW, couldn't this just be

producer != TCS && (consumer == TCS || consumer == TES || consumer == GS)

Your call whether you want to rewrite it. [No comment on the rest of
the patch...]

  -ilia

> +
>     bool valid = true;
>
> +   void *name_buffer = NULL;
> +   size_t name_buffer_size = 0;
> +
>     gl_shader_variable const **outputs =
>        (gl_shader_variable const **) calloc(producer->NumProgramResourceList,
>                                             sizeof(gl_shader_variable *));
> @@ -1463,11 +1476,52 @@ validate_io(struct gl_shader_program *producer,
>              }
>           }
>        } else {
> +         char *consumer_name = consumer_var->name;
> +
> +         if (nonarray_stage_to_array_stage &&
> +             consumer_var->interface_type != NULL &&
> +             consumer_var->interface_type->is_array() &&
> +             !is_gl_identifier(consumer_var->name)) {
> +            const size_t name_len = strlen(consumer_var->name);
> +
> +            if (name_len >= name_buffer_size) {
> +               free(name_buffer);
> +
> +               name_buffer_size = name_len + 1;
> +               name_buffer = malloc(name_buffer_size);
> +               if (name_buffer == NULL) {
> +                  valid = false;
> +                  goto out;
> +               }
> +            }
> +
> +            consumer_name = (char *) name_buffer;
> +
> +            char *s = strchr(consumer_var->name, '[');
> +            if (s == NULL) {
> +               valid = false;
> +               goto out;
> +            }
> +
> +            char *t = strchr(s, ']');
> +            if (t == NULL) {
> +               valid = false;
> +               goto out;
> +            }
> +
> +            assert(t[1] == '.' || t[1] == '[');
> +
> +            const ptrdiff_t base_name_len = s - consumer_var->name;
> +
> +            memcpy(consumer_name, consumer_var->name, base_name_len);
> +            strcpy(consumer_name + base_name_len, t + 1);
> +         }
> +
>           for (unsigned j = 0; j < num_outputs; j++) {
>              const gl_shader_variable *const var = outputs[j];
>
>              if (!var->explicit_location &&
> -                strcmp(consumer_var->name, var->name) == 0) {
> +                strcmp(consumer_name, var->name) == 0) {
>                 producer_var = var;
>                 match_index = j;
>                 break;
> @@ -1529,25 +1583,53 @@ validate_io(struct gl_shader_program *producer,
>         * Note that location mismatches are detected by the loops above that
>         * find the producer variable that goes with the consumer variable.
>         */
> -      if (producer_var->type != consumer_var->type ||
> -          producer_var->interpolation != consumer_var->interpolation ||
> -          producer_var->precision != consumer_var->precision) {
> +      if (nonarray_stage_to_array_stage) {
> +         if (!consumer_var->type->is_array() ||
> +             consumer_var->type->fields.array != producer_var->type) {
> +            valid = false;
> +            goto out;
> +         }
> +
> +         if (consumer_var->interface_type != NULL) {
> +            if (!consumer_var->interface_type->is_array() ||
> +                consumer_var->interface_type->fields.array != producer_var->interface_type) {
> +               valid = false;
> +               goto out;
> +            }
> +         } else if (producer_var->interface_type != NULL) {
> +            valid = false;
> +            goto out;
> +         }
> +      } else {
> +         if (producer_var->type != consumer_var->type) {
> +            valid = false;
> +            goto out;
> +         }
> +
> +         if (producer_var->interface_type != consumer_var->interface_type) {
> +            valid = false;
> +            goto out;
> +         }
> +      }
> +
> +      if (producer_var->interpolation != consumer_var->interpolation) {
>           valid = false;
>           goto out;
>        }
>
> -      if (producer_var->outermost_struct_type != consumer_var->outermost_struct_type) {
> +      if (producer_var->precision != consumer_var->precision) {
>           valid = false;
>           goto out;
>        }
>
> -      if (producer_var->interface_type != consumer_var->interface_type) {
> +      if (producer_var->outermost_struct_type != consumer_var->outermost_struct_type) {
>           valid = false;
>           goto out;
>        }
>     }
>
>   out:
> +   free(name_buffer);
>     free(outputs);
>     return valid && num_outputs == 0;
>  }
> @@ -1579,7 +1661,9 @@ _mesa_validate_pipeline_io(struct gl_pipeline_object *pipeline)
>           if (shProg[idx]->_LinkedShaders[idx]->Stage == MESA_SHADER_COMPUTE)
>              break;
>
> -         if (!validate_io(shProg[prev], shProg[idx]))
> +         if (!validate_io(shProg[prev], shProg[idx],
> +                          shProg[prev]->_LinkedShaders[prev]->Stage,
> +                          shProg[idx]->_LinkedShaders[idx]->Stage))
>              return false;
>
>           prev = idx;
> --
> 2.5.5
>