Kenneth Graunke <[email protected]> writes: > On Thursday, January 22, 2015 06:32:21 PM Francisco Jerez wrote: >> Both do_vs_prog and do_gs_prog initialize brw_stage_prog_data::nr_params to >> the number of uniform *vectors* required by the shader rather than the number >> of uniform components, contradicting the comment. This is inconsistent with >> what the state upload code and scalar path expect but it happens to work >> until >> Gen8 because vec4_visitor interprets it as a number of vectors on >> construction >> and later on overwrites its original value with the number of uniform >> components referenced by the shader. >> >> Also there's no need to add the number of samplers, they're not actually >> passed in as uniforms. >> >> Fixes a memory corruption issue on BDW with SIMD8 VS. >> --- >> src/mesa/drivers/dri/i965/brw_gs.c | 6 +----- >> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 3 ++- >> src/mesa/drivers/dri/i965/brw_vs.c | 10 +--------- >> 3 files changed, 4 insertions(+), 15 deletions(-) > > Yikes, sorry...I thought this patch landed a long time ago. > > This looks good to me, but I'm having a bit of trouble figuring out > what's actually changing. It looks like fs_visitor::assign_constant_locations > sets nr_params to an appropriate value, like the vec4 backend does. > > One difference I noticed was that fs_visitor::init() allocates the > param_size array to have nr_params elements, which would be too small; > your patch would fix that. I could see that wreaking random havoc. > Yeah, exactly, that's the reason for the memory corruption, the FS back-end assumes that nr_params is in components so it ends up allocating less memory than we need.
> With s/CEILING/DIV_ROUND_UP/, this gets: > > Cc: "10.5" <[email protected]> > Reviewed-by: Kenneth Graunke <[email protected]> > Thanks! >> >> diff --git a/src/mesa/drivers/dri/i965/brw_gs.c >> b/src/mesa/drivers/dri/i965/brw_gs.c >> index c7ebe5f..ce3cba4 100644 >> --- a/src/mesa/drivers/dri/i965/brw_gs.c >> +++ b/src/mesa/drivers/dri/i965/brw_gs.c >> @@ -69,11 +69,7 @@ do_gs_prog(struct brw_context *brw, >> rzalloc_array(NULL, const gl_constant_value *, param_count); >> c.prog_data.base.base.pull_param = >> rzalloc_array(NULL, const gl_constant_value *, param_count); >> - /* Setting nr_params here NOT to the size of the param and pull_param >> - * arrays, but to the number of uniform components vec4_visitor >> - * needs. vec4_visitor::setup_uniforms() will set it back to a proper >> value. >> - */ >> - c.prog_data.base.base.nr_params = ALIGN(param_count, 4) / 4 + >> gs->num_samplers; >> + c.prog_data.base.base.nr_params = param_count; >> >> if (brw->gen >= 7) { >> if (gp->program.OutputType == GL_POINTS) { >> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp >> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp >> index 8b8b27f..f06ee53 100644 >> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp >> @@ -3624,7 +3624,8 @@ vec4_visitor::vec4_visitor(struct brw_context *brw, >> */ >> this->uniform_array_size = 1; >> if (prog_data) { >> - this->uniform_array_size = MAX2(stage_prog_data->nr_params, 1); >> + this->uniform_array_size = MAX2(CEILING(stage_prog_data->nr_params, >> 4), >> + 1); >> } >> >> this->uniform_size = rzalloc_array(mem_ctx, int, >> this->uniform_array_size); >> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c >> b/src/mesa/drivers/dri/i965/brw_vs.c >> index 2d56b74..f360d4e 100644 >> --- a/src/mesa/drivers/dri/i965/brw_vs.c >> +++ b/src/mesa/drivers/dri/i965/brw_vs.c >> @@ -241,15 +241,7 @@ do_vs_prog(struct brw_context *brw, >> rzalloc_array(NULL, const gl_constant_value *, param_count); >> stage_prog_data->pull_param = >> rzalloc_array(NULL, const gl_constant_value *, param_count); >> - >> - /* Setting nr_params here NOT to the size of the param and pull_param >> - * arrays, but to the number of uniform components vec4_visitor >> - * needs. vec4_visitor::setup_uniforms() will set it back to a proper >> value. >> - */ >> - stage_prog_data->nr_params = ALIGN(param_count, 4) / 4; >> - if (vs) { >> - stage_prog_data->nr_params += vs->num_samplers; >> - } >> + stage_prog_data->nr_params = param_count; >> >> GLbitfield64 outputs_written = vp->program.Base.OutputsWritten; >> prog_data.inputs_read = vp->program.Base.InputsRead; >>
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
