[Mesa-dev] [PATCH v2] i965: Refactor image uniform setup
Jason Ekstrand
jason at jlekstrand.net
Wed Aug 19 06:38:45 PDT 2015
On Aug 19, 2015 2:56 AM, "Francisco Jerez" <currojerez at riseup.net> wrote:
>
> Jason Ekstrand <jason at 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 <currojerez at riseup.net>
> > Cc: Kenneth Graunke <kenneth at 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.3
>
> Did 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.
How is this FS-specific? setup_vector_uniform_values was exactly the same
code in both backends and the same as this static helper. They both have an
array of params. Am I missing something?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150819/3153ed9a/attachment-0001.html>
More information about the mesa-dev
mailing list