Abdiel Janulgue <abdiel.janul...@linux.intel.com> writes: > 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.janul...@linux.intel.com> > --- > src/mesa/drivers/dri/i965/brw_gs.c | 1 + > src/mesa/drivers/dri/i965/brw_program.h | 3 +++ > src/mesa/drivers/dri/i965/brw_vs.c | 3 +++ > src/mesa/drivers/dri/i965/brw_wm.c | 22 ++++++++++++++++++++++ > 4 files changed, 29 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/brw_gs.c > b/src/mesa/drivers/dri/i965/brw_gs.c > index 17e87b8..7641cc5 100644 > --- a/src/mesa/drivers/dri/i965/brw_gs.c > +++ b/src/mesa/drivers/dri/i965/brw_gs.c > @@ -72,6 +72,7 @@ brw_codegen_gs_prog(struct brw_context *brw, > rzalloc_array(NULL, struct brw_image_param, gs->NumImages); > c.prog_data.base.base.nr_params = param_count; > c.prog_data.base.base.nr_image_params = gs->NumImages; > + c.prog_data.base.base.nr_ubo_params = brw_count_ubo_params(gs); > 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_program.h > b/src/mesa/drivers/dri/i965/brw_program.h > index 00e8f3f..20f5371 100644 > --- a/src/mesa/drivers/dri/i965/brw_program.h > +++ b/src/mesa/drivers/dri/i965/brw_program.h > @@ -182,6 +182,9 @@ void > brw_dump_ir(const char *stage, struct gl_shader_program *shader_prog, > struct gl_shader *shader, struct gl_program *prog); > > +int > +brw_count_ubo_params(struct gl_shader *fs); > + > #ifdef __cplusplus > } /* extern "C" */ > #endif > diff --git a/src/mesa/drivers/dri/i965/brw_vs.c > b/src/mesa/drivers/dri/i965/brw_vs.c > index 8501796..1ec2bc9 100644 > --- a/src/mesa/drivers/dri/i965/brw_vs.c > +++ b/src/mesa/drivers/dri/i965/brw_vs.c > @@ -141,6 +141,9 @@ brw_codegen_vs_prog(struct brw_context *brw, > stage_prog_data->nr_image_params); > stage_prog_data->nr_params = param_count; > > + stage_prog_data->nr_ubo_params = 0;
This assignment seems redundant, the prog data struct is already memset
to zero at the top of the function.
> + if (vs)
> + stage_prog_data->nr_ubo_params = brw_count_ubo_params(vs);
You could move this assignment into the other 'if (vs)' conditional
above.
> stage_prog_data->nr_gather_table = 0;
> stage_prog_data->gather_table =
> rzalloc_size(NULL, sizeof(*stage_prog_data->gather_table) *
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c
> b/src/mesa/drivers/dri/i965/brw_wm.c
> index 204baa6..44efba0 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -33,6 +33,7 @@
> #include "main/framebuffer.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"
> @@ -149,6 +150,23 @@ brw_wm_prog_data_compare(const void *in_a, const void
> *in_b)
> return true;
> }
>
> +int
The result of this function is always non-negative so I guess its return
type should be unsigned.
> +brw_count_ubo_params(struct gl_shader *shader)
> +{
> + int nr_ubo = 0;
Same here.
> + for (int i = 0; i < shader->NumUniformBlocks; i++) {
> + for (int p = 0; p < shader->UniformBlocks[i].NumUniforms; p++) {
> + const struct glsl_type *type =
> shader->UniformBlocks[i].Uniforms[p].Type;
> + int array_sz = glsl_get_array_size(type);
> + array_sz = MAX2(array_sz, 1);
> + int components =
> glsl_get_components(glsl_get_type_without_array(type));
> + nr_ubo += components * array_sz;
Why don't you just do 'nr_ubo +=
shader->UniformBlocks[i].Uniforms[p].Type->component_slots();'?
> + }
> + }
> +
> + return nr_ubo;
> +}
> +
I guess brw_shader.cpp would be a more appropriate place for this
function rather than brw_wm.c since it doesn't do anything specific to
fragment shaders. If anything you'd be able to use the C++ methods of
glsl_type and the last two patches would become unnecessary.
> /**
> * All Mesa program -> GPU code generation goes through this function.
> * Depending on the instructions used (i.e. flow control instructions)
> @@ -208,6 +226,10 @@ brw_codegen_wm_prog(struct brw_context *brw,
> prog_data.base.nr_image_params);
> prog_data.base.nr_params = param_count;
>
> + prog_data.base.nr_ubo_params = 0;
> + if (fs)
> + prog_data.base.nr_ubo_params = brw_count_ubo_params(fs);
> +
Same comments as for brw_codegen_vs_prog(). And I guess you're missing
an analogous change for compute shaders?
> prog_data.base.nr_gather_table = 0;
> prog_data.base.gather_table =
> rzalloc_size(NULL, sizeof(*prog_data.base.gather_table) *
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
