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.
With s/CEILING/DIV_ROUND_UP/, this gets:
Cc: "10.5" <mesa-sta...@lists.freedesktop.org>
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
>
> 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: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
