[Mesa-dev] [PATCH] glsl: fix names in lower_constant_arrays_to_uniforms

[Kenneth Graunke]

On Thursday, March 19, 2015 11:59:50 AM Tapani Pälli wrote:
> Patch adds a counter around the lowering pass so that arrays
> from different stages cannot end up having same name for uniform.
> 
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89590
> Cc: 10.5 10.4 <mesa-stable at lists.freedesktop.org>
> ---
>  src/glsl/ir_optimization.h                  |  2 +-
>  src/glsl/linker.cpp                         |  5 +++--
>  src/glsl/lower_const_arrays_to_uniforms.cpp | 13 +++++++------
>  3 files changed, 11 insertions(+), 9 deletions(-)

This seems a bit complicated - why not just pass in the shader stage,
and use that in the variable name?

Although I suppose you could have multiple FSes and run into the same
problem...

Just using %p with the variable would totally solve this problem, but
it would make the debug output different on each invocation, which is
unfortunate.

> 
> diff --git a/src/glsl/ir_optimization.h b/src/glsl/ir_optimization.h
> index e6939f3..261d2fb 100644
> --- a/src/glsl/ir_optimization.h
> +++ b/src/glsl/ir_optimization.h
> @@ -117,7 +117,7 @@ bool lower_noise(exec_list *instructions);
>  bool lower_variable_index_to_cond_assign(exec_list *instructions,
>      bool lower_input, bool lower_output, bool lower_temp, bool lower_uniform);
>  bool lower_quadop_vector(exec_list *instructions, bool dont_lower_swz);
> -bool lower_const_arrays_to_uniforms(exec_list *instructions);
> +bool lower_const_arrays_to_uniforms(exec_list *instructions, unsigned *amount);
>  bool lower_clip_distance(gl_shader *shader);
>  void lower_output_reads(exec_list *instructions);
>  bool lower_packing_builtins(exec_list *instructions, int op_mask);
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 0c44677..a688baa 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -2693,7 +2693,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>      * uniforms, and varyings.  Later optimization could possibly make
>      * some of that unused.
>      */
> -   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> +   for (unsigned i = 0, arrays = 0; i < MESA_SHADER_STAGES; i++) {
>        if (prog->_LinkedShaders[i] == NULL)
>  	 continue;
>  
> @@ -2710,7 +2710,8 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>                                      ctx->Const.NativeIntegers))
>  	 ;
>  
> -      lower_const_arrays_to_uniforms(prog->_LinkedShaders[i]->ir);
> +      lower_const_arrays_to_uniforms(prog->_LinkedShaders[i]->ir,
> +                                     &arrays);
>     }
>  
>     /* Check and validate stream emissions in geometry shaders */
> diff --git a/src/glsl/lower_const_arrays_to_uniforms.cpp b/src/glsl/lower_const_arrays_to_uniforms.cpp
> index 2243f47..49c1f31 100644
> --- a/src/glsl/lower_const_arrays_to_uniforms.cpp
> +++ b/src/glsl/lower_const_arrays_to_uniforms.cpp
> @@ -45,11 +45,11 @@
>  namespace {
>  class lower_const_array_visitor : public ir_rvalue_visitor {
>  public:
> -   lower_const_array_visitor(exec_list *insts)
> +   lower_const_array_visitor(exec_list *insts, unsigned *amount)
>     {
>        instructions = insts;
>        progress = false;
> -      index = 0;
> +      array_amount = amount;
>     }
>  
>     bool run()
> @@ -63,7 +63,7 @@ public:
>  private:
>     exec_list *instructions;
>     bool progress;
> -   unsigned index;
> +   unsigned *array_amount;
>  };
>  
>  void
> @@ -82,7 +82,8 @@ lower_const_array_visitor::handle_rvalue(ir_rvalue **rvalue)
>  
>     void *mem_ctx = ralloc_parent(con);
>  
> -   char *uniform_name = ralloc_asprintf(mem_ctx, "constarray__%d", index++);
> +   char *uniform_name = ralloc_asprintf(mem_ctx, "constarray__%d",
> +                                        (*array_amount)++);
>  
>     ir_variable *uni =
>        new(mem_ctx) ir_variable(con->type, uniform_name, ir_var_uniform);
> @@ -104,8 +105,8 @@ lower_const_array_visitor::handle_rvalue(ir_rvalue **rvalue)
>  } /* anonymous namespace */
>  
>  bool
> -lower_const_arrays_to_uniforms(exec_list *instructions)
> +lower_const_arrays_to_uniforms(exec_list *instructions, unsigned *amount)
>  {
> -   lower_const_array_visitor v(instructions);
> +   lower_const_array_visitor v(instructions, amount);
>     return v.run();
>  }
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150319/655d94c7/attachment.sig>