glsl: Cross validate variable's invariance by explicit invariance only

Submitted by Danylo Piliaiev on Sept. 4, 2018, 11:44 a.m.

Details

Message ID 20180904114516.8126-1-danylo.piliaiev@globallogic.com
State New
Headers show
Series "glsl: Cross validate variable's invariance by explicit invariance only" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Danylo Piliaiev Sept. 4, 2018, 11:44 a.m.
'invariant' qualifier is propagated on variables which are used
to calculate other invariant variables, however when we are matching
variable's declarations we should take into account only explicitly
declared invariance because invariance propagation is an implementation
specific detail.

Thus new flag is added to ir_variable_data which indicates 'invariant'
qualifier being explicitly set in the shader.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100316

Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
---
I'll send additional piglit test to test fixed case with invariance
propagation when there are several shaders in one stage.
At the moment there is a piglit test which tests other case fixed
by this patch: 
"[PATCH] glsl-1.20: test that 'invariant' qualifier does not propagate on uniforms"

 src/compiler/glsl/ast_to_hir.cpp    |  8 ++++++--
 src/compiler/glsl/ir.cpp            |  1 +
 src/compiler/glsl/ir.h              | 13 +++++++++++++
 src/compiler/glsl/ir_reader.cpp     |  3 ++-
 src/compiler/glsl/link_varyings.cpp |  6 +++---
 src/compiler/glsl/linker.cpp        |  2 +-
 6 files changed, 26 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 5d3f10b682..a535707449 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3932,7 +3932,8 @@  apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
                           "`invariant' after being used",
                           var->name);
       } else {
-         var->data.invariant = 1;
+         var->data.explicit_invariant = true;
+         var->data.invariant = true;
       }
    }
 
@@ -4140,8 +4141,10 @@  apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
       }
    }
 
-   if (state->all_invariant && var->data.mode == ir_var_shader_out)
+   if (state->all_invariant && var->data.mode == ir_var_shader_out) {
+      var->data.explicit_invariant = true;
       var->data.invariant = true;
+   }
 
    var->data.interpolation =
       interpret_interpolation_qualifier(qual, var->type,
@@ -4852,6 +4855,7 @@  ast_declarator_list::hir(exec_list *instructions,
                             "`invariant' after being used",
                             earlier->name);
          } else {
+            earlier->data.explicit_invariant = true;
             earlier->data.invariant = true;
          }
       }
diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
index 1d1a56ae9a..f5aa1be4e2 100644
--- a/src/compiler/glsl/ir.cpp
+++ b/src/compiler/glsl/ir.cpp
@@ -1734,6 +1734,7 @@  ir_variable::ir_variable(const struct glsl_type *type, const char *name,
    this->data.centroid = false;
    this->data.sample = false;
    this->data.patch = false;
+   this->data.explicit_invariant = false;
    this->data.invariant = false;
    this->data.how_declared = ir_var_declared_normally;
    this->data.mode = mode;
diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
index f478b29a6b..7e4b500b66 100644
--- a/src/compiler/glsl/ir.h
+++ b/src/compiler/glsl/ir.h
@@ -657,6 +657,19 @@  public:
       unsigned centroid:1;
       unsigned sample:1;
       unsigned patch:1;
+      /**
+       * Was an 'invariant' qualifier explicitly set in the shader?
+       *
+       * This is used to cross validate qualifiers.
+       */
+      unsigned explicit_invariant:1;
+      /**
+       * Is the variable invariant?
+       *
+       * It can happen either by having the 'invariant' qualifier
+       * explicitly set in the shader or by being used in calculations
+       * of other invariant variables.
+       */
       unsigned invariant:1;
       unsigned precise:1;
 
diff --git a/src/compiler/glsl/ir_reader.cpp b/src/compiler/glsl/ir_reader.cpp
index b87933ba51..70677c5215 100644
--- a/src/compiler/glsl/ir_reader.cpp
+++ b/src/compiler/glsl/ir_reader.cpp
@@ -420,7 +420,8 @@  ir_reader::read_declaration(s_expression *expr)
       } else if (strcmp(qualifier->value(), "patch") == 0) {
          var->data.patch = 1;
       } else if (strcmp(qualifier->value(), "invariant") == 0) {
-	 var->data.invariant = 1;
+        var->data.explicit_invariant = true;
+        var->data.invariant = true;
       } else if (strcmp(qualifier->value(), "uniform") == 0) {
 	 var->data.mode = ir_var_uniform;
       } else if (strcmp(qualifier->value(), "shader_storage") == 0) {
diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
index 211633d9ee..28bb2ca0e3 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -308,16 +308,16 @@  cross_validate_types_and_qualifiers(struct gl_context *ctx,
     *    "The invariance of varyings that are declared in both the vertex
     *     and fragment shaders must match."
     */
-   if (input->data.invariant != output->data.invariant &&
+   if (input->data.explicit_invariant != output->data.explicit_invariant &&
        prog->data->Version < (prog->IsES ? 300 : 430)) {
       linker_error(prog,
                    "%s shader output `%s' %s invariant qualifier, "
                    "but %s shader input %s invariant qualifier\n",
                    _mesa_shader_stage_to_string(producer_stage),
                    output->name,
-                   (output->data.invariant) ? "has" : "lacks",
+                   (output->data.explicit_invariant) ? "has" : "lacks",
                    _mesa_shader_stage_to_string(consumer_stage),
-                   (input->data.invariant) ? "has" : "lacks");
+                   (input->data.explicit_invariant) ? "has" : "lacks");
       return;
    }
 
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index f08971d780..5777bc8a2d 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1089,7 +1089,7 @@  cross_validate_globals(struct gl_context *ctx, struct gl_shader_program *prog,
             }
          }
 
-         if (existing->data.invariant != var->data.invariant) {
+         if (existing->data.explicit_invariant != var->data.explicit_invariant) {
             linker_error(prog, "declarations for %s `%s' have "
                          "mismatching invariant qualifiers\n",
                          mode_string(var), var->name);