[Mesa-dev] [PATCH 06/11] i965/shader: Pull setup_image_uniform_values out of backend_shader
Iago Toral
itoral at igalia.com
Fri Oct 2 01:33:16 PDT 2015
On Fri, 2015-10-02 at 10:29 +0200, Iago Toral wrote:
> On Wed, 2015-09-30 at 18:41 -0700, Jason Ekstrand wrote:
> > I tried to do this once before but Curro pointed out that having it in
> > backend_shader meant it could use the setup_vec4_uniform_values helper
> > which did different things in vec4 and fs. Now the setup_uniform_values
> > function differs only by an assert in the two backends so there's no real
> > good reason to be using it anymore.
> > ---
> > src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 3 +-
> > src/mesa/drivers/dri/i965/brw_shader.cpp | 52 +++++++++++++++++++++-----------
> > src/mesa/drivers/dri/i965/brw_shader.h | 7 +++--
> > 3 files changed, 42 insertions(+), 20 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > index 7a965cd..d33e452 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > @@ -236,7 +236,8 @@ fs_visitor::nir_setup_uniform(nir_variable *var)
> > }
> >
> > if (storage->type->is_image()) {
> > - setup_image_uniform_values(index, storage);
> > + brw_setup_image_uniform_values(stage, stage_prog_data,
> > + index, storage);
> > } 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 72388ce..1d184a7 100644
> > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> > @@ -1419,32 +1419,50 @@ 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_vec4_uniform_value(const gl_constant_value **params,
> > + const gl_constant_value *values,
> > + unsigned n)
> > +{
> > + static const gl_constant_value zero = { 0 };
> > +
> > + for (unsigned i = 0; i < n; ++i)
> > + params[i] = &values[i];
> > +
> > + for (unsigned i = n; i < 4; ++i)
> > + params[i] = &zero;
> > +}
>
> I actually liked the version that received an offset into params better,
> since that allows us to assert that we are not messing our writes to
> params.
Oh, but that was a requirement exclusive to the vec4 backend, so I guess
this is okay...
> Not a big deal though, so either way:
>
> Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>
>
> > void
> > -backend_shader::setup_image_uniform_values(unsigned param_offset,
> > - const gl_uniform_storage *storage)
> > +brw_setup_image_uniform_values(gl_shader_stage stage,
> > + struct brw_stage_prog_data *stage_prog_data,
> > + unsigned param_start_index,
> > + const gl_uniform_storage *storage)
> > {
> > - const unsigned stage = _mesa_program_enum_to_shader_stage(prog->Target);
> > + const gl_constant_value **param =
> > + &stage_prog_data->param[param_start_index];
> >
> > for (unsigned i = 0; i < MAX2(storage->array_elements, 1); i++) {
> > const unsigned image_idx = storage->image[stage].index + i;
> > - const brw_image_param *param = &stage_prog_data->image_param[image_idx];
> > + const brw_image_param *image_param =
> > + &stage_prog_data->image_param[image_idx];
> >
> > /* Upload the brw_image_param structure. The order is expected to match
> > * the BRW_IMAGE_PARAM_*_OFFSET defines.
> > */
> > - setup_vec4_uniform_value(param_offset + BRW_IMAGE_PARAM_SURFACE_IDX_OFFSET,
> > - (const gl_constant_value *)¶m->surface_idx, 1);
> > - setup_vec4_uniform_value(param_offset + BRW_IMAGE_PARAM_OFFSET_OFFSET,
> > - (const gl_constant_value *)param->offset, 2);
> > - setup_vec4_uniform_value(param_offset + BRW_IMAGE_PARAM_SIZE_OFFSET,
> > - (const gl_constant_value *)param->size, 3);
> > - setup_vec4_uniform_value(param_offset + BRW_IMAGE_PARAM_STRIDE_OFFSET,
> > - (const gl_constant_value *)param->stride, 4);
> > - setup_vec4_uniform_value(param_offset + BRW_IMAGE_PARAM_TILING_OFFSET,
> > - (const gl_constant_value *)param->tiling, 3);
> > - setup_vec4_uniform_value(param_offset + BRW_IMAGE_PARAM_SWIZZLING_OFFSET,
> > - (const gl_constant_value *)param->swizzling, 2);
> > - param_offset += BRW_IMAGE_PARAM_SIZE;
> > + setup_vec4_uniform_value(param + BRW_IMAGE_PARAM_SURFACE_IDX_OFFSET,
> > + (const gl_constant_value *)&image_param->surface_idx, 1);
> > + setup_vec4_uniform_value(param + BRW_IMAGE_PARAM_OFFSET_OFFSET,
> > + (const gl_constant_value *)image_param->offset, 2);
> > + setup_vec4_uniform_value(param + BRW_IMAGE_PARAM_SIZE_OFFSET,
> > + (const gl_constant_value *)image_param->size, 3);
> > + setup_vec4_uniform_value(param + BRW_IMAGE_PARAM_STRIDE_OFFSET,
> > + (const gl_constant_value *)image_param->stride, 4);
> > + setup_vec4_uniform_value(param + BRW_IMAGE_PARAM_TILING_OFFSET,
> > + (const gl_constant_value *)image_param->tiling, 3);
> > + setup_vec4_uniform_value(param + BRW_IMAGE_PARAM_SWIZZLING_OFFSET,
> > + (const gl_constant_value *)image_param->swizzling, 2);
> > + param += BRW_IMAGE_PARAM_SIZE;
> >
> > brw_mark_surface_used(
> > stage_prog_data,
> > diff --git a/src/mesa/drivers/dri/i965/brw_shader.h b/src/mesa/drivers/dri/i965/brw_shader.h
> > index ccccf4d..ee6afce 100644
> > --- a/src/mesa/drivers/dri/i965/brw_shader.h
> > +++ b/src/mesa/drivers/dri/i965/brw_shader.h
> > @@ -273,12 +273,15 @@ public:
> > virtual void setup_vec4_uniform_value(unsigned param_offset,
> > const gl_constant_value *values,
> > unsigned n) = 0;
> > - void setup_image_uniform_values(unsigned param_offset,
> > - const gl_uniform_storage *storage);
> > };
> >
> > uint32_t brw_texture_offset(int *offsets, unsigned num_components);
> >
> > +void brw_setup_image_uniform_values(gl_shader_stage stage,
> > + struct brw_stage_prog_data *stage_prog_data,
> > + unsigned param_start_index,
> > + const gl_uniform_storage *storage);
> > +
> > #endif /* __cplusplus */
> >
> > enum brw_reg_type brw_type_for_base_type(const struct glsl_type *type);
>
More information about the mesa-dev
mailing list