On Saturday, June 25, 2016 8:37:47 AM PDT Rob Clark wrote: > From: Rob Clark <robcl...@freedesktop.org> > > Some games are sloppy.. perhaps because it is defined behavior for DX or > perhaps because nv blob driver defaults things to zero. > > So add driconf param to force uninitialized variables to default to zero. > > This issue was observed with rust, from steam store. But has surfaced > elsewhere in the past. > > Signed-off-by: Rob Clark <robcl...@freedesktop.org> > --- > Note that I left out the drirc bit, since not entirely sure how to > identify this game. (I don't actually have the game, just working off > of an apitrace) > > Possibly worth mentioning that for the shaders using uninitialized vars > having zero-initializers lets constant-propagation get rid of a whole > lot of instructions. One shader I saw dropped to less than half of > it's original instruction count. > > Second patch in the series is just fixing an i965 bug that was exposed > by this patch.
I'm a bit surprised to see this at the GLSL IR level...handling it for
nir_ssa_undef would probably be simpler. But I suppose this works too.
> src/compiler/glsl/ast_to_hir.cpp | 9 +++++++++
> src/compiler/glsl/glsl_parser_extras.cpp | 1 +
> src/compiler/glsl/glsl_parser_extras.h | 1 +
> src/gallium/include/state_tracker/st_api.h | 1 +
> src/gallium/state_trackers/dri/dri_screen.c | 2 ++
> src/mesa/drivers/dri/common/xmlpool/t_options.h | 5 ++++-
> src/mesa/drivers/dri/i965/brw_context.c | 2 ++
> src/mesa/drivers/dri/i965/intel_screen.c | 4 ++++
> src/mesa/main/mtypes.h | 5 +++++
> src/mesa/state_tracker/st_extensions.c | 2 ++
> 10 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/src/compiler/glsl/ast_to_hir.cpp
> b/src/compiler/glsl/ast_to_hir.cpp
> index 0cfce68..d2c284f 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -4697,6 +4697,15 @@ ast_declarator_list::hir(exec_list *instructions,
> apply_layout_qualifier_to_variable(&this->type->qualifier, var, state,
> &loc);
>
> + if ((var->data.mode == ir_var_auto || var->data.mode ==
> ir_var_temporary)
> + && (var->type->base_type >= GLSL_TYPE_UINT)
> + && (var->type->base_type <= GLSL_TYPE_BOOL)
I'd prefer:
&& (var->type->is_numeric() || var->type->is_boolean())
Either way,
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
Patch 2 also gets an R-b with Jason's feedback (d[0] and u[0] on LHS)
taken care of.
> + && state->zero_init) {
> + const ir_constant_data data = {0};
> + var->data.has_initializer = true;
> + var->constant_initializer = new(var) ir_constant(var->type, &data);
> + }
> +
> if (this->type->qualifier.flags.q.invariant) {
> if (!is_varying_var(var, state->stage)) {
> _mesa_glsl_error(&loc, state,
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp
> b/src/compiler/glsl/glsl_parser_extras.cpp
> index 09f7477..fc2859a 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -74,6 +74,7 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct
> gl_context *_ctx,
> /* Set default language version and extensions */
> this->language_version = 110;
> this->forced_language_version = ctx->Const.ForceGLSLVersion;
> + this->zero_init = ctx->Const.GLSLZeroInit;
> this->es_shader = false;
> this->ARB_texture_rectangle_enable = true;
>
> diff --git a/src/compiler/glsl/glsl_parser_extras.h
> b/src/compiler/glsl/glsl_parser_extras.h
> index 8c43292..669b3d1 100644
> --- a/src/compiler/glsl/glsl_parser_extras.h
> +++ b/src/compiler/glsl/glsl_parser_extras.h
> @@ -306,6 +306,7 @@ struct _mesa_glsl_parse_state {
> bool es_shader;
> unsigned language_version;
> unsigned forced_language_version;
> + bool zero_init;
> gl_shader_stage stage;
>
> /**
> diff --git a/src/gallium/include/state_tracker/st_api.h
> b/src/gallium/include/state_tracker/st_api.h
> index 41daa47..21d5177 100644
> --- a/src/gallium/include/state_tracker/st_api.h
> +++ b/src/gallium/include/state_tracker/st_api.h
> @@ -242,6 +242,7 @@ struct st_config_options
> unsigned force_glsl_version;
> boolean force_s3tc_enable;
> boolean allow_glsl_extension_directive_midshader;
> + boolean glsl_zero_init;
> };
>
> /**
> diff --git a/src/gallium/state_trackers/dri/dri_screen.c
> b/src/gallium/state_trackers/dri/dri_screen.c
> index 2ac55c8..b16585a 100644
> --- a/src/gallium/state_trackers/dri/dri_screen.c
> +++ b/src/gallium/state_trackers/dri/dri_screen.c
> @@ -74,6 +74,7 @@ const __DRIconfigOptionsExtension gallium_config_options = {
>
> DRI_CONF_SECTION_MISCELLANEOUS
> DRI_CONF_ALWAYS_HAVE_DEPTH_BUFFER("false")
> + DRI_CONF_GLSL_ZERO_INIT("false")
> DRI_CONF_SECTION_END
> DRI_CONF_END
> };
> @@ -98,6 +99,7 @@ dri_fill_st_options(struct st_config_options *options,
> driQueryOptionb(optionCache, "force_s3tc_enable");
> options->allow_glsl_extension_directive_midshader =
> driQueryOptionb(optionCache,
> "allow_glsl_extension_directive_midshader");
> + options->glsl_zero_init = driQueryOptionb(optionCache, "glsl_zero_init");
> }
>
> static const __DRIconfig **
> diff --git a/src/mesa/drivers/dri/common/xmlpool/t_options.h
> b/src/mesa/drivers/dri/common/xmlpool/t_options.h
> index 4b298a4..341c376 100644
> --- a/src/mesa/drivers/dri/common/xmlpool/t_options.h
> +++ b/src/mesa/drivers/dri/common/xmlpool/t_options.h
> @@ -337,7 +337,10 @@ DRI_CONF_OPT_BEGIN_B(always_have_depth_buffer, def) \
> DRI_CONF_DESC(en,gettext("Create all visuals with a depth buffer")) \
> DRI_CONF_OPT_END
>
> -
> +#define DRI_CONF_GLSL_ZERO_INIT(def) \
> +DRI_CONF_OPT_BEGIN_B(glsl_zero_init, def) \
> + DRI_CONF_DESC(en,gettext("Force uninitialized variables to default
> to zero")) \
> +DRI_CONF_OPT_END
>
> /**
> * \brief Initialization configuration options
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index c7a66cb..66d4e94 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -826,6 +826,8 @@ brw_process_driconf_options(struct brw_context *brw)
> ctx->Const.AllowGLSLExtensionDirectiveMidShader =
> driQueryOptionb(options, "allow_glsl_extension_directive_midshader");
>
> + ctx->Const.GLSLZeroInit = driQueryOptionb(options, "glsl_zero_init");
> +
> brw->dual_color_blend_by_location =
> driQueryOptionb(options, "dual_color_blend_by_location");
> }
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> b/src/mesa/drivers/dri/i965/intel_screen.c
> index 869119b..5aa28c6 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -88,6 +88,10 @@ DRI_CONF_BEGIN
> DRI_CONF_DESC(en, "Perform code generation at shader link time.")
> DRI_CONF_OPT_END
> DRI_CONF_SECTION_END
> +
> + DRI_CONF_SECTION_MISCELLANEOUS
> + DRI_CONF_GLSL_ZERO_INIT("false")
> + DRI_CONF_SECTION_END
> DRI_CONF_END
> };
>
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 73ae55d..2210658 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -3514,6 +3514,11 @@ struct gl_constants
> GLboolean AllowGLSLExtensionDirectiveMidShader;
>
> /**
> + * Force uninitialized variables to default to zero.
> + */
> + GLboolean GLSLZeroInit;
> +
> + /**
> * Does the driver support real 32-bit integers? (Otherwise, integers are
> * simulated via floats.)
> */
> diff --git a/src/mesa/state_tracker/st_extensions.c
> b/src/mesa/state_tracker/st_extensions.c
> index 412f598..e56d282 100644
> --- a/src/mesa/state_tracker/st_extensions.c
> +++ b/src/mesa/state_tracker/st_extensions.c
> @@ -926,6 +926,8 @@ void st_init_extensions(struct pipe_screen *screen,
> extensions->EXT_texture_integer = GL_FALSE;
> }
>
> + consts->GLSLZeroInit = options->glsl_zero_init;
> +
> consts->UniformBooleanTrue = consts->NativeIntegers ? ~0U : fui(1.0f);
>
> /* Below are the cases which cannot be moved into tables easily. */
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
