[Mesa-dev] [PATCH/RFC] glsl: allow redeclaring variables as 'precise invariant'
Erik Faye-Lund
erik.faye-lund at collabora.com
Tue Oct 30 11:48:37 UTC 2018
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-lund at 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-lund at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list