[Mesa-dev] [PATCH 2/5] glsl: last duplicated layout-qualifier-name in multiple layout-qualifiers overrides the former
Andres Gomez
agomez at igalia.com
Thu Oct 6 22:52:05 UTC 2016
>From page 46 (page 52 of the PDF) of the GLSL 4.20 spec:
" More than one layout qualifier may appear in a single
declaration. If the same layout-qualifier-name occurs in multiple
layout qualifiers for the same declaration, the last one overrides
the former ones."
Consider this example:
" #version 150
#extension GL_ARB_shading_language_420pack: enable
#extension GL_ARB_enhanced_layouts: enable
layout(max_vertices=2) layout(max_vertices=3) out;
layout(max_vertices=3) out;"
Although different values for the "max_vertices" layout-qualifier-name
should end in a compilation failure, since only the last occurrence is
taken into account, this small piece of code from a shader is valid.
Hence, when merging qualifiers in an ast_type_qualifier, we now ignore
new appearances of a same layout-qualifier-name if the new
"is_multiple_layouts_merge" parameter is on, since the GLSL parser
works in this case from right to left.
Also, the GLSL parser has been simplified to check for the needed
GL_ARB_shading_language_420pack extension just when merging the
qualifiers in the proper cases.
In addition, any special treatment for the buffer, uniform, in or out
layout defaults has been moved in the GLSL parser to the rule
triggered just after any previous processing/merging on the
layout-qualifiers has happened in a single declaration since it was
run too soon previously.
Finally, the merging of an ast_layout_expression is now done
prepending instead of appending since the processing of the qualifier
constant returns the first value in the list and the last appearing
declaration of a variable or default overrides the previous
declarations.
Fixes GL44-CTS.shading_language_420pack.qualifier_override_layout
Signed-off-by: Andres Gomez <agomez at igalia.com>
---
src/compiler/glsl/ast.h | 5 +-
src/compiler/glsl/ast_type.cpp | 34 +++++++---
src/compiler/glsl/glsl_parser.yy | 131 +++++++++++++++------------------------
3 files changed, 77 insertions(+), 93 deletions(-)
diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
index 4c648d0..73c73b7 100644
--- a/src/compiler/glsl/ast.h
+++ b/src/compiler/glsl/ast.h
@@ -382,7 +382,7 @@ public:
void merge_qualifier(ast_layout_expression *l_expr)
{
- layout_const_expressions.append_list(&l_expr->layout_const_expressions);
+ layout_const_expressions.prepend_list(&l_expr->layout_const_expressions);
}
exec_list layout_const_expressions;
@@ -746,7 +746,8 @@ struct ast_type_qualifier {
bool merge_qualifier(YYLTYPE *loc,
_mesa_glsl_parse_state *state,
const ast_type_qualifier &q,
- bool is_single_layout_merge);
+ bool is_single_layout_merge,
+ bool is_multiple_layouts_merge = false);
bool merge_out_qualifier(YYLTYPE *loc,
_mesa_glsl_parse_state *state,
diff --git a/src/compiler/glsl/ast_type.cpp b/src/compiler/glsl/ast_type.cpp
index 504b533..f02f71b 100644
--- a/src/compiler/glsl/ast_type.cpp
+++ b/src/compiler/glsl/ast_type.cpp
@@ -108,15 +108,21 @@ ast_type_qualifier::has_auxiliary_storage() const
}
/**
- * This function merges both duplicate identifies within a single layout and
- * multiple layout qualifiers on a single variable declaration. The
- * is_single_layout_merge param is used differentiate between the two.
+ * This function merges duplicate layout identifiers.
+ *
+ * It deals with duplicates within a single layout qualifier, among multiple
+ * layout qualifiers on a single declaration and on several declarations for
+ * the same variable.
+ *
+ * The is_single_layout_merge and is_multiple_layouts_merge parameters are
+ * used to differentiate among them.
*/
bool
ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
_mesa_glsl_parse_state *state,
const ast_type_qualifier &q,
- bool is_single_layout_merge)
+ bool is_single_layout_merge,
+ bool is_multiple_layouts_merge)
{
ast_type_qualifier ubo_mat_mask;
ubo_mat_mask.flags.i = 0;
@@ -186,6 +192,12 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
return false;
}
+ if (is_multiple_layouts_merge && !state->has_420pack_or_es31()) {
+ _mesa_glsl_error(loc, state,
+ "duplicate layout(...) qualifiers");
+ return false;
+ }
+
if (q.flags.q.prim_type) {
if (this->flags.q.prim_type && this->prim_type != q.prim_type) {
_mesa_glsl_error(loc, state,
@@ -196,7 +208,8 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
}
if (q.flags.q.max_vertices) {
- if (this->max_vertices && !is_single_layout_merge) {
+ if (this->max_vertices
+ && !is_single_layout_merge && !is_multiple_layouts_merge) {
this->max_vertices->merge_qualifier(q.max_vertices);
} else {
this->max_vertices = q.max_vertices;
@@ -213,7 +226,8 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
}
if (q.flags.q.invocations) {
- if (this->invocations && !is_single_layout_merge) {
+ if (this->invocations
+ && !is_single_layout_merge && !is_multiple_layouts_merge) {
this->invocations->merge_qualifier(q.invocations);
} else {
this->invocations = q.invocations;
@@ -263,7 +277,7 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
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_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 {
@@ -275,7 +289,8 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
}
if (q.flags.q.vertices) {
- if (this->vertices && !is_single_layout_merge) {
+ if (this->vertices
+ && !is_single_layout_merge && !is_multiple_layouts_merge) {
this->vertices->merge_qualifier(q.vertices);
} else {
this->vertices = q.vertices;
@@ -313,7 +328,8 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
for (int i = 0; i < 3; i++) {
if (q.flags.q.local_size & (1 << i)) {
- if (this->local_size[i] && !is_single_layout_merge) {
+ if (this->local_size[i]
+ && !is_single_layout_merge && !is_multiple_layouts_merge) {
this->local_size[i]->merge_qualifier(q.local_size[i]);
} else {
this->local_size[i] = q.local_size[i];
diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
index 9e1fd9e..f4bf8fc 100644
--- a/src/compiler/glsl/glsl_parser.yy
+++ b/src/compiler/glsl/glsl_parser.yy
@@ -297,10 +297,10 @@ static bool match_layout_qualifier(const char *s1, const char *s2,
%type <node> for_init_statement
%type <for_rest_statement> for_rest_statement
%type <node> layout_defaults
-%type <node> layout_uniform_defaults
-%type <node> layout_buffer_defaults
-%type <node> layout_in_defaults
-%type <node> layout_out_defaults
+%type <type_qualifier> layout_uniform_defaults
+%type <type_qualifier> layout_buffer_defaults
+%type <type_qualifier> layout_in_defaults
+%type <type_qualifier> layout_out_defaults
%right THEN ELSE
%%
@@ -1862,11 +1862,8 @@ type_qualifier:
* precise qualifiers since these are useful in ARB_separate_shader_objects.
* There is no clear spec guidance on this either.
*/
- if (!state->has_420pack_or_es31() && $2.has_layout())
- _mesa_glsl_error(&@1, state, "duplicate layout(...) qualifiers");
-
$$ = $1;
- $$.merge_qualifier(&@1, state, $2, false);
+ $$.merge_qualifier(&@1, state, $2, false, $2.has_layout());
}
| subroutine_qualifier type_qualifier
{
@@ -2688,13 +2685,9 @@ interface_block:
{
ast_interface_block *block = (ast_interface_block *) $2;
- if (!state->has_420pack_or_es31() && block->layout.has_layout() &&
- !block->layout.is_default_qualifier) {
- _mesa_glsl_error(&@1, state, "duplicate layout(...) qualifiers");
- YYERROR;
- }
-
- if (!block->layout.merge_qualifier(& @1, state, $1, false)) {
+ if (!block->layout.merge_qualifier(& @1, state, $1, false,
+ block->layout.has_layout() &&
+ !block->layout.is_default_qualifier)) {
YYERROR;
}
@@ -2828,43 +2821,59 @@ member_declaration:
layout_uniform_defaults:
layout_qualifier layout_uniform_defaults
{
- $$ = NULL;
- if (!state->has_420pack_or_es31()) {
- _mesa_glsl_error(&@1, state, "duplicate layout(...) qualifiers");
+ $$ = $1;
+ if (!$$.merge_qualifier(& @2, state, $2, false, true)) {
YYERROR;
- } else {
- if (!state->default_uniform_qualifier->
- merge_qualifier(& @1, state, $1, false)) {
- YYERROR;
- }
}
}
| layout_qualifier UNIFORM ';'
+ ;
+
+layout_buffer_defaults:
+ layout_qualifier layout_buffer_defaults
{
- if (!state->default_uniform_qualifier->
- merge_qualifier(& @1, state, $1, false)) {
+ $$ = $1;
+ if (!$$.merge_qualifier(& @2, state, $2, false, true)) {
YYERROR;
}
- $$ = NULL;
}
+ | layout_qualifier BUFFER ';'
;
-layout_buffer_defaults:
- layout_qualifier layout_buffer_defaults
+layout_in_defaults:
+ layout_qualifier layout_in_defaults
+ {
+ $$ = $1;
+ if (!$$.merge_qualifier(& @2, state, $2, false, true)) {
+ YYERROR;
+ }
+ }
+ | layout_qualifier IN_TOK ';'
+ ;
+
+layout_out_defaults:
+ layout_qualifier layout_out_defaults
+ {
+ $$ = $1;
+ if (!$$.merge_qualifier(& @2, state, $2, false, true)) {
+ YYERROR;
+ }
+ }
+ | layout_qualifier OUT_TOK ';'
+ ;
+
+layout_defaults:
+ layout_uniform_defaults
{
$$ = NULL;
- if (!state->has_420pack_or_es31()) {
- _mesa_glsl_error(&@1, state, "duplicate layout(...) qualifiers");
+ if (!state->default_uniform_qualifier->
+ merge_qualifier(& @1, state, $1, false)) {
YYERROR;
- } else {
- if (!state->default_shader_storage_qualifier->
- merge_qualifier(& @1, state, $1, false)) {
- YYERROR;
- }
}
}
- | layout_qualifier BUFFER ';'
+ | layout_buffer_defaults
{
+ $$ = NULL;
if (!state->default_shader_storage_qualifier->
merge_qualifier(& @1, state, $1, false)) {
YYERROR;
@@ -2879,27 +2888,8 @@ layout_buffer_defaults:
_mesa_glsl_error(& @1, state,
"binding qualifier cannot be set for default layout");
}
-
- $$ = NULL;
}
- ;
-
-layout_in_defaults:
- layout_qualifier layout_in_defaults
- {
- $$ = NULL;
- if (!state->has_420pack_or_es31()) {
- _mesa_glsl_error(&@1, state, "duplicate layout(...) qualifiers");
- YYERROR;
- } else {
- if (!state->in_qualifier->
- merge_in_qualifier(& @1, state, $1, $$, false)) {
- YYERROR;
- }
- $$ = $2;
- }
- }
- | layout_qualifier IN_TOK ';'
+ | layout_in_defaults
{
$$ = NULL;
if (!state->in_qualifier->
@@ -2907,35 +2897,12 @@ layout_in_defaults:
YYERROR;
}
}
- ;
-
-layout_out_defaults:
- layout_qualifier layout_out_defaults
- {
- $$ = NULL;
- if (!state->has_420pack_or_es31()) {
- _mesa_glsl_error(&@1, state, "duplicate layout(...) qualifiers");
- YYERROR;
- } else {
- if (!state->out_qualifier->
- merge_out_qualifier(& @1, state, $1, $$, false)) {
- YYERROR;
- }
- $$ = $2;
- }
- }
- | layout_qualifier OUT_TOK ';'
+ | layout_out_defaults
{
$$ = NULL;
if (!state->out_qualifier->
- merge_out_qualifier(& @1, state, $1, $$, true))
+ merge_out_qualifier(& @1, state, $1, $$, true)) {
YYERROR;
+ }
}
;
-
-layout_defaults:
- layout_uniform_defaults
- | layout_buffer_defaults
- | layout_in_defaults
- | layout_out_defaults
- ;
--
2.9.3
More information about the mesa-dev
mailing list