[Mesa-dev] [PATCH 02/21] i965: Make setup_vec4_uniform_value and _image_uniform_values take an offset

Jason Ekstrand jason at jlekstrand.net
Thu Aug 20 07:32:53 PDT 2015


On Thu, Aug 20, 2015 at 7:20 AM, Francisco Jerez <currojerez at riseup.net> wrote:
> Jason Ekstrand <jason at jlekstrand.net> writes:
>
>> This way they don't implicitly increment the uniforms variable and don't
>> have to be called in-sequence during uniform setup.
>>
>> Cc: Francisco Jerez <currojerez at riseup.net>
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp           |  7 ++++---
>>  src/mesa/drivers/dri/i965/brw_fs.h             |  3 ++-
>>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp       |  7 +++++--
>>  src/mesa/drivers/dri/i965/brw_shader.cpp       | 16 +++++++++-------
>>  src/mesa/drivers/dri/i965/brw_shader.h         |  6 ++++--
>>  src/mesa/drivers/dri/i965/brw_vec4.h           |  3 ++-
>>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 18 ++++++++++++------
>>  7 files changed, 38 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 47cc167..6ee9f3a 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -943,16 +943,17 @@ fs_visitor::import_uniforms(fs_visitor *v)
>>  }
>>
>>  void
>> -fs_visitor::setup_vec4_uniform_value(const gl_constant_value *values,
>> +fs_visitor::setup_vec4_uniform_value(unsigned param_offset,
>> +                                     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];
>> +      stage_prog_data->param[param_offset + i] = &values[i];
>>
>>     for (unsigned i = n; i < 4; ++i)
>> -      stage_prog_data->param[uniforms++] = &zero;
>> +      stage_prog_data->param[param_offset + i] = &zero;
>>  }
>>
>>  fs_reg *
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
>> index 1a56c2a..9484e63 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>> @@ -291,7 +291,8 @@ public:
>>
>>     struct brw_reg interp_reg(int location, int channel);
>>
>> -   virtual void setup_vec4_uniform_value(const gl_constant_value *values,
>> +   virtual void setup_vec4_uniform_value(unsigned param_offset,
>> +                                         const gl_constant_value *values,
>>                                           unsigned n);
>>
>>     int implied_mrf_writes(fs_inst *inst);
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index ce4153d..7873d3f 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -244,10 +244,13 @@ 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 =
>
> const?

That line gets deleted 3 or 4 patches later so I'd rather not deal
with the merge conflicts.

>>              BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1);
>>
>> -         setup_image_uniform_values(storage);
>> +         setup_image_uniform_values(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 48fbe8f..a7453fa 100644
>> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
>> @@ -1420,7 +1420,8 @@ backend_shader::assign_common_binding_table_offsets(uint32_t next_binding_table_
>>  }
>>
>>  void
>> -backend_shader::setup_image_uniform_values(const gl_uniform_storage *storage)
>> +backend_shader::setup_image_uniform_values(unsigned param_offset,
>> +                                           const gl_uniform_storage *storage)
>>  {
>>     const unsigned stage = _mesa_program_enum_to_shader_stage(prog->Target);
>>
>> @@ -1431,18 +1432,19 @@ 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_vec4_uniform_value(
>> +      setup_vec4_uniform_value(param_offset,
>>           (const gl_constant_value *)&param->surface_idx, 1);
>> -      setup_vec4_uniform_value(
>> +      setup_vec4_uniform_value(param_offset + 4,
>>           (const gl_constant_value *)param->offset, 2);
>> -      setup_vec4_uniform_value(
>> +      setup_vec4_uniform_value(param_offset + 8,
>>           (const gl_constant_value *)param->size, 3);
>> -      setup_vec4_uniform_value(
>> +      setup_vec4_uniform_value(param_offset + 12,
>>           (const gl_constant_value *)param->stride, 4);
>> -      setup_vec4_uniform_value(
>> +      setup_vec4_uniform_value(param_offset + 16,
>>           (const gl_constant_value *)param->tiling, 3);
>> -      setup_vec4_uniform_value(
>> +      setup_vec4_uniform_value(param_offset + 20,
>>           (const gl_constant_value *)param->swizzling, 2);
>> +      param_offset += 24;
>>
> These could use the symbolic BRW_IMAGE_PARAM_* defines instead of
> hardcoded constants.  With that fixed:

I'd forgotten those existed.  Fixed locally.

> Reviewed-by: Francisco Jerez <currojerez at riseup.net>

Thanks!

>>        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 933869a..d449ffa 100644
>> --- a/src/mesa/drivers/dri/i965/brw_shader.h
>> +++ b/src/mesa/drivers/dri/i965/brw_shader.h
>> @@ -270,9 +270,11 @@ public:
>>
>>     virtual void invalidate_live_intervals() = 0;
>>
>> -   virtual void setup_vec4_uniform_value(const gl_constant_value *values,
>> +   virtual void setup_vec4_uniform_value(unsigned param_offset,
>> +                                         const gl_constant_value *values,
>>                                           unsigned n) = 0;
>> -   void setup_image_uniform_values(const gl_uniform_storage *storage);
>> +   void setup_image_uniform_values(unsigned param_offset,
>> +                                   const gl_uniform_storage *storage);
>>  };
>>
>>  uint32_t brw_texture_offset(int *offsets, unsigned num_components);
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
>> index f8f31fc..b21f8dc 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
>> @@ -177,7 +177,8 @@ public:
>>     void fail(const char *msg, ...);
>>
>>     void setup_uniform_clipplane_values(gl_clip_plane *clip_planes);
>> -   virtual void setup_vec4_uniform_value(const gl_constant_value *values,
>> +   virtual void setup_vec4_uniform_value(unsigned param_offset,
>> +                                         const gl_constant_value *values,
>>                                           unsigned n);
>>     void setup_uniform_values(ir_variable *ir);
>>     void setup_builtin_uniform_values(ir_variable *ir);
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> index 64d4abe..f5701c8 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> @@ -696,18 +696,21 @@ dst_reg::dst_reg(class vec4_visitor *v, const struct glsl_type *type)
>>  }
>>
>>  void
>> -vec4_visitor::setup_vec4_uniform_value(const gl_constant_value *values,
>> +vec4_visitor::setup_vec4_uniform_value(unsigned param_offset,
>> +                                       const gl_constant_value *values,
>>                                         unsigned n)
>>  {
>>     static const gl_constant_value zero = { 0 };
>>
>> +   assert(param_offset % 4 == 0);
>> +
>>     for (unsigned i = 0; i < n; ++i)
>> -      stage_prog_data->param[4 * uniforms + i] = &values[i];
>> +      stage_prog_data->param[param_offset + i] = &values[i];
>>
>>     for (unsigned i = n; i < 4; ++i)
>> -      stage_prog_data->param[4 * uniforms + i] = &zero;
>> +      stage_prog_data->param[param_offset + i] = &zero;
>>
>> -   uniform_vector_size[uniforms++] = n;
>> +   uniform_vector_size[param_offset / 4] = n;
>>  }
>>
>>  /* Our support for uniforms is piggy-backed on the struct
>> @@ -743,9 +746,12 @@ vec4_visitor::setup_uniform_values(ir_variable *ir)
>>                                       storage->type->matrix_columns);
>>        const unsigned vector_size = storage->type->vector_elements;
>>
>> -      for (unsigned s = 0; s < vector_count; s++)
>> -         setup_vec4_uniform_value(&storage->storage[s * vector_size],
>> +      for (unsigned s = 0; s < vector_count; s++) {
>> +         setup_vec4_uniform_value(uniforms * 4,
>> +                                  &storage->storage[s * vector_size],
>>                                    vector_size);
>> +         uniforms++;
>> +      }
>>     }
>>  }
>>
>> --
>> 2.4.3


More information about the mesa-dev mailing list