Ping? (Added Timothy the the CC-list, as he's the most recent person to modify the parser, it seems)
On Wed, 2018-08-29 at 13:59 +0200, Erik Faye-Lund wrote: > Ping? > > On on., aug. 22, 2018 at 7:34 PM, Erik Faye-Lund < > erik.faye-l...@collabora.com> wrote: > > 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_l > > owp. > > > > 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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev