[Mesa-dev] [PATCH 2/2] linker: Eliminate more dead code after demoting shader inputs and outputs

[Yuanhan Liu]

On Fri, Oct 21, 2011 at 11:49:44AM -0700, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> Consider the following vertex shader and fragment shader:
> 
> // vertex shader
> varying vec4 v;
> uniform vec4 u;
> 
> void main() { gl_Position = vec4(0.0); v = u; }
> 
> // fragment shader
> void main() { gl_FragColor = vec4(0.0); }
> 
> Since the fragment shader does not use 'v', it is demoted from a
> varying to a simple global variable.  Once that happens, the
> assignment to 'v' is useless, and it should be removed.  In addition,
> 'u' is no longer active, and it should also be removed.
> 
> Performing extra dead code elimination after demoting shader inputs
> and outputs takes care of this.  This elimination must occur before
> assigning uniform locations, or the declaration of 'u' cannot be
> removed.
> 
> This change *breaks* the piglit test getuniform-01, but that test is
> already incorrect.  The test uses a vertex shader that assigns to a
> user-defined varying, but it has no fragment shader.  Since Mesa does
> not support ARB_separate_shader_objects (we only support the EXT
> version), the linker correctly eliminates the user-defined varying.
> The cascading effect is that the uniform queried by the C code of the
> test is also (correctly) eliminated.

Reviewed-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>	

> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41980
> Cc: Brian Paul <brianp at vmware.com>
> Cc: Bryan Cain <bryancain3 at gmail.com>
> Cc: Vinson Lee <vlee at vmware.com>
> Cc: José Fonseca <jfonseca at vmware.com>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/glsl/linker.cpp |   26 ++++++++++++++++++++++----
>  1 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index d4d2496..beadec6 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1746,10 +1746,6 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>  	 ;
>     }
>  
> -   update_array_sizes(prog);
> -
> -   assign_uniform_locations(prog);
> -
>     /* FINISHME: The value of the max_attribute_index parameter is
>      * FINISHME: implementation dependent based on the value of
>      * FINISHME: GL_MAX_VERTEX_ATTRIBS.  GL_MAX_VERTEX_ATTRIBS must be
> @@ -1785,6 +1781,12 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>     if (prog->_LinkedShaders[MESA_SHADER_VERTEX] != NULL) {
>        demote_shader_inputs_and_outputs(prog->_LinkedShaders[MESA_SHADER_VERTEX],
>  				       ir_var_out);
> +
> +      /* Eliminate code that is now dead due to unused vertex outputs being
> +       * demoted.
> +       */
> +      while (do_dead_code(prog->_LinkedShaders[MESA_SHADER_VERTEX]->ir, false))
> +	 ;
>     }
>  
>     if (prog->_LinkedShaders[MESA_SHADER_GEOMETRY] != NULL) {
> @@ -1793,14 +1795,30 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>        demote_shader_inputs_and_outputs(sh, ir_var_in);
>        demote_shader_inputs_and_outputs(sh, ir_var_inout);
>        demote_shader_inputs_and_outputs(sh, ir_var_out);
> +
> +      /* Eliminate code that is now dead due to unused geometry outputs being
> +       * demoted.
> +       */
> +      while (do_dead_code(prog->_LinkedShaders[MESA_SHADER_GEOMETRY]->ir, false))
> +	 ;
>     }
>  
>     if (prog->_LinkedShaders[MESA_SHADER_FRAGMENT] != NULL) {
>        gl_shader *const sh = prog->_LinkedShaders[MESA_SHADER_FRAGMENT];
>  
>        demote_shader_inputs_and_outputs(sh, ir_var_in);
> +
> +      /* Eliminate code that is now dead due to unused fragment inputs being
> +       * demoted.  This shouldn't actually do anything other than remove
> +       * declarations of the (now unused) global variables.
> +       */
> +      while (do_dead_code(prog->_LinkedShaders[MESA_SHADER_FRAGMENT]->ir, false))
> +	 ;
>     }
>  
> +   update_array_sizes(prog);
> +   assign_uniform_locations(prog);
> +
>     /* OpenGL ES requires that a vertex shader and a fragment shader both be
>      * present in a linked program.  By checking for use of shading language
>      * version 1.00, we also catch the GL_ARB_ES2_compatibility case.
> -- 
> 1.7.6.4
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev