[Mesa-dev] [PATCH] i965: Set nr_params to the number of uniform components in the VS/GS path.

[Francisco Jerez]

Francisco Jerez <currojerez at riseup.net> writes:

> 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.

Ping?

> ---
>  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(-)
>
> 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;
> -- 
> 2.1.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150316/05172b09/attachment.sig>