On Fri, Oct 21, 2011 at 11:49:43AM -0700, Ian Romanick wrote:
> From: Ian Romanick <ian.d.roman...@intel.com>
>
> Setting this flag prevents declarations of uniforms from being removed
> from the IR. Since the IR is directly used by several API functions
> that query uniforms in shaders, uniform declarations cannot be removed
> after the locations have been set. However, it should still be safe
> to reorder the declarations (this is not tested).
I was debugging this issue(uniform var removed) yesterday. One thing I didn't
understand is why it would fail the test:
72 if ((entry->referenced_count > entry->assigned_count)
73 || !entry->declaration)
74 continue;
I found the entry->referenced_count and entry->assigned_count both with
value of 0.
Here is the test shader:
#version 120
varying vec4 Color_out;
uniform vec4 expVal;
uniform sampler2D samp;
void main() {
vec4 v = texture2D(samp, gl_TexCoord[0].xy);
if ((expVal.r - v.r) != 0 &&
(expVal.g - v.g) != 0 &&
(expVal.b - v.b) != 0 &&
(expVal.a - v.a) != 0) {
Color_out = vec4(1.0, 0.0, 0.0, 1.0);
} else {
Color_out = v;
}
gl_Position = gl_Vertex;
}
#version 120
uniform vec4 expVal;
uniform sampler2D samp;
void main() {
vec4 v = texture2D(samp, gl_TexCoord[0].xy);
if ((expVal.r - v.r) != 0 &&
(expVal.g - v.g) != 0 &&
(expVal.b - v.b) != 0 &&
(expVal.a - v.a) != 0) {
gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
} else {
gl_FragColor = v;
}
}
I found that both samp and expVal are removed. This patch fix this
issue, though I still didn't get it why the refereneced_count would
be 0. So,
Reviewed-by: Yuanhan Liu <yuanhan....@linux.intel.com>
>
> Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41980
> Cc: Brian Paul <bri...@vmware.com>
> Cc: Bryan Cain <bryanca...@gmail.com>
> Cc: Vinson Lee <v...@vmware.com>
> Cc: José Fonseca <jfons...@vmware.com>
> Cc: Kenneth Graunke <kenn...@whitecape.org>
> ---
> src/glsl/glsl_parser_extras.cpp | 23 +++++++++++++++++++++--
> src/glsl/ir_optimization.h | 6 ++++--
> src/glsl/linker.cpp | 2 +-
> src/glsl/main.cpp | 2 +-
> src/glsl/opt_dead_code.cpp | 14 ++++++++++----
> src/glsl/test_optpass.cpp | 4 ++--
> src/mesa/drivers/dri/i965/brw_shader.cpp | 3 ++-
> src/mesa/main/ff_fragment_shader.cpp | 2 +-
> src/mesa/program/ir_to_mesa.cpp | 6 ++++--
> src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 4 +++-
> 10 files changed, 49 insertions(+), 17 deletions(-)
>
> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
> index a9075b2..e2112fe 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -883,8 +883,27 @@ ast_struct_specifier::ast_struct_specifier(char
> *identifier,
> this->declarations.push_degenerate_list_at_head(&declarator_list->link);
> }
>
> +/**
> + * Do the set of common optimizations passes
> + *
> + * \param ir List of instructions to be optimized
> + * \param linked Is the shader linked? This enables
> + * optimizations passes that remove code
> at
> + * global scope and could cause linking to
> + * fail.
> + * \param uniform_locations_assigned Have locations already been assigned
> for
> + * uniforms? This prevents the
> declarations
> + * of unused uniforms from being removed.
> + * The setting of this flag only matters
> if
> + * \c linked is \c true.
> + * \param max_unroll_iterations Maximum number of loop iterations to be
> + * unrolled. Setting to 0 forces all
> loops
> + * to be unrolled.
> + */
> bool
> -do_common_optimization(exec_list *ir, bool linked, unsigned
> max_unroll_iterations)
> +do_common_optimization(exec_list *ir, bool linked,
> + bool uniform_locations_assigned,
> + unsigned max_unroll_iterations)
> {
> GLboolean progress = GL_FALSE;
>
> @@ -900,7 +919,7 @@ do_common_optimization(exec_list *ir, bool linked,
> unsigned max_unroll_iteration
> progress = do_copy_propagation(ir) || progress;
> progress = do_copy_propagation_elements(ir) || progress;
> if (linked)
> - progress = do_dead_code(ir) || progress;
> + progress = do_dead_code(ir, uniform_locations_assigned) || progress;
> else
> progress = do_dead_code_unlinked(ir) || progress;
> progress = do_dead_code_local(ir) || progress;
> diff --git a/src/glsl/ir_optimization.h b/src/glsl/ir_optimization.h
> index af80e26..7b32e84 100644
> --- a/src/glsl/ir_optimization.h
> +++ b/src/glsl/ir_optimization.h
> @@ -37,7 +37,9 @@
> #define MOD_TO_FRACT 0x20
> #define INT_DIV_TO_MUL_RCP 0x40
>
> -bool do_common_optimization(exec_list *ir, bool linked, unsigned
> max_unroll_iterations);
> +bool do_common_optimization(exec_list *ir, bool linked,
> + bool uniform_locations_assigned,
> + unsigned max_unroll_iterations);
>
> bool do_algebraic(exec_list *instructions);
> bool do_constant_folding(exec_list *instructions);
> @@ -46,7 +48,7 @@ bool do_constant_variable_unlinked(exec_list *instructions);
> bool do_copy_propagation(exec_list *instructions);
> bool do_copy_propagation_elements(exec_list *instructions);
> bool do_constant_propagation(exec_list *instructions);
> -bool do_dead_code(exec_list *instructions);
> +bool do_dead_code(exec_list *instructions, bool uniform_locations_assigned);
> bool do_dead_code_local(exec_list *instructions);
> bool do_dead_code_unlinked(exec_list *instructions);
> bool do_dead_functions(exec_list *instructions);
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index a7c38a3..d4d2496 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1742,7 +1742,7 @@ link_shaders(struct gl_context *ctx, struct
> gl_shader_program *prog)
> if (ctx->ShaderCompilerOptions[i].LowerClipDistance)
> lower_clip_distance(prog->_LinkedShaders[i]->ir);
>
> - while (do_common_optimization(prog->_LinkedShaders[i]->ir, true, 32))
> + while (do_common_optimization(prog->_LinkedShaders[i]->ir, true,
> false, 32))
> ;
> }
>
> diff --git a/src/glsl/main.cpp b/src/glsl/main.cpp
> index 0192137..e174224 100644
> --- a/src/glsl/main.cpp
> +++ b/src/glsl/main.cpp
> @@ -166,7 +166,7 @@ compile_shader(struct gl_context *ctx, struct gl_shader
> *shader)
> if (!state->error && !shader->ir->is_empty()) {
> bool progress;
> do {
> - progress = do_common_optimization(shader->ir, false, 32);
> + progress = do_common_optimization(shader->ir, false, false, 32);
> } while (progress);
>
> validate_ir_tree(shader->ir);
> diff --git a/src/glsl/opt_dead_code.cpp b/src/glsl/opt_dead_code.cpp
> index cb500d2..5b9546a 100644
> --- a/src/glsl/opt_dead_code.cpp
> +++ b/src/glsl/opt_dead_code.cpp
> @@ -42,7 +42,7 @@ static bool debug = false;
> * for usage on an unlinked instruction stream.
> */
> bool
> -do_dead_code(exec_list *instructions)
> +do_dead_code(exec_list *instructions, bool uniform_locations_assigned)
> {
> ir_variable_refcount_visitor v;
> bool progress = false;
> @@ -94,10 +94,11 @@ do_dead_code(exec_list *instructions)
> */
>
> /* uniform initializers are precious, and could get used by another
> - * stage.
> + * stage. Also, once uniform locations have been assigned, the
> + * declaration cannot be deleted.
> */
> if (entry->var->mode == ir_var_uniform &&
> - entry->var->constant_value)
> + (uniform_locations_assigned || entry->var->constant_value))
> continue;
>
> entry->var->remove();
> @@ -132,7 +133,12 @@ do_dead_code_unlinked(exec_list *instructions)
> foreach_iter(exec_list_iterator, sigiter, *f) {
> ir_function_signature *sig =
> (ir_function_signature *) sigiter.get();
> - if (do_dead_code(&sig->body))
> + /* The setting of the uniform_locations_assigned flag here is
> + * irrelevent. If there is a uniform declaration encountered
> + * inside the body of the function, something has already gone
> + * terribly, terribly wrong.
> + */
> + if (do_dead_code(&sig->body, false))
> progress = true;
> }
> }
> diff --git a/src/glsl/test_optpass.cpp b/src/glsl/test_optpass.cpp
> index 89b7f83..6abafb5 100644
> --- a/src/glsl/test_optpass.cpp
> +++ b/src/glsl/test_optpass.cpp
> @@ -64,7 +64,7 @@ do_optimization(struct exec_list *ir, const char
> *optimization)
>
> if (sscanf(optimization, "do_common_optimization ( %d , %d ) ",
> &int_0, &int_1) == 2) {
> - return do_common_optimization(ir, int_0 != 0, int_1);
> + return do_common_optimization(ir, int_0 != 0, false, int_1);
> } else if (strcmp(optimization, "do_algebraic") == 0) {
> return do_algebraic(ir);
> } else if (strcmp(optimization, "do_constant_folding") == 0) {
> @@ -80,7 +80,7 @@ do_optimization(struct exec_list *ir, const char
> *optimization)
> } else if (strcmp(optimization, "do_constant_propagation") == 0) {
> return do_constant_propagation(ir);
> } else if (strcmp(optimization, "do_dead_code") == 0) {
> - return do_dead_code(ir);
> + return do_dead_code(ir, false);
> } else if (strcmp(optimization, "do_dead_code_local") == 0) {
> return do_dead_code_local(ir);
> } else if (strcmp(optimization, "do_dead_code_unlinked") == 0) {
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp
> b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index 0858c40..d9d9414 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -138,7 +138,8 @@ brw_link_shader(struct gl_context *ctx, struct
> gl_shader_program *prog)
> false /* loops */
> ) || progress;
>
> - progress = do_common_optimization(shader->ir, true, 32) || progress;
> + progress = do_common_optimization(shader->ir, true, true, 32)
> + || progress;
> } while (progress);
>
> validate_ir_tree(shader->ir);
> diff --git a/src/mesa/main/ff_fragment_shader.cpp
> b/src/mesa/main/ff_fragment_shader.cpp
> index 160a97c..3e449b0 100644
> --- a/src/mesa/main/ff_fragment_shader.cpp
> +++ b/src/mesa/main/ff_fragment_shader.cpp
> @@ -1464,7 +1464,7 @@ create_new_program(struct gl_context *ctx, struct
> state_key *key)
>
> validate_ir_tree(p.shader->ir);
>
> - while (do_common_optimization(p.shader->ir, false, 32))
> + while (do_common_optimization(p.shader->ir, false, false, 32))
> ;
> reparent_ir(p.shader->ir, p.shader->ir);
>
> diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
> index fecab50..635ebdd 100644
> --- a/src/mesa/program/ir_to_mesa.cpp
> +++ b/src/mesa/program/ir_to_mesa.cpp
> @@ -3212,7 +3212,9 @@ _mesa_ir_link_shader(struct gl_context *ctx, struct
> gl_shader_program *prog)
>
> progress = do_lower_jumps(ir, true, true, options->EmitNoMainReturn,
> options->EmitNoCont, options->EmitNoLoops) || progress;
>
> - progress = do_common_optimization(ir, true,
> options->MaxUnrollIterations) || progress;
> + progress = do_common_optimization(ir, true, true,
> + options->MaxUnrollIterations)
> + || progress;
>
> progress = lower_quadop_vector(ir, true) || progress;
>
> @@ -3321,7 +3323,7 @@ _mesa_glsl_compile_shader(struct gl_context *ctx,
> struct gl_shader *shader)
> /* Do some optimization at compile time to reduce shader IR size
> * and reduce later work if the same shader is linked multiple times
> */
> - while (do_common_optimization(shader->ir, false, 32))
> + while (do_common_optimization(shader->ir, false, false, 32))
> ;
>
> validate_ir_tree(shader->ir);
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 35fd1ff..145bd7d 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -5058,7 +5058,9 @@ st_link_shader(struct gl_context *ctx, struct
> gl_shader_program *prog)
>
> progress = do_lower_jumps(ir, true, true,
> options->EmitNoMainReturn, options->EmitNoCont, options->EmitNoLoops) ||
> progress;
>
> - progress = do_common_optimization(ir, true,
> options->MaxUnrollIterations) || progress;
> + progress = do_common_optimization(ir, true, true,
> + options->MaxUnrollIterations)
> + || progress;
>
> progress = lower_quadop_vector(ir, false) || progress;
>
> --
> 1.7.6.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
