There's seems to be nothing in the GLSL (ES) specifications that diallow redeclaring a variable as both 'precise' and 'invariant' in the same statement. But the way the parse-rules are structured this fails to parse, because this is handled in single_declaration, which has an exhaustive list of qualifiers here, and it does not include an option with both.
So let's factor out the precise/invariant handling from the type_qualifier rule so we can reuse it, as there's some intricate subtleties here. For reference: glslangValidator already allows this. Signed-off-by: Erik Faye-Lund <erik.faye-l...@collabora.com> --- My yacc/bison skills aren't all that, so I wouldn't be too surprised if this the wrong approach. This is why I'm posting this as an RFC. I'm hoping someone can tell me if this is the right kind of approach or not. This fixes a failure to compile a shader that appears in virgl after an invariant output has been lowered to precise TGSI-opcodes, and re-emitted GLSL for it (in virglrenderer). The problematic shader comes from dEQP-GLES2.functional.shaders.invariance.highp.subexpression_precision_lowp. src/compiler/glsl/glsl_parser.yy | 77 +++++++++++++++----------------- 1 file changed, 36 insertions(+), 41 deletions(-) diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy index cb7376995d..042e654a26 100644 --- a/src/compiler/glsl/glsl_parser.yy +++ b/src/compiler/glsl/glsl_parser.yy @@ -187,6 +187,7 @@ static bool match_layout_qualifier(const char *s1, const char *s2, %type <node> statement_list %type <node> simple_statement %type <n> precision_qualifier +%type <type_qualifier> invariant_or_precise_qualifier %type <type_qualifier> type_qualifier %type <type_qualifier> auxiliary_storage_qualifier %type <type_qualifier> storage_qualifier @@ -1106,7 +1107,7 @@ single_declaration: $$->declarations.push_tail(&decl->link); state->symbols->add_variable(new(state) ir_variable(NULL, $2, ir_var_auto)); } - | INVARIANT variable_identifier + | invariant_or_precise_qualifier variable_identifier { void *ctx = state->linalloc; ast_declaration *decl = new(ctx) ast_declaration($2, NULL, NULL); @@ -1114,19 +1115,8 @@ single_declaration: $$ = new(ctx) ast_declarator_list(NULL); $$->set_location_range(@1, @2); - $$->invariant = true; - - $$->declarations.push_tail(&decl->link); - } - | PRECISE variable_identifier - { - void *ctx = state->linalloc; - ast_declaration *decl = new(ctx) ast_declaration($2, NULL, NULL); - decl->set_location(@2); - - $$ = new(ctx) ast_declarator_list(NULL); - $$->set_location_range(@1, @2); - $$->precise = true; + $$->invariant = $1.flags.q.invariant; + $$->precise = $1.flags.q.precise; $$->declarations.push_tail(&decl->link); } @@ -1889,7 +1879,7 @@ interpolation_qualifier: } ; -type_qualifier: +invariant_or_precise_qualifier: /* Single qualifiers */ INVARIANT { @@ -1901,31 +1891,7 @@ type_qualifier: memset(& $$, 0, sizeof($$)); $$.flags.q.precise = 1; } - | auxiliary_storage_qualifier - | storage_qualifier - | interpolation_qualifier - | layout_qualifier - | memory_qualifier - | subroutine_qualifier - | precision_qualifier - { - memset(&$$, 0, sizeof($$)); - $$.precision = $1; - } - - /* Multiple qualifiers: - * In GLSL 4.20, these can be specified in any order. In earlier versions, - * they appear in this order (see GLSL 1.50 section 4.7 & comments below): - * - * invariant interpolation auxiliary storage precision ...or... - * layout storage precision - * - * Each qualifier's rule ensures that the accumulated qualifiers on the right - * side don't contain any that must appear on the left hand side. - * For example, when processing a storage qualifier, we check that there are - * no auxiliary, interpolation, layout, invariant, or precise qualifiers to the right. - */ - | PRECISE type_qualifier + | PRECISE invariant_or_precise_qualifier { if ($2.flags.q.precise) _mesa_glsl_error(&@1, state, "duplicate \"precise\" qualifier"); @@ -1933,7 +1899,7 @@ type_qualifier: $$ = $2; $$.flags.q.precise = 1; } - | INVARIANT type_qualifier + | INVARIANT invariant_or_precise_qualifier { if ($2.flags.q.invariant) _mesa_glsl_error(&@1, state, "duplicate \"invariant\" qualifier"); @@ -1958,6 +1924,35 @@ type_qualifier: if (state->is_version(430, 300) && $$.flags.q.in) _mesa_glsl_error(&@1, state, "invariant qualifiers cannot be used with shader inputs"); } + +type_qualifier: + /* Single qualifiers */ + invariant_or_precise_qualifier + | auxiliary_storage_qualifier + | storage_qualifier + | interpolation_qualifier + | layout_qualifier + | memory_qualifier + | subroutine_qualifier + | precision_qualifier + { + memset(&$$, 0, sizeof($$)); + $$.precision = $1; + } + + /* Multiple qualifiers: + * In GLSL 4.20, these can be specified in any order. In earlier versions, + * they appear in this order (see GLSL 1.50 section 4.7 & comments below): + * + * invariant interpolation auxiliary storage precision ...or... + * layout storage precision + * + * Each qualifier's rule ensures that the accumulated qualifiers on the right + * side don't contain any that must appear on the left hand side. + * For example, when processing a storage qualifier, we check that there are + * no auxiliary, interpolation, layout, invariant, or precise qualifiers to the right. + */ + | interpolation_qualifier type_qualifier { /* Section 4.3 of the GLSL 1.40 specification states: -- 2.17.1 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev