[Mesa-dev] [PATCH V2 01/12] glsl: simplify interface block stream qualifier validation

[Samuel Iglesias Gonsálvez]

Reviewed-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>

On 08/11/15 23:34, Timothy Arceri wrote:
> From: Timothy Arceri <timothy.arceri at collabora.com>
> 
> Qualifiers on member variables are redundent all we need to do
> if check if it matches the stream associated with the block and
> throw an error if its not.
> 
> Cc: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> Cc: Emil Velikov <emil.l.velikov at gmail.com>
> ---
>  src/glsl/ast_to_hir.cpp   | 27 +++++++++++++--------------
>  src/glsl/nir/glsl_types.h | 10 +---------
>  2 files changed, 14 insertions(+), 23 deletions(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 0306530..5a22820 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -5964,8 +5964,19 @@ ast_process_structure_or_interface_block(exec_list *instructions,
>           fields[i].sample = qual->flags.q.sample ? 1 : 0;
>           fields[i].patch = qual->flags.q.patch ? 1 : 0;
>  
> -         /* Only save explicitly defined streams in block's field */
> -         fields[i].stream = qual->flags.q.explicit_stream ? qual->stream : -1;
> +         /* From Section 4.4.2.3 (Geometry Outputs) of the GLSL 4.50 spec:
> +          *
> +          *   "A block member may be declared with a stream identifier, but
> +          *   the specified stream must match the stream associated with the
> +          *   containing block."
> +          */
> +         if (qual->flags.q.explicit_stream &&
> +             qual->stream != layout->stream) {
> +            _mesa_glsl_error(&loc, state, "stream layout qualifier on "
> +                             "interface block member `%s' does not match "
> +                             "the interface block (%d vs %d)",
> +                             fields[i].name, qual->stream, layout->stream);
> +         }
>  
>           if (qual->flags.q.row_major || qual->flags.q.column_major) {
>              if (!qual->flags.q.uniform && !qual->flags.q.buffer) {
> @@ -6267,18 +6278,6 @@ ast_interface_block::hir(exec_list *instructions,
>  
>     state->struct_specifier_depth--;
>  
> -   for (unsigned i = 0; i < num_variables; i++) {
> -      if (fields[i].stream != -1 &&
> -          (unsigned) fields[i].stream != this->layout.stream) {
> -         _mesa_glsl_error(&loc, state,
> -                          "stream layout qualifier on "
> -                          "interface block member `%s' does not match "
> -                          "the interface block (%d vs %d)",
> -                          fields[i].name, fields[i].stream,
> -                          this->layout.stream);
> -      }
> -   }
> -
>     if (!redeclaring_per_vertex) {
>        validate_identifier(this->block_name, loc, state);
>  
> diff --git a/src/glsl/nir/glsl_types.h b/src/glsl/nir/glsl_types.h
> index 52ca826..1f17ad5 100644
> --- a/src/glsl/nir/glsl_types.h
> +++ b/src/glsl/nir/glsl_types.h
> @@ -829,13 +829,6 @@ struct glsl_struct_field {
>     unsigned patch:1;
>  
>     /**
> -    * For interface blocks, it has a value if this variable uses multiple vertex
> -    * streams (as in ir_variable::stream). -1 otherwise.
> -    */
> -   int stream;
> -
> -
> -   /**
>      * Image qualifiers, applicable to buffer variables defined in shader
>      * storage buffer objects (SSBOs)
>      */
> @@ -847,8 +840,7 @@ struct glsl_struct_field {
>  
>     glsl_struct_field(const struct glsl_type *_type, const char *_name)
>        : type(_type), name(_name), location(-1), interpolation(0), centroid(0),
> -        sample(0), matrix_layout(GLSL_MATRIX_LAYOUT_INHERITED), patch(0),
> -        stream(-1)
> +        sample(0), matrix_layout(GLSL_MATRIX_LAYOUT_INHERITED), patch(0)
>     {
>        /* empty */
>     }
>