On Mon, 2016-11-14 at 19:15 +0200, Andres Gomez wrote:
> Currently, the default in layout qualifier merge performs specific
> validation and merge.
>
> We want to split out the validation from the merge so they can be
> done
> independently.
>
> Additionally, for simplification, the direction of the validation and
> merge is changed so the ast_type_qualifier calling the method is the
> one validated and merged against the default in qualifier.
>
> Signed-off-by: Andres Gomez <ago...@igalia.com>
> ---
> src/compiler/glsl/ast.h | 16 +++--
> src/compiler/glsl/ast_type.cpp | 127 ++++++++++++++++++++++-------
> ----------
> src/compiler/glsl/glsl_parser.yy | 12 ++--
> 3 files changed, 93 insertions(+), 62 deletions(-)
>
> diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
> index 14936f1..62ccb9d 100644
> --- a/src/compiler/glsl/ast.h
> +++ b/src/compiler/glsl/ast.h
> @@ -768,10 +768,18 @@ struct ast_type_qualifier {
> _mesa_glsl_parse_state *state,
> ast_node* &node, bool create_node);
>
> - bool merge_in_qualifier(YYLTYPE *loc,
> - _mesa_glsl_parse_state *state,
> - const ast_type_qualifier &q,
> - ast_node* &node, bool create_node);
> + /**
> + * Validate current qualifier against the global in one.
> + */
> + bool validate_in_qualifier(YYLTYPE *loc,
> + _mesa_glsl_parse_state *state);
> +
> + /**
> + * Merge current qualifier into the global in one.
> + */
> + bool merge_into_in_qualifier(YYLTYPE *loc,
> + _mesa_glsl_parse_state *state,
> + ast_node* &node, bool create_node);
>
> bool validate_flags(YYLTYPE *loc,
> _mesa_glsl_parse_state *state,
> diff --git a/src/compiler/glsl/ast_type.cpp
> b/src/compiler/glsl/ast_type.cpp
> index aaf7838..803d952 100644
> --- a/src/compiler/glsl/ast_type.cpp
> +++ b/src/compiler/glsl/ast_type.cpp
> @@ -461,27 +461,25 @@
> ast_type_qualifier::merge_into_out_qualifier(YYLTYPE *loc,
> return r;
> }
>
> -ast_type_qualifier::merge_in_qualifier(YYLTYPE *loc,
> - _mesa_glsl_parse_state
> *state,
> - const ast_type_qualifier &q,
> - ast_node* &node, bool
> create_node)
> +bool
> +ast_type_qualifier::validate_in_qualifier(YYLTYPE *loc,
> + _mesa_glsl_parse_state
> *state)
> {
> - void *lin_ctx = state->linalloc;
> - bool create_gs_ast = false;
> - bool create_cs_ast = false;
> + bool r = true;
> ast_type_qualifier valid_in_mask;
> valid_in_mask.flags.i = 0;
>
> switch (state->stage) {
> case MESA_SHADER_TESS_EVAL:
> - if (q.flags.q.prim_type) {
> + if (this->flags.q.prim_type) {
> /* Make sure this is a valid input primitive type. */
> - switch (q.prim_type) {
> + switch (this->prim_type) {
> case GL_TRIANGLES:
> case GL_QUADS:
> case GL_ISOLINES:
> break;
> default:
> + r = false;
> _mesa_glsl_error(loc, state,
> "invalid tessellation evaluation "
> "shader input primitive type");
> @@ -495,9 +493,9 @@ ast_type_qualifier::merge_in_qualifier(YYLTYPE
> *loc,
> valid_in_mask.flags.q.point_mode = 1;
> break;
> case MESA_SHADER_GEOMETRY:
> - if (q.flags.q.prim_type) {
> + if (this->flags.q.prim_type) {
> /* Make sure this is a valid input primitive type. */
> - switch (q.prim_type) {
> + switch (this->prim_type) {
> case GL_POINTS:
> case GL_LINES:
> case GL_LINES_ADJACENCY:
> @@ -505,16 +503,13 @@ ast_type_qualifier::merge_in_qualifier(YYLTYPE
> *loc,
> case GL_TRIANGLES_ADJACENCY:
> break;
> default:
> + r = false;
> _mesa_glsl_error(loc, state,
> "invalid geometry shader input
> primitive type");
> break;
> }
> }
>
> - create_gs_ast |=
> - q.flags.q.prim_type &&
> - !state->in_qualifier->flags.q.prim_type;
> -
> valid_in_mask.flags.q.prim_type = 1;
> valid_in_mask.flags.q.invocations = 1;
> break;
> @@ -522,97 +517,121 @@ ast_type_qualifier::merge_in_qualifier(YYLTYPE
> *loc,
> valid_in_mask.flags.q.early_fragment_tests = 1;
> break;
> case MESA_SHADER_COMPUTE:
> - create_cs_ast |=
> - q.flags.q.local_size != 0 &&
> - state->in_qualifier->flags.q.local_size == 0;
> -
> valid_in_mask.flags.q.local_size = 7;
> valid_in_mask.flags.q.local_size_variable = 1;
> break;
> default:
> + r = false;
> _mesa_glsl_error(loc, state,
> "input layout qualifiers only valid in "
> - "geometry, fragment and compute shaders");
> + "geometry, tessellation, fragment and compute
> shaders");
> break;
> }
>
> /* Generate an error when invalid input layout qualifiers are
> used. */
> - if ((q.flags.i & ~valid_in_mask.flags.i) != 0) {
> + if ((this->flags.i & ~valid_in_mask.flags.i) != 0) {
> + r = false;
> _mesa_glsl_error(loc, state, "invalid input layout qualifiers
> used");
> - return false;
> + }
> +
> + return r;
> +}
> +
> +bool
> +ast_type_qualifier::merge_into_in_qualifier(YYLTYPE *loc,
> + _mesa_glsl_parse_state
> *state,
> + ast_node* &node, bool
> create_node)
> +{
> + void *lin_ctx = state->linalloc;
> + bool create_gs_ast = false;
> + bool create_cs_ast = false;
> +
> + switch (state->stage) {
> + case MESA_SHADER_GEOMETRY:
> + create_gs_ast |=
> + this->flags.q.prim_type &&
> + !state->in_qualifier->flags.q.prim_type;
> + break;
> + case MESA_SHADER_COMPUTE:
> + create_cs_ast |=
> + this->flags.q.local_size != 0 &&
> + state->in_qualifier->flags.q.local_size == 0;
> + break;
> + default:
> + break;
> }
>
> /* Input layout qualifiers can be specified multiple
> * times in separate declarations, as long as they match.
> */
> - if (this->flags.q.prim_type) {
> - if (q.flags.q.prim_type &&
> - this->prim_type != q.prim_type) {
> + if (state->in_qualifier->flags.q.prim_type) {
> + if (this->flags.q.prim_type &&
> + state->in_qualifier->prim_type != this->prim_type) {
> _mesa_glsl_error(loc, state,
> "conflicting input primitive %s
> specified",
> state->stage == MESA_SHADER_GEOMETRY ?
> "type" : "mode");
> }
> - } else if (q.flags.q.prim_type) {
> + } else if (this->flags.q.prim_type) {
> state->in_qualifier->flags.q.prim_type = 1;
> - state->in_qualifier->prim_type = q.prim_type;
> + state->in_qualifier->prim_type = this->prim_type;
> }
>
> - if (q.flags.q.invocations) {
> - this->flags.q.invocations = 1;
> - if (this->invocations) {
> - this->invocations->merge_qualifier(q.invocations);
> + if (this->flags.q.invocations) {
> + state->in_qualifier->flags.q.invocations = 1;
> + if (state->in_qualifier->invocations) {
> + state->in_qualifier->invocations->merge_qualifier(this-
> >invocations);
> } else {
> - this->invocations = q.invocations;
> + state->in_qualifier->invocations = this->invocations;
> }
> }
>
> - if (q.flags.q.early_fragment_tests) {
> + if (this->flags.q.early_fragment_tests) {
> state->fs_early_fragment_tests = true;
> }
>
> - if (this->flags.q.vertex_spacing) {
> - if (q.flags.q.vertex_spacing &&
> - this->vertex_spacing != q.vertex_spacing) {
> + if (state->in_qualifier->flags.q.vertex_spacing) {
> + if (this->flags.q.vertex_spacing &&
> + state->in_qualifier->vertex_spacing != this-
> >vertex_spacing) {
> _mesa_glsl_error(loc, state,
> "conflicting vertex spacing specified");
> }
> - } else if (q.flags.q.vertex_spacing) {
> - this->flags.q.vertex_spacing = 1;
> - this->vertex_spacing = q.vertex_spacing;
> + } else if (this->flags.q.vertex_spacing) {
> + state->in_qualifier->flags.q.vertex_spacing = 1;
> + state->in_qualifier->vertex_spacing = this->vertex_spacing;
> }
>
> - if (this->flags.q.ordering) {
> - if (q.flags.q.ordering &&
> - this->ordering != q.ordering) {
> + if (state->in_qualifier->flags.q.ordering) {
> + if (this->flags.q.ordering &&
> + state->in_qualifier->ordering != this->ordering) {
> _mesa_glsl_error(loc, state,
> "conflicting ordering specified");
> }
> - } else if (q.flags.q.ordering) {
> - this->flags.q.ordering = 1;
> - this->ordering = q.ordering;
> + } else if (this->flags.q.ordering) {
> + state->in_qualifier->flags.q.ordering = 1;
> + state->in_qualifier->ordering = this->ordering;
> }
>
> - if (this->flags.q.point_mode) {
> - if (q.flags.q.point_mode &&
> - this->point_mode != q.point_mode) {
> + if (state->in_qualifier->flags.q.point_mode) {
> + if (this->flags.q.point_mode &&
> + state->in_qualifier->point_mode != this->point_mode) {
> _mesa_glsl_error(loc, state,
> "conflicting point mode specified");
> }
> - } else if (q.flags.q.point_mode) {
> - this->flags.q.point_mode = 1;
> - this->point_mode = q.point_mode;
> + } else if (this->flags.q.point_mode) {
> + state->in_qualifier->flags.q.point_mode = 1;
> + state->in_qualifier->point_mode = this->point_mode;
> }
>
> - if (q.flags.q.local_size_variable) {
> + if (this->flags.q.local_size_variable) {
> state->cs_input_local_size_variable_specified = true;
> }
>
> if (create_node) {
> if (create_gs_ast) {
> - node = new(lin_ctx) ast_gs_input_layout(*loc, q.prim_type);
> + node = new(lin_ctx) ast_gs_input_layout(*loc, this-
> >prim_type);
> } else if (create_cs_ast) {
> - node = new(lin_ctx) ast_cs_input_layout(*loc,
> q.local_size);
> + node = new(lin_ctx) ast_cs_input_layout(*loc, this-
> >local_size);
> }
> }
>
> diff --git a/src/compiler/glsl/glsl_parser.yy
> b/src/compiler/glsl/glsl_parser.yy
> index 50f7097..9f18c15 100644
> --- a/src/compiler/glsl/glsl_parser.yy
> +++ b/src/compiler/glsl/glsl_parser.yy
> @@ -2913,8 +2913,10 @@ layout_in_defaults:
> _mesa_glsl_error(&@1, state, "duplicate layout(...)
> qualifiers");
> YYERROR;
> } else {
> - if (!state->in_qualifier->
> - merge_in_qualifier(& @1, state, $1, $$, false)) {
> + if (!$1.validate_in_qualifier(& @1, state)) {
> + YYERROR;
> + }
> + if (!$1.merge_into_in_qualifier(& @1, state, $$, false)) {
> YYERROR;
> }
To avoid regressions between patches the order should be merge then
validate right?
> $$ = $2;
> @@ -2923,8 +2925,10 @@ layout_in_defaults:
> | layout_qualifier IN_TOK ';'
> {
> $$ = NULL;
> - if (!state->in_qualifier->
> - merge_in_qualifier(& @1, state, $1, $$, true)) {
> + if (!$1.validate_in_qualifier(& @1, state)) {
> + YYERROR;
> + }
> + if (!$1.merge_into_in_qualifier(& @1, state, $$, true)) {
> YYERROR;
> }
> }
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
