[Mesa-dev] [PATCH 16/27] i965: Include UBO parameter sizes in push constant parameters

[Timothy Arceri]

On Tue, 2015-04-28 at 23:08 +0300, Abdiel Janulgue wrote:
> Now that we consider UBO constants as push constants, we need to include
> the sizes of the UBO's constant slots in the visitor's uniform slot sizes.
> This information is needed to properly pack vector constants tightly next to
> each other.
> 
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_gs.c | 11 +++++++++++
>  src/mesa/drivers/dri/i965/brw_vs.c | 13 +++++++++++++
>  src/mesa/drivers/dri/i965/brw_wm.c | 13 +++++++++++++
>  3 files changed, 37 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c
> index 97658d5..2dc3ea1 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_types.h"
>  
> 
>  bool
> @@ -70,6 +71,16 @@ brw_compile_gs_prog(struct brw_context *brw,
>     c.prog_data.base.base.pull_param =
>        rzalloc_array(NULL, const gl_constant_value *, param_count);
>     c.prog_data.base.base.nr_params = param_count;
> +   c.prog_data.base.base.nr_ubo_params = 0;
> +   for (int i = 0; i < gs->NumUniformBlocks; i++) {
> +      for (int p = 0; p < gs->UniformBlocks[i].NumUniforms; p++) {
> +         const struct glsl_type *type = gs->UniformBlocks[i].Uniforms[p].Type;
> +         const struct glsl_type *elem = glsl_get_element_type(type);
> +         int array_sz = elem ? glsl_get_array_size(type) : 1;
> +         int components = elem ? glsl_get_components(elem) : glsl_get_components(type);

As mentioned on the previous patch I've sent a patch to remove the
element type helper. I'm not sure I understand the reason the nir
wrappers need to be used here can you explain for my benefit?

Another way to write this without element type could be something like
this:

const struct glsl_type *type = gs->UniformBlocks[i].Uniforms[p].Type;
int array_sz = MAX2(glsl_get_array_size(type), 1);
int components = glsl_get_components(glsl_get_type_without_array(type));

You would obviously need to wrapper the without_array() helper instead.

Assuming arrays of arrays support is required here in future (the spec
says uniform blocks can be arrays of arrays but I'm not overly familiar
with the code your working on) now the only bit missing would be
multiplying array size by the other array dimensions.


> +         c.prog_data.base.base.nr_ubo_params += components * array_sz;
> +      }
> +   }
>     c.prog_data.base.base.nr_gather_table = 0;
>     c.prog_data.base.base.gather_table =
>        rzalloc_size(NULL, sizeof(*c.prog_data.base.base.gather_table) *
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
> index 52333c9..86bef5e 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -37,6 +37,7 @@
>  #include "brw_state.h"
>  #include "program/prog_print.h"
>  #include "program/prog_parameter.h"
> +#include "glsl/nir/nir_types.h"
>  
>  #include "util/ralloc.h"
>  
> @@ -243,6 +244,18 @@ brw_compile_vs_prog(struct brw_context *brw,
>        rzalloc_array(NULL, const gl_constant_value *, param_count);
>     stage_prog_data->nr_params = param_count;
>  
> +   stage_prog_data->nr_ubo_params = 0;
> +   if (vs) {
> +      for (int i = 0; i < vs->NumUniformBlocks; i++) {
> +         for (int p = 0; p < vs->UniformBlocks[i].NumUniforms; p++) {
> +            const struct glsl_type *type = vs->UniformBlocks[i].Uniforms[p].Type;
> +            const struct glsl_type *elem = glsl_get_element_type(type);
> +            int array_sz = elem ? glsl_get_array_size(type) : 1;
> +            int components = elem ? glsl_get_components(elem) : glsl_get_components(type);
> +            stage_prog_data->nr_ubo_params += components * array_sz;
> +         }
> +      }
> +   }
>     stage_prog_data->nr_gather_table = 0;
>     stage_prog_data->gather_table = rzalloc_size(NULL, sizeof(*stage_prog_data->gather_table) *
>                                                  (stage_prog_data->nr_params +
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
> index 13a64d8..2060eab 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -38,6 +38,7 @@
>  #include "main/samplerobj.h"
>  #include "program/prog_parameter.h"
>  #include "program/program.h"
> +#include "glsl/nir/nir_types.h"
>  #include "intel_mipmap_tree.h"
>  
>  #include "util/ralloc.h"
> @@ -205,6 +206,18 @@ brw_compile_wm_prog(struct brw_context *brw,
>        rzalloc_array(NULL, const gl_constant_value *, param_count);
>     prog_data.base.nr_params = param_count;
>  
> +   prog_data.base.nr_ubo_params = 0;
> +   if (fs) {
> +      for (int i = 0; i < fs->NumUniformBlocks; i++) {
> +         for (int p = 0; p < fs->UniformBlocks[i].NumUniforms; p++) {
> +            const struct glsl_type *type = fs->UniformBlocks[i].Uniforms[p].Type;
> +            const struct glsl_type *elem = glsl_get_element_type(type);
> +            int array_sz = elem ? glsl_get_array_size(type) : 1;
> +            int components = elem ? glsl_get_components(elem) : glsl_get_components(type);
> +            prog_data.base.nr_ubo_params += components * array_sz;
> +         }
> +      }
> +   }
>     prog_data.base.nr_gather_table = 0;
>     prog_data.base.gather_table = rzalloc_size(NULL, sizeof(*prog_data.base.gather_table) *
>                                                (prog_data.base.nr_params +