<div id="geary-body" dir="auto"><div>Ping?</div></div><div id="geary-quote" dir="auto"><br>On on., aug. 22, 2018 at 7:34 PM, Erik Faye-Lund <erik.faye-lund@collabora.com> wrote:<br><blockquote type="cite"><div class="plaintext" style="white-space: pre-wrap;">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 <<a href="mailto:erik.faye-lund@collabora.com">erik.faye-lund@collabora.com</a>>
---

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:
<div>-- 
</div>2.17.1

</div></blockquote></div>