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

[Ian Romanick]

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.

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