Re: [Mesa-dev] [PATCH 5/7] glsl: add always_active_io attribute to ir_variable

[Timothy Arceri]

On Sat, 2015-11-21 at 19:02 +1100, Timothy Arceri wrote:
> From: Gregory Hainaut 
> 
> 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
> 
> Signed-off-by: Gregory Hainaut 
> Reviewed-by: Timothy Arceri 
> ---
>  src/glsl/ir.cpp |  1 +
>  src/glsl/ir.h   |  7 ++
>  src/glsl/linker.cpp | 72
> +
>  3 files changed, 80 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 386c993..6eb703b 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 1db7b7a..9b6efa9 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -3988,6 +3988,75 @@ 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();
> +
> +  /* Don't set alway active on builtins */
> +  if (var == NULL || var->data.mode != io_mode ||
> +  var->data.how_declared == ir_var_declared_implicitly)
> + continue;

Note: I've changed this locally for clarity to:

  if (var == NULL || var->data.mode != io_mode)
 continue;

  /* Don't set alway active on builtins that haven't been redeclared */
  if(var->data.how_declared == ir_var_declared_implicitly)
 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.
> + */
> +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-dev] [PATCH 5/7] glsl: add always_active_io attribute to ir_variable

[Timothy Arceri]

From: Gregory Hainaut 

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

Signed-off-by: Gregory Hainaut 
Reviewed-by: Timothy Arceri 
---
 src/glsl/ir.cpp |  1 +
 src/glsl/ir.h   |  7 ++
 src/glsl/linker.cpp | 72 +
 3 files changed, 80 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 386c993..6eb703b 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 1db7b7a..9b6efa9 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -3988,6 +3988,75 @@ 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();
+
+  /* Don't set alway active on builtins */
+  if (var == NULL || var->data.mode != io_mode ||
+  var->data.how_declared == ir_var_declared_implicitly)
+ 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.
+ */
+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 +4324,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;
 
-- 
2.4.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev