[Mesa-dev] [PATCH] glsl: remove unused varyings before packing them

[Timothy Arceri]

On Tue, 2015-12-22 at 11:25 +1100, Timothy Arceri wrote:
> Previously we would pack varyings before trying to remove them, this
> meant the varying would only be removed if all the varyings in the
> packed
> location were unused or if the varying wasn't packed.
> 
> This change makes no difference to the public shader-db.

Looking at this some more it seems the packing pass was relying on
these varyings to not be given a location and therefore they didn't get
packed.

I'd still like to go ahead with this patch as its a good clean-up and
will make it easier to work on some inprovements to packing if we just
get rid of these as early as possible. 

I'll update the commit message to something like:

Previously we packed varyings before trying to remove them, the packing
pass relied on these unused varyings having a location of -1 in order
to not pack them. This change makes the code eaiser to follow and gets
rid of the varyings earlier so we don't have to work around them.


> ---
>  src/glsl/link_varyings.cpp | 43
> +++++++++++++++++++++++++++++++++++++++
>  src/glsl/linker.cpp        | 50 ------------------------------------
> ----------
>  2 files changed, 43 insertions(+), 50 deletions(-)
> 
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index 9f6467b..d5c1b20 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -309,6 +309,45 @@ cross_validate_outputs_to_inputs(struct
> gl_shader_program *prog,
>     }
>  }
>  
> +/**
> + * Demote shader inputs and outputs that are not used in other
> stages, and
> + * remove them via dead code elimination.
> + */
> +void
> +remove_unused_shader_inputs_and_outputs(struct gl_shader_program
> *prog,
> +                                        gl_shader *sh,
> +                                        enum ir_variable_mode mode)
> +{
> +   bool var_demoted = false;
> +
> +   if (prog->SeparateShader)
> +      return;
> +
> +   foreach_in_list(ir_instruction, node, sh->ir) {
> +      ir_variable *const var = node->as_variable();
> +
> +      if ((var == NULL) || (var->data.mode != int(mode)))
> +         continue;
> +
> +      /* A shader 'in' or 'out' variable is only really an input or
> output if
> +       * its value is used by other shader stages. This will cause
> the
> +       * variable to have a location assigned.
> +       */
> +      if (var->data.is_unmatched_generic_inout) {
> +         assert(var->data.mode != ir_var_temporary);
> +         var->data.mode = ir_var_auto;
> +         var_demoted = true;
> +      }
> +   }
> +
> +   /* Eliminate code that is now dead due to unused inputs/outputs
> being
> +    * demoted.
> +    */
> +   if (var_demoted) {
> +      while (do_dead_code(sh->ir, false))
> +         ;
> +   }
> +}
>  
>  /**
>   * Initialize this object based on a string that was passed to
> @@ -1639,10 +1678,14 @@ assign_varying_locations(struct gl_context
> *ctx,
>  
>     if (!disable_varying_packing) {
>        if (producer) {
> +         remove_unused_shader_inputs_and_outputs(prog, producer,
> +                                                 ir_var_shader_out);
>           lower_packed_varyings(mem_ctx, slots_used,
> ir_var_shader_out,
>                                 0, producer);
>        }
>        if (consumer) {
> +         remove_unused_shader_inputs_and_outputs(prog, consumer,
> +                                                 ir_var_shader_in);
>           lower_packed_varyings(mem_ctx, slots_used,
> ir_var_shader_in,
>                                 consumer_vertices, consumer);
>        }
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index ff22b3d..17450b8 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -2723,30 +2723,6 @@ match_explicit_outputs_to_inputs(struct
> gl_shader_program *prog,
>  }
>  
>  /**
> - * Demote shader inputs and outputs that are not used in other
> stages
> - */
> -void
> -demote_shader_inputs_and_outputs(gl_shader *sh, enum
> ir_variable_mode mode)
> -{
> -   foreach_in_list(ir_instruction, node, sh->ir) {
> -      ir_variable *const var = node->as_variable();
> -
> -      if ((var == NULL) || (var->data.mode != int(mode)))
> -	 continue;
> -
> -      /* A shader 'in' or 'out' variable is only really an input or
> output if
> -       * its value is used by other shader stages.  This will cause
> the variable
> -       * to have a location assigned.
> -       */
> -      if (var->data.is_unmatched_generic_inout) {
> -         assert(var->data.mode != ir_var_temporary);
> -	 var->data.mode = ir_var_auto;
> -      }
> -   }
> -}
> -
> -
> -/**
>   * Store the gl_FragDepth layout in the gl_shader_program struct.
>   */
>  static void
> @@ -4442,15 +4418,6 @@ link_shaders(struct gl_context *ctx, struct
> gl_shader_program *prog)
>  
>        do_dead_builtin_varyings(ctx, sh, NULL,
>                                 num_tfeedback_decls,
> tfeedback_decls);
> -
> -      if (!prog->SeparateShader) {
> -         demote_shader_inputs_and_outputs(sh, ir_var_shader_out);
> -         /* Eliminate code that is now dead due to unused outputs
> being
> -          * demoted.
> -          */
> -         while (do_dead_code(sh->ir, false))
> -            ;
> -      }
>     }
>     else if (first == MESA_SHADER_FRAGMENT) {
>        /* If the program only contains a fragment shader...
> @@ -4467,13 +4434,6 @@ link_shaders(struct gl_context *ctx, struct
> gl_shader_program *prog)
>                                         0 /* num_tfeedback_decls */,
>                                         NULL /* tfeedback_decls */))
>              goto done;
> -      } else {
> -         demote_shader_inputs_and_outputs(sh, ir_var_shader_in);
> -         /* Eliminate code that is now dead due to unused inputs
> being
> -          * demoted.
> -          */
> -         while (do_dead_code(sh->ir, false))
> -            ;
>        }
>     }
>  
> @@ -4494,16 +4454,6 @@ link_shaders(struct gl_context *ctx, struct
> gl_shader_program *prog)
>                  next == MESA_SHADER_FRAGMENT ? num_tfeedback_decls :
> 0,
>                  tfeedback_decls);
>  
> -      demote_shader_inputs_and_outputs(sh_i, ir_var_shader_out);
> -      demote_shader_inputs_and_outputs(sh_next, ir_var_shader_in);
> -
> -      /* Eliminate code that is now dead due to unused outputs being
> demoted.
> -       */
> -      while (do_dead_code(sh_i->ir, false))
> -         ;
> -      while (do_dead_code(sh_next->ir, false))
> -         ;
> -
>        /* This must be done after all dead varyings are eliminated.
> */
>        if (!check_against_output_limit(ctx, prog, sh_i))
>           goto done;