<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Thu, Jun 30, 2016 at 11:42 AM, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Saturday, June 25, 2016 8:37:47 AM PDT Rob Clark wrote: > From: Rob Clark <<a href="mailto:robclark@freedesktop.org">robclark@freedesktop.org</a>> > > 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 <<a href="mailto:robclark@freedesktop.org">robclark@freedesktop.org</a>> > --- > 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. </blockquote><div> </div><div>That was my suggestion. I figured the gallium people would want it too. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > 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 <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> Patch 2 also gets an R-b with Jason's feedback (d[0] and u[0] on LHS) taken care of. <div class="HOEnZb"><div class="h5"> > + && 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. */ > </div></div></blockquote></div> </div></div>