[Mesa-dev] [PATCH 1/3] glsl: disable dead code removal of lowered ubos
Kenneth Graunke
kenneth at whitecape.org
Thu Dec 29 10:26:04 UTC 2016
On Thursday, December 29, 2016 1:40:00 PM PST Timothy Arceri wrote:
> This lets us assign uniform storage for packed UBOs after
> they have been lowered otherwise the var is removed too early.
> ---
> src/compiler/glsl/glsl_parser_extras.cpp | 5 +++--
> src/compiler/glsl/ir_optimization.h | 4 +++-
> src/compiler/glsl/link_varyings.cpp | 2 +-
> src/compiler/glsl/linker.cpp | 1 +
> src/compiler/glsl/opt_dead_code.cpp | 8 +++++---
> src/compiler/glsl/standalone.cpp | 1 +
> src/compiler/glsl/test_optpass.cpp | 5 +++--
> src/mesa/drivers/dri/i965/brw_link.cpp | 2 +-
> src/mesa/main/ff_fragment_shader.cpp | 2 +-
> src/mesa/program/ir_to_mesa.cpp | 2 +-
> src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 2 +-
> 11 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
> index f1820b9..673f6d2 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -1969,7 +1969,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, false, options,
> + while (do_common_optimization(shader->ir, false, false, false, options,
> ctx->Const.NativeIntegers))
> ;
>
> @@ -2067,6 +2067,7 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
> bool
> do_common_optimization(exec_list *ir, bool linked,
> bool uniform_locations_assigned,
> + bool ubos_lowered,
> const struct gl_shader_compiler_options *options,
> bool native_integers)
> {
> @@ -2109,7 +2110,7 @@ do_common_optimization(exec_list *ir, bool linked,
> }
>
> if (linked)
> - OPT(do_dead_code, ir, uniform_locations_assigned);
> + OPT(do_dead_code, ir, uniform_locations_assigned, ubos_lowered);
> else
> OPT(do_dead_code_unlinked, ir);
> OPT(do_dead_code_local, ir);
> diff --git a/src/compiler/glsl/ir_optimization.h b/src/compiler/glsl/ir_optimization.h
> index 0d6c4e6..88a4ebd 100644
> --- a/src/compiler/glsl/ir_optimization.h
> +++ b/src/compiler/glsl/ir_optimization.h
> @@ -77,6 +77,7 @@ enum lower_packing_builtins_op {
>
> bool do_common_optimization(exec_list *ir, bool linked,
> bool uniform_locations_assigned,
> + bool ubos_lowered,
> const struct gl_shader_compiler_options *options,
> bool native_integers);
>
> @@ -97,7 +98,8 @@ void do_dead_builtin_varyings(struct gl_context *ctx,
> gl_linked_shader *consumer,
> unsigned num_tfeedback_decls,
> class tfeedback_decl *tfeedback_decls);
> -bool do_dead_code(exec_list *instructions, bool uniform_locations_assigned);
> +bool do_dead_code(exec_list *instructions, bool uniform_locations_assigned,
> + bool ubos_lowered);
> 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/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
> index fe499a5..dd1c36a 100644
> --- a/src/compiler/glsl/link_varyings.cpp
> +++ b/src/compiler/glsl/link_varyings.cpp
> @@ -607,7 +607,7 @@ remove_unused_shader_inputs_and_outputs(bool is_separate_shader_object,
> /* Eliminate code that is now dead due to unused inputs/outputs being
> * demoted.
> */
> - while (do_dead_code(sh->ir, false))
> + while (do_dead_code(sh->ir, false, true))
> ;
>
> }
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 126d11d..d7ed436 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -5053,6 +5053,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
> }
>
> while (do_common_optimization(prog->_LinkedShaders[i]->ir, true, false,
> + false,
> &ctx->Const.ShaderCompilerOptions[i],
> ctx->Const.NativeIntegers))
> ;
> diff --git a/src/compiler/glsl/opt_dead_code.cpp b/src/compiler/glsl/opt_dead_code.cpp
> index 75e668a..980660e 100644
> --- a/src/compiler/glsl/opt_dead_code.cpp
> +++ b/src/compiler/glsl/opt_dead_code.cpp
> @@ -43,7 +43,8 @@ static bool debug = false;
> * for usage on an unlinked instruction stream.
> */
> bool
> -do_dead_code(exec_list *instructions, bool uniform_locations_assigned)
> +do_dead_code(exec_list *instructions, bool uniform_locations_assigned,
> + bool ubos_lowered)
> {
> ir_variable_refcount_visitor v;
> bool progress = false;
> @@ -144,7 +145,8 @@ do_dead_code(exec_list *instructions, bool uniform_locations_assigned)
> * layouts, do not eliminate it.
> */
> if (entry->var->is_in_buffer_block()) {
> - if (entry->var->get_interface_type_packing() !=
> + if (ubos_lowered ||
> + entry->var->get_interface_type_packing() !=
> GLSL_INTERFACE_PACKING_PACKED)
> continue;
Maybe format this as:
if (entry->var->is_in_buffer_block()) {
/* Don't eliminate UBO variables after lowering. We want
* the variables to live on so we can examine them when
* assigning uniform storage. Calling this pass before
* UBO lowering should have eliminated any truly dead
* variables anyway.
*/
if (ubos_lowered)
continue;
/* Section 2.11.6 (Uniform Variables) of the OpenGL ES
* 3.0.3 spec says:
*
* (rest of the comment above this block)
*/
if (entry->var->get_interface_type_packing() !=
GLSL_INTERFACE_PACKING_PACKED)
continue;
}
I think adding the comment and keeping both conditions separate might be
nice. But up to you.
Either way, this patch is
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
(I haven't looked at the other two yet.)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161229/49080b8b/attachment.sig>
More information about the mesa-dev
mailing list