[Mesa-dev] [PATCH 03/11] i965: Pull stage_prog_data.nr_params out of the NIR shader

[Iago Toral]

On Wed, 2015-09-30 at 18:41 -0700, Jason Ekstrand wrote:
> Previously, we had a bunch of code in each stage to figure out how many
> slots we needed in stage_prog_data.param.  This code was mostly identical
> across the stages and had been copied and pasted around.  Unfortunately,
> this meant that any time you did something special, you had to add code for
> it to each of these places.  In particular, none of the stages took
> subroutines into account; they were working entirely by accident.  By
> taking this data from the NIR shader, we know the exact number of entries
> we need and everything goes a bit smoother.
> ---
>  src/mesa/drivers/dri/i965/brw_cs.c |  4 ++--
>  src/mesa/drivers/dri/i965/brw_gs.c |  5 ++---
>  src/mesa/drivers/dri/i965/brw_vs.c | 16 ++++------------
>  src/mesa/drivers/dri/i965/brw_wm.c | 10 +++-------
>  4 files changed, 11 insertions(+), 24 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_cs.c b/src/mesa/drivers/dri/i965/brw_cs.c
> index 02eeeda..24120fb 100644
> --- a/src/mesa/drivers/dri/i965/brw_cs.c
> +++ b/src/mesa/drivers/dri/i965/brw_cs.c
> @@ -30,6 +30,7 @@
>  #include "intel_mipmap_tree.h"
>  #include "brw_state.h"
>  #include "intel_batchbuffer.h"
> +#include "glsl/nir/nir.h"
>  
>  static bool
>  brw_codegen_cs_prog(struct brw_context *brw,
> @@ -55,8 +56,7 @@ brw_codegen_cs_prog(struct brw_context *brw,
>      * prog_data associated with the compiled program, and which will be freed
>      * by the state cache.
>      */
> -   int param_count = cs->base.num_uniform_components +
> -                     cs->base.NumImages * BRW_IMAGE_PARAM_SIZE;
> +   int param_count = cp->program.Base.nir->num_uniforms;
>  
>     /* The backend also sometimes adds params for texture size. */
>     param_count += 2 * ctx->Const.Program[MESA_SHADER_COMPUTE].MaxTextureImageUnits;
> diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c
> index 61e7b2a..0cf7ec8 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs.c
> @@ -32,6 +32,7 @@
>  #include "brw_vec4_gs_visitor.h"
>  #include "brw_state.h"
>  #include "brw_ff_gs.h"
> +#include "glsl/nir/nir.h"
>  
> 
>  bool
> @@ -60,9 +61,7 @@ brw_codegen_gs_prog(struct brw_context *brw,
>      * every uniform is a float which gets padded to the size of a vec4.
>      */
>     struct gl_shader *gs = prog->_LinkedShaders[MESA_SHADER_GEOMETRY];
> -   int param_count = gs->num_uniform_components * 4;
> -
> -   param_count += gs->NumImages * BRW_IMAGE_PARAM_SIZE;
> +   int param_count = gp->program.Base.nir->num_uniforms * 4;

I think the vec4 nir backend does not handle image uniforms at the
moment, does it? At least I see that the FS backend has code
specifically for that in fs_visitor::nir_setup_uniform. Not sure if we
support images in geometry stages though, but the code you remove seems
to account for that...

>     c.prog_data.base.base.param =
>        rzalloc_array(NULL, const gl_constant_value *, param_count);
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
> index e1a0d9c..391411c 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -109,18 +109,10 @@ brw_codegen_vs_prog(struct brw_context *brw,
>      * prog_data associated with the compiled program, and which will be freed
>      * by the state cache.
>      */
> -   int param_count;
> -   if (vs) {
> -      /* We add padding around uniform values below vec4 size, with the worst
> -       * case being a float value that gets blown up to a vec4, so be
> -       * conservative here.
> -       */
> -      param_count = vs->base.num_uniform_components * 4 +
> -                    vs->base.NumImages * BRW_IMAGE_PARAM_SIZE;
> -      stage_prog_data->nr_image_params = vs->base.NumImages;
> -   } else {
> -      param_count = vp->program.Base.Parameters->NumParameters * 4;
> -   }
> +   int param_count = vp->program.Base.nir->num_uniforms;
> +   if (!brw->intelScreen->compiler->scalar_vs)
> +      param_count *= 4;

Same thing here.

Also, I guess this also means that for the scalar_vs cases we were
computing a larger param_count than we really should, right?

>     /* vec4_visitor::setup_uniform_clipplane_values() also uploads user clip
>      * planes as uniforms.
>      */
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
> index cc97d6a..08f2416 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -35,6 +35,7 @@
>  #include "program/prog_parameter.h"
>  #include "program/program.h"
>  #include "intel_mipmap_tree.h"
> +#include "glsl/nir/nir.h"
>  
>  #include "util/ralloc.h"
>  
> @@ -173,14 +174,9 @@ brw_codegen_wm_prog(struct brw_context *brw,
>      * prog_data associated with the compiled program, and which will be freed
>      * by the state cache.
>      */
> -   int param_count;
> -   if (fs) {
> -      param_count = fs->base.num_uniform_components +
> -                    fs->base.NumImages * BRW_IMAGE_PARAM_SIZE;
> +   int param_count = fp->program.Base.nir->num_uniforms;
> +   if (fs)
>        prog_data.base.nr_image_params = fs->base.NumImages;
> -   } else {
> -      param_count = fp->program.Base.Parameters->NumParameters * 4;
> -   }
>     /* The backend also sometimes adds params for texture size. */
>     param_count += 2 * ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxTextureImageUnits;
>     prog_data.base.param =