[Mesa-dev] [PATCH] glsl: don't eliminate texcoords that can be set by GL_COORD_REPLACE

[Ian Romanick]

On 08/09/2013 01:40 PM, Marek Olšák wrote:
> Tested by examining generated TGSI shaders from piglit/glsl-routing.
>
> Cc: mesa-stable at lists.freedesktop.org

This patch was in my review-queue, and I forgot about it.  Sorry. :(

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

Since this also fixes an application, do you have any idea what could be 
done to make a piglit test to reproduce the failure?  We have some folks 
writing piglit tests for us this summer, and this sounds like a good one 
for them. :)

> ---
>   src/glsl/ir_optimization.h             |  2 +-
>   src/glsl/linker.cpp                    |  6 +++---
>   src/glsl/opt_dead_builtin_varyings.cpp | 27 +++++++++++++++++++--------
>   3 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/src/glsl/ir_optimization.h b/src/glsl/ir_optimization.h
> index a61227b..b79c2b7 100644
> --- a/src/glsl/ir_optimization.h
> +++ b/src/glsl/ir_optimization.h
> @@ -77,7 +77,7 @@ bool do_copy_propagation(exec_list *instructions);
>   bool do_copy_propagation_elements(exec_list *instructions);
>   bool do_constant_propagation(exec_list *instructions);
>   void do_dead_builtin_varyings(struct gl_context *ctx,
> -                              exec_list *producer, exec_list *consumer,
> +                              gl_shader *producer, gl_shader *consumer,
>                                 unsigned num_tfeedback_decls,
>                                 class tfeedback_decl *tfeedback_decls);
>   bool do_dead_code(exec_list *instructions, bool uniform_locations_assigned);
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index d36f627..f87ae0e 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -2091,7 +2091,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>               goto done;
>         }
>
> -      do_dead_builtin_varyings(ctx, sh->ir, NULL,
> +      do_dead_builtin_varyings(ctx, sh, NULL,
>                                  num_tfeedback_decls, tfeedback_decls);
>
>         demote_shader_inputs_and_outputs(sh, ir_var_shader_out);
> @@ -2106,7 +2106,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>          */
>         gl_shader *const sh = prog->_LinkedShaders[first];
>
> -      do_dead_builtin_varyings(ctx, NULL, sh->ir,
> +      do_dead_builtin_varyings(ctx, NULL, sh,
>                                  num_tfeedback_decls, tfeedback_decls);
>
>         demote_shader_inputs_and_outputs(sh, ir_var_shader_in);
> @@ -2130,7 +2130,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>                   tfeedback_decls, gs_input_vertices))
>            goto done;
>
> -      do_dead_builtin_varyings(ctx, sh_i->ir, sh_next->ir,
> +      do_dead_builtin_varyings(ctx, sh_i, sh_next,
>                   next == MESA_SHADER_FRAGMENT ? num_tfeedback_decls : 0,
>                   tfeedback_decls);
>
> diff --git a/src/glsl/opt_dead_builtin_varyings.cpp b/src/glsl/opt_dead_builtin_varyings.cpp
> index 2e813d2..6745d5c 100644
> --- a/src/glsl/opt_dead_builtin_varyings.cpp
> +++ b/src/glsl/opt_dead_builtin_varyings.cpp
> @@ -409,7 +409,7 @@ lower_texcoord_array(exec_list *ir, const varying_info_visitor *info)
>
>   void
>   do_dead_builtin_varyings(struct gl_context *ctx,
> -                         exec_list *producer, exec_list *consumer,
> +                         gl_shader *producer, gl_shader *consumer,
>                            unsigned num_tfeedback_decls,
>                            tfeedback_decl *tfeedback_decls)
>   {
> @@ -431,44 +431,55 @@ do_dead_builtin_varyings(struct gl_context *ctx,
>      varying_info_visitor consumer_info(ir_var_shader_in);
>
>      if (producer) {
> -      producer_info.get(producer, num_tfeedback_decls, tfeedback_decls);
> +      producer_info.get(producer->ir, num_tfeedback_decls, tfeedback_decls);
>
>         if (!consumer) {
>            /* At least eliminate unused gl_TexCoord elements. */
>            if (producer_info.lower_texcoord_array) {
> -            lower_texcoord_array(producer, &producer_info);
> +            lower_texcoord_array(producer->ir, &producer_info);
>            }
>            return;
>         }
>      }
>
>      if (consumer) {
> -      consumer_info.get(consumer, 0, NULL);
> +      consumer_info.get(consumer->ir, 0, NULL);
>
>         if (!producer) {
>            /* At least eliminate unused gl_TexCoord elements. */
>            if (consumer_info.lower_texcoord_array) {
> -            lower_texcoord_array(consumer, &consumer_info);
> +            lower_texcoord_array(consumer->ir, &consumer_info);
>            }
>            return;
>         }
>      }
>
> -   /* Eliminate the varyings unused by the other shader. */
> +   /* Eliminate the outputs unused by the consumer. */
>      if (producer_info.lower_texcoord_array ||
>          producer_info.color_usage ||
>          producer_info.has_fog) {
> -      replace_varyings_visitor(producer,
> +      replace_varyings_visitor(producer->ir,
>                                  &producer_info,
>                                  consumer_info.texcoord_usage,
>                                  consumer_info.color_usage,
>                                  consumer_info.has_fog);
>      }
>
> +   /* The gl_TexCoord fragment shader inputs can be initialized
> +    * by GL_COORD_REPLACE, so we can't eliminate them.
> +    *
> +    * This doesn't prevent elimination of the gl_TexCoord elements which
> +    * are not read by the fragment shader. We want to eliminate those anyway.
> +    */
> +   if (consumer->Type == GL_FRAGMENT_SHADER) {
> +      producer_info.texcoord_usage = (1 << MAX_TEXTURE_COORD_UNITS) - 1;
> +   }
> +
> +   /* Eliminate the inputs uninitialized by the producer. */
>      if (consumer_info.lower_texcoord_array ||
>          consumer_info.color_usage ||
>          consumer_info.has_fog) {
> -      replace_varyings_visitor(consumer,
> +      replace_varyings_visitor(consumer->ir,
>                                  &consumer_info,
>                                  producer_info.texcoord_usage,
>                                  producer_info.color_usage,
>