[Mesa-dev] [PATCH 3/6] glsl: add always_active_io attribute to ir_variable

[Tapani Pälli]

Reviewed-by: Tapani Pälli <tapani.palli at intel.com>

(some typos and tiny code nit noted below)

On 11/25/2015 11:54 AM, Timothy Arceri wrote:
> From: Gregory Hainaut <gregory.hainaut at gmail.com>
>
> The value will be set in separate-shader program when an input/output
> must remains active. e.g. when deadcode removal isn't allowed because
> it will create interface location/name-matching mismatch.
>
> v3:
> * Rename the attribute
> * Use ir_variable directly instead of ir_variable_refcount_visitor
> * Move the foreach IR code in the linker file
>
> v4:
> * Fix variable name in assert
>
> v5 (by Timothy Arceri):
> * Rename functions and reword comments
> * Don't set alway active on builtins

alway -> always

>
> Signed-off-by: Gregory Hainaut <gregory.hainaut at gmail.com>
> Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>
> ---
>   src/glsl/ir.cpp     |  1 +
>   src/glsl/ir.h       |  7 +++++
>   src/glsl/linker.cpp | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 82 insertions(+)
>
> diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
> index ca520f5..f989e9b 100644
> --- a/src/glsl/ir.cpp
> +++ b/src/glsl/ir.cpp
> @@ -1669,6 +1669,7 @@ ir_variable::ir_variable(const struct glsl_type *type, const char *name,
>      this->data.pixel_center_integer = false;
>      this->data.depth_layout = ir_depth_layout_none;
>      this->data.used = false;
> +   this->data.always_active_io = false;
>      this->data.read_only = false;
>      this->data.centroid = false;
>      this->data.sample = false;
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index 20c94a1..717b036 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -659,6 +659,13 @@ public:
>         unsigned assigned:1;
>
>         /**
> +       * When separate shader programs are enabled, only input/outputs between
> +       * the stages of a multi-stage separate program can be safely removed
> +       * from the shader interface. Other input/outputs must remains active.
> +       */
> +      unsigned always_active_io:1;
> +
> +      /**
>          * Enum indicating how the variable was declared.  See
>          * ir_var_declaration_type.
>          *
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 930b585..4745e86 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -3988,6 +3988,77 @@ split_ubos_and_ssbos(void *mem_ctx,
>      assert(*num_ubos + *num_ssbos == num_blocks);
>   }
>
> +static void
> +set_always_active_io(exec_list *ir, ir_variable_mode io_mode)
> +{
> +   assert(io_mode == ir_var_shader_in || io_mode == ir_var_shader_out);
> +
> +   foreach_in_list(ir_instruction, node, ir) {
> +      ir_variable *const var = node->as_variable();
> +
> +      if (var == NULL || var->data.mode != io_mode)
> +         continue;
> +
> +      /* Don't set alway active on builtins that haven't been redeclared */

alway -> always

> +      if(var->data.how_declared == ir_var_declared_implicitly)

space after if

> +         continue;
> +
> +      var->data.always_active_io = true;
> +   }
> +}
> +
> +/**
> + * When separate shader programs are enabled, only input/outputs between
> + * the stages of a multi-stage separate program can be safely removed
> + * from the shader interface. Other input/outputs must remains active.

input -> inputs
remains -> remain

> + */
> +static void
> +disable_varying_optimizations_for_sso(struct gl_shader_program *prog)
> +{
> +   unsigned first, last;
> +   assert(prog->SeparateShader);
> +
> +   first = MESA_SHADER_STAGES;
> +   last = 0;
> +
> +   /* Determine first and last stage. Excluding the compute stage */
> +   for (unsigned i = 0; i < MESA_SHADER_COMPUTE; i++) {
> +      if (!prog->_LinkedShaders[i])
> +         continue;
> +      if (first == MESA_SHADER_STAGES)
> +         first = i;
> +      last = i;
> +   }
> +
> +   if (first == MESA_SHADER_STAGES)
> +      return;
> +
> +   for (unsigned stage = 0; stage < MESA_SHADER_STAGES; stage++) {
> +      gl_shader *sh = prog->_LinkedShaders[stage];
> +      if (!sh)
> +         continue;
> +
> +      if (first == last) {
> +         /* For a single shader program only allow inputs to the vertex shader
> +          * and outputs from the fragment shader to be removed.
> +          */
> +         if (stage != MESA_SHADER_VERTEX)
> +            set_always_active_io(sh->ir, ir_var_shader_in);
> +         if (stage != MESA_SHADER_FRAGMENT)
> +            set_always_active_io(sh->ir, ir_var_shader_out);
> +      } else {
> +         /* For multi-stage separate shader programs only allow inputs and
> +          * outputs between the shader stages to be removed as well as inputs
> +          * to the vertex shader and outputs from the fragment shader.
> +          */
> +         if (stage == first && stage != MESA_SHADER_VERTEX)
> +            set_always_active_io(sh->ir, ir_var_shader_in);
> +         else if (stage == last && stage != MESA_SHADER_FRAGMENT)
> +            set_always_active_io(sh->ir, ir_var_shader_out);
> +      }
> +   }
> +}
> +
>   void
>   link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>   {
> @@ -4255,6 +4326,9 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>         }
>      }
>
> +   if (prog->SeparateShader)
> +      disable_varying_optimizations_for_sso(prog);
> +
>      if (!interstage_cross_validate_uniform_blocks(prog))
>         goto done;
>
>