[Mesa-dev] [PATCH 6/9] glsl: push layout-qualifier-name values from variable declarations to global

[Timothy Arceri]

On Sat, 2016-10-22 at 23:09 +0300, Andres Gomez wrote:
> After the previous modifications in the merging of the
> layout-qualifier-name values, we no longer push the final value in a
> declaration to the global values.
> 
> This regression happens because we don't call for merging on the
> right-most layout qualifier of a declaration which is also the
> overriding one in case of multiple appearances.

I'm a little confused about this change. How do all the other layout
qualifiers get set if we are not calling merge on the rightmost
qualifier?

> 
> Now, we add a new method to push these values to the global ones and
> we call for this just after all the layout-qualifier collapsing has
> happened in a declaration.
> 
> This simplifies how this was working in two ways; we make a clear
> differentiation of when we are pushing this to the global values
> since
> before it was mixed in the merging call and we only run this once all
> the processing for layout-qualifiers in a declaration has happened.
> 
> Signed-off-by: Andres Gomez <agomez at igalia.com>
> ---
>  src/compiler/glsl/ast.h          |  6 +++++
>  src/compiler/glsl/ast_type.cpp   | 48 ++++++++++++++++++++++------
> ------------
>  src/compiler/glsl/glsl_parser.yy | 26 ++++++++++++++++++++++
>  3 files changed, 58 insertions(+), 22 deletions(-)
> 
> diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
> index ace92ed..15614cb 100644
> --- a/src/compiler/glsl/ast.h
> +++ b/src/compiler/glsl/ast.h
> @@ -766,6 +766,12 @@ struct ast_type_qualifier {
>                             const ast_type_qualifier &q,
>                             ast_node* &node, bool create_node);
>  
> +   /**
> +    * Push pending layout qualifiers to the global values.
> +    */
> +   bool push_to_global(YYLTYPE *loc,
> +                       _mesa_glsl_parse_state *state);
> +
>     bool validate_flags(YYLTYPE *loc,
>                         _mesa_glsl_parse_state *state,
>                         const ast_type_qualifier &allowed_flags,
> diff --git a/src/compiler/glsl/ast_type.cpp
> b/src/compiler/glsl/ast_type.cpp
> index 02f96ea..30f05aa 100644
> --- a/src/compiler/glsl/ast_type.cpp
> +++ b/src/compiler/glsl/ast_type.cpp
> @@ -272,29 +272,10 @@ ast_type_qualifier::merge_qualifier(YYLTYPE
> *loc,
>           }
>        }
>  
> -      if (q.flags.q.explicit_xfb_stride)
> +      if (q.flags.q.explicit_xfb_stride) {
> +         this->flags.q.xfb_stride = 1;
> +         this->flags.q.explicit_xfb_stride = 1;
>           this->xfb_stride = q.xfb_stride;
> -
> -      /* Merge all we xfb_stride qualifiers into the global out */
> -      if (q.flags.q.explicit_xfb_stride || this->flags.q.xfb_stride) 
> {
> -
> -         /* Set xfb_stride flag to 0 to avoid adding duplicates
> every time
> -          * there is a merge.
> -          */
> -         this->flags.q.xfb_stride = 0;
> -
> -         unsigned buff_idx;
> -         if (process_qualifier_constant(state, loc, "xfb_buffer",
> -                                        this->xfb_buffer,
> &buff_idx)) {
> -            if (state->out_qualifier->out_xfb_stride[buff_idx]
> -                && !is_single_layout_merge &&
> !is_multiple_layouts_merge) {
> -               state->out_qualifier->out_xfb_stride[buff_idx]-
> >merge_qualifier(
> -                  new(state) ast_layout_expression(*loc, this-
> >xfb_stride));
> -            } else {
> -               state->out_qualifier->out_xfb_stride[buff_idx] =
> -                  new(state) ast_layout_expression(*loc, this-
> >xfb_stride);
> -            }
> -         }
>        }
>     }
>  
> @@ -616,6 +597,29 @@ ast_type_qualifier::merge_in_qualifier(YYLTYPE
> *loc,
>     return true;
>  }
>  
> +bool
> +ast_type_qualifier::push_to_global(YYLTYPE *loc,
> +                                   _mesa_glsl_parse_state *state)
> +{
> +   if (this->flags.q.xfb_stride) {
> +      this->flags.q.xfb_stride = 0;
> +
> +      unsigned buff_idx;
> +      if (process_qualifier_constant(state, loc, "xfb_buffer",
> +                                     this->xfb_buffer, &buff_idx)) {
> +         if (state->out_qualifier->out_xfb_stride[buff_idx]) {
> +            state->out_qualifier->out_xfb_stride[buff_idx]-
> >merge_qualifier(
> +               new(state) ast_layout_expression(*loc, this-
> >xfb_stride));
> +         } else {
> +            state->out_qualifier->out_xfb_stride[buff_idx] =
> +               new(state) ast_layout_expression(*loc, this-
> >xfb_stride);
> +         }
> +      }
> +   }
> +
> +   return true;
> +}
> +
>  /**
>   * Check if the current type qualifier has any illegal flags.
>   *
> diff --git a/src/compiler/glsl/glsl_parser.yy
> b/src/compiler/glsl/glsl_parser.yy
> index 41a5a17..39641c8 100644
> --- a/src/compiler/glsl/glsl_parser.yy
> +++ b/src/compiler/glsl/glsl_parser.yy
> @@ -844,6 +844,9 @@ declaration:
>              YYERROR;
>           }
>        }
> +      if (!block->layout.push_to_global(& @1, state)) {
> +         YYERROR;
> +      }
>        $$ = $1;
>     }
>     ;
> @@ -919,6 +922,9 @@ parameter_declaration:
>     {
>        $$ = $2;
>        $$->type->qualifier = $1;
> +      if (!$$->type->qualifier.push_to_global(& @1, state)) {
> +         YYERROR;
> +      }
>     }
>     | parameter_qualifier parameter_type_specifier
>     {
> @@ -928,6 +934,9 @@ parameter_declaration:
>        $$->type = new(ctx) ast_fully_specified_type();
>        $$->type->set_location_range(@1, @2);
>        $$->type->qualifier = $1;
> +      if (!$$->type->qualifier.push_to_global(& @1, state)) {
> +         YYERROR;
> +      }
>        $$->type->specifier = $2;
>     }
>     ;
> @@ -1143,6 +1152,9 @@ fully_specified_type:
>        $$ = new(ctx) ast_fully_specified_type();
>        $$->set_location_range(@1, @2);
>        $$->qualifier = $1;
> +      if (!$$->qualifier.push_to_global(& @1, state)) {
> +         YYERROR;
> +      }
>        $$->specifier = $2;
>        if ($$->specifier->structure != NULL &&
>            $$->specifier->structure->is_declaration) {
> @@ -2889,6 +2901,10 @@ layout_defaults:
>               merge_qualifier(& @1, state, $1, false)) {
>           YYERROR;
>        }
> +      if (!state->default_uniform_qualifier->
> +             push_to_global(& @1, state)) {
> +         YYERROR;
> +      }
>     }
>     | layout_buffer_defaults
>     {
> @@ -2897,6 +2913,10 @@ layout_defaults:
>               merge_qualifier(& @1, state, $1, false)) {
>           YYERROR;
>        }
> +      if (!state->default_shader_storage_qualifier->
> +             push_to_global(& @1, state)) {
> +         YYERROR;
> +      }
>  
>        /* From the GLSL 4.50 spec, section 4.4.5:
>         *
> @@ -2915,6 +2935,9 @@ layout_defaults:
>               merge_in_qualifier(& @1, state, $1, $$, true)) {
>           YYERROR;
>        }
> +      if (!state->in_qualifier->push_to_global(& @1, state)) {
> +         YYERROR;
> +      }
>     }
>     | layout_out_defaults
>     {
> @@ -2923,5 +2946,8 @@ layout_defaults:
>               merge_out_qualifier(& @1, state, $1, $$, true)) {
>           YYERROR;
>        }
> +      if (!state->out_qualifier->push_to_global(& @1, state)) {
> +         YYERROR;
> +      }
>     }
>     ;