Mesa (master): glsl: disable varying packing for varying used by interpolateAt*

[Timothy Arceri]

Module: Mesa
Branch: master
Commit: eb8aa93c03ee89ffd3041d41b6293e4b282b6ce6
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=eb8aa93c03ee89ffd3041d41b6293e4b282b6ce6

Author: Timothy Arceri <tarceri at itsqueeze.com>
Date:   Fri Apr 14 16:25:58 2017 +1000

glsl: disable varying packing for varying used by interpolateAt*

Currently the NIR backends depend on GLSL IR copy propagation to
fix up the interpolateAt* function params after varying packing
changes the shader input to a global. It's possible copy propagation
might not always do what we need it too, and we also shouldn't
depend on optimisations to do this type of thing for us.

I'm not sure if the same is true for TGSI, but the following
commit should re-enable packing for most cases in a safer way,
so we just disable it everywhere.

No change in shader-db for i965 (BDW)

Acked-by: Elie Tournier <elie.tournier at collabora.com>
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

---

 src/compiler/glsl/ast_function.cpp          |  2 ++
 src/compiler/glsl/link_varyings.cpp         | 17 +++++++++++++----
 src/compiler/glsl/lower_packed_varyings.cpp |  7 ++++---
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/src/compiler/glsl/ast_function.cpp b/src/compiler/glsl/ast_function.cpp
index 9f03f32148..1b90937ec8 100644
--- a/src/compiler/glsl/ast_function.cpp
+++ b/src/compiler/glsl/ast_function.cpp
@@ -235,6 +235,8 @@ verify_parameter_modes(_mesa_glsl_parse_state *state,
                              formal->name);
             return false;
          }
+
+         val->variable_referenced()->data.must_be_shader_input = 1;
       }
 
       /* Verify that 'out' and 'inout' actual parameters are lvalues. */
diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
index e6947692af..6bcec72f5f 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -1461,17 +1461,24 @@ varying_matches::record(ir_variable *producer_var, ir_variable *consumer_var)
       ? consumer_stage : producer_stage;
    const glsl_type *type = get_varying_type(var, stage);
 
+   if (producer_var && consumer_var &&
+       consumer_var->data.must_be_shader_input) {
+      producer_var->data.must_be_shader_input = 1;
+   }
+
    this->matches[this->num_matches].packing_class
       = this->compute_packing_class(var);
    this->matches[this->num_matches].packing_order
       = this->compute_packing_order(var);
-   if (this->disable_varying_packing && !is_varying_packing_safe(type, var)) {
+   if ((this->disable_varying_packing && !is_varying_packing_safe(type, var)) ||
+       var->data.must_be_shader_input) {
       unsigned slots = type->count_attribute_slots(false);
       this->matches[this->num_matches].num_components = slots * 4;
    } else {
       this->matches[this->num_matches].num_components
          = type->component_slots();
    }
+
    this->matches[this->num_matches].producer_var = producer_var;
    this->matches[this->num_matches].consumer_var = consumer_var;
    this->num_matches++;
@@ -1544,7 +1551,8 @@ varying_matches::assign_locations(struct gl_shader_program *prog,
        * we can pack varyings together that are only used for transform
        * feedback.
        */
-      if ((this->disable_varying_packing &&
+      if (var->data.must_be_shader_input ||
+          (this->disable_varying_packing &&
            !(previous_var_xfb_only && var->data.is_xfb_only)) ||
           (i > 0 && this->matches[i - 1].packing_class
           != this->matches[i].packing_class )) {
@@ -1660,8 +1668,9 @@ varying_matches::compute_packing_class(const ir_variable *var)
     * Therefore, the packing class depends only on the interpolation type.
     */
    unsigned packing_class = var->data.centroid | (var->data.sample << 1) |
-                            (var->data.patch << 2);
-   packing_class *= 4;
+                            (var->data.patch << 2) |
+                            (var->data.must_be_shader_input << 3);
+   packing_class *= 8;
    packing_class += var->is_interpolation_flat()
       ? unsigned(INTERP_MODE_FLAT) : var->data.interpolation;
    return packing_class;
diff --git a/src/compiler/glsl/lower_packed_varyings.cpp b/src/compiler/glsl/lower_packed_varyings.cpp
index 13f7e5b52d..b1a3b49b1d 100644
--- a/src/compiler/glsl/lower_packed_varyings.cpp
+++ b/src/compiler/glsl/lower_packed_varyings.cpp
@@ -742,10 +742,11 @@ lower_packed_varyings_visitor::get_packed_varying_deref(
 bool
 lower_packed_varyings_visitor::needs_lowering(ir_variable *var)
 {
-   /* Things composed of vec4's and varyings with explicitly assigned
-    * locations don't need lowering.  Everything else does.
+   /* Things composed of vec4's, varyings with explicitly assigned
+    * locations or varyings marked as must_be_shader_input (which might be used
+    * by interpolateAt* functions) shouldn't be lowered. Everything else can be.
     */
-   if (var->data.explicit_location)
+   if (var->data.explicit_location || var->data.must_be_shader_input)
       return false;
 
    /* Override disable_varying_packing if the var is only used by transform