Jason Ekstrand <ja...@jlekstrand.net> writes:
> Previously, setting up image uniforms relied on being called after
> fs_visitor::uniforms was set and with fs_visitor::uniforms not allocating
> space for it. This made sense in an ir_visitor world because the visitor
> assigns locations and uploads data as it walks through the variables. In
> NIR it also happened to work because nir_lower_io assumed zero space for
> images. In the near future, we will be able to reserve space using
> nir_lower_io and these invariants will be broken.
>
> This commit makes setup_image_uniforms take a pointer to the location in
> the prog_data params array that indicates where the image uniforms should
> be stored. This way it can be called from anywhere in the shader setup.
>
> v2: Fix nir_setup_uniform to correctly correspond with this change
> (rebase fail)
>
> Cc: Francisco Jerez <curroje...@riseup.net>
> Cc: Kenneth Graunke <kenn...@whitecape.org>
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 12 ------------
> src/mesa/drivers/dri/i965/brw_fs.h | 3 ---
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 6 ++++--
> src/mesa/drivers/dri/i965/brw_shader.cpp | 28 +++++++++++++++++++++-------
> src/mesa/drivers/dri/i965/brw_shader.h | 5 ++---
> 5 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 82cb499..d1550e6 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -942,18 +942,6 @@ fs_visitor::import_uniforms(fs_visitor *v)
> this->param_size = v->param_size;
> }
>
> -void
> -fs_visitor::setup_vector_uniform_values(const gl_constant_value *values,
> unsigned n)
> -{
> - static const gl_constant_value zero = { 0 };
> -
> - for (unsigned i = 0; i < n; ++i)
> - stage_prog_data->param[uniforms++] = &values[i];
> -
> - for (unsigned i = n; i < 4; ++i)
> - stage_prog_data->param[uniforms++] = &zero;
> -}
> -
> fs_reg *
> fs_visitor::emit_fragcoord_interpolation(bool pixel_center_integer,
> bool origin_upper_left)
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index 975183e..28fcfa3 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -291,9 +291,6 @@ public:
>
> struct brw_reg interp_reg(int location, int channel);
>
> - virtual void setup_vector_uniform_values(const gl_constant_value *values,
> - unsigned n);
> -
> int implied_mrf_writes(fs_inst *inst);
>
> virtual void dump_instructions();
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index ce4153d..c8ea649 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -244,10 +244,12 @@ fs_visitor::nir_setup_uniform(nir_variable *var)
> * space for them here at the end of the parameter array.
> */
> var->data.driver_location = uniforms;
> - param_size[uniforms] =
> + unsigned size =
> BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1);
>
> - setup_image_uniform_values(storage);
> + setup_image_uniform_values(stage_prog_data->param + uniforms,
> storage);
> + param_size[uniforms] = size;
> + uniforms += size;
> } else {
> unsigned slots = storage->type->component_slots();
> if (storage->array_elements)
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp
> b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index 6b92806..b31ae9b 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -1419,8 +1419,22 @@
> backend_shader::assign_common_binding_table_offsets(uint32_t
> next_binding_table_
> /* prog_data->base.binding_table.size will be set by
> brw_mark_surface_used. */
> }
>
> +static void
> +setup_vector_uniform_values(const gl_constant_value ***dst,
> + const gl_constant_value *values, unsigned n)
> +{
> + static const gl_constant_value zero = { 0 };
> +
> + for (unsigned i = 0; i < n; ++i)
> + *((*dst)++) = &values[i];
> +
> + for (unsigned i = n; i < 4; ++i)
> + *((*dst)++) = &zero;
> +}
> +
> void
> -backend_shader::setup_image_uniform_values(const gl_uniform_storage *storage)
> +backend_shader::setup_image_uniform_values(const gl_constant_value
> **prog_param,
> + const gl_uniform_storage *storage)
> {
> const unsigned stage = _mesa_program_enum_to_shader_stage(prog->Target);
>
> @@ -1431,17 +1445,17 @@ backend_shader::setup_image_uniform_values(const
> gl_uniform_storage *storage)
> /* Upload the brw_image_param structure. The order is expected to
> match
> * the BRW_IMAGE_PARAM_*_OFFSET defines.
> */
> - setup_vector_uniform_values(
> + setup_vector_uniform_values(&prog_param,
> (const gl_constant_value *)¶m->surface_idx, 1);
> - setup_vector_uniform_values(
> + setup_vector_uniform_values(&prog_param,
> (const gl_constant_value *)param->offset, 2);
> - setup_vector_uniform_values(
> + setup_vector_uniform_values(&prog_param,
> (const gl_constant_value *)param->size, 3);
> - setup_vector_uniform_values(
> + setup_vector_uniform_values(&prog_param,
> (const gl_constant_value *)param->stride, 4);
> - setup_vector_uniform_values(
> + setup_vector_uniform_values(&prog_param,
> (const gl_constant_value *)param->tiling, 3);
> - setup_vector_uniform_values(
> + setup_vector_uniform_values(&prog_param,
> (const gl_constant_value *)param->swizzling, 2);
>
> brw_mark_surface_used(
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.h
> b/src/mesa/drivers/dri/i965/brw_shader.h
> index 2cc97f2..2cd0a5a 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.h
> +++ b/src/mesa/drivers/dri/i965/brw_shader.h
> @@ -270,9 +270,8 @@ public:
>
> virtual void invalidate_live_intervals() = 0;
>
> - virtual void setup_vector_uniform_values(const gl_constant_value *values,
> - unsigned n) = 0;
> - void setup_image_uniform_values(const gl_uniform_storage *storage);
> + void setup_image_uniform_values(const gl_constant_value **prog_param,
> + const gl_uniform_storage *storage);
> };
>
> uint32_t brw_texture_offset(int *offsets, unsigned num_components);
> --
> 2.4.3Did you notice that this makes backend_shader::setup_image_uniform_values() FS-specific? I think passing the base uniform index where it should be set up would do what you want and at the same time could be implemented in a backend-agnostic way.
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
