[Mesa-dev] [PATCH] i965/nir: Use uniform index instead of lookup by name

[Matt Turner]

On Thu, Mar 10, 2016 at 10:13 AM, Topi Pohjolainen
<topi.pohjolainen at intel.com> wrote:
> Uniform linking in (see link_assign_uniform_locations()) already
> stores the index to the storage in ir_variable which is further
> stored into nir_variable (see nir_visitor::visit(ir_variable *)).
>
> Instead of doing uniform_num^2 string comparisons one can recur
> over the uniform type the same way uniform linking does.
>
> Unfortunately I didn't see any improvement in performance tests,
> at least on BDW. Only the the fps numbers in a few synthetic
> benchmarks started to vary more than before between two subsequent
> runs.
>
> CC: Kenneth Graunke <kenneth at whitecape.org>
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp | 113 +++++++++++++++----------
>  1 file changed, 67 insertions(+), 46 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
> index f3361d6..f8ee0af 100644
> --- a/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
> @@ -67,62 +67,83 @@ brw_nir_setup_glsl_builtin_uniform(nir_variable *var,
>  }
>
>  static void
> -brw_nir_setup_glsl_uniform(gl_shader_stage stage, nir_variable *var,
> -                           struct gl_shader_program *shader_prog,
> +brw_nir_setup_glsl_uniform(gl_shader_stage stage, bool is_scalar,
>                             struct brw_stage_prog_data *stage_prog_data,
> -                           bool is_scalar)
> +                           const struct gl_uniform_storage *storage,
> +                           unsigned *uniform_index)
>  {
> -   int namelen = strlen(var->name);
> -
> -   /* The data for our (non-builtin) uniforms is stored in a series of
> -    * gl_uniform_driver_storage structs for each subcomponent that
> -    * glGetUniformLocation() could name.  We know it's been set up in the same
> -    * order we'd walk the type, so walk the list of storage and find anything
> -    * with our name, or the prefix of a component that starts with our name.
> -    */
> -   unsigned uniform_index = var->data.driver_location / 4;
> -   for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) {
> -      struct gl_uniform_storage *storage = &shader_prog->UniformStorage[u];
> -
> -      if (storage->builtin)
> -         continue;
> +   if (storage->type->is_image()) {
> +      brw_setup_image_uniform_values(stage, stage_prog_data,
> +                                     *uniform_index, storage);
> +      *uniform_index +=
> +         BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1);
> +   } else {
> +      gl_constant_value *components = storage->storage;
> +      unsigned vector_count = (MAX2(storage->array_elements, 1) *
> +                               storage->type->matrix_columns);
> +      unsigned vector_size = storage->type->vector_elements;
> +
> +      for (unsigned s = 0; s < vector_count; s++) {
> +         unsigned i;
> +         for (i = 0; i < vector_size; i++) {
> +            stage_prog_data->param[(*uniform_index)++] = components++;
> +         }
>
> -      if (strncmp(var->name, storage->name, namelen) != 0 ||
> -          (storage->name[namelen] != 0 &&
> -           storage->name[namelen] != '.' &&
> -           storage->name[namelen] != '[')) {
> -         continue;
> +         if (!is_scalar) {
> +            /* Pad out with zeros if needed (only needed for vec4) */
> +            for (; i < 4; i++) {
> +               static const gl_constant_value zero = { 0.0 };
> +               stage_prog_data->param[(*uniform_index)++] = &zero;
> +            }
> +         }
>        }
> +   }
> +}
>
> -      if (storage->type->is_image()) {
> -         brw_setup_image_uniform_values(stage, stage_prog_data,
> -                                        uniform_index, storage);
> -         uniform_index +=
> -            BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1);
> -      } else {
> -         gl_constant_value *components = storage->storage;
> -         unsigned vector_count = (MAX2(storage->array_elements, 1) *
> -                                  storage->type->matrix_columns);
> -         unsigned vector_size = storage->type->vector_elements;
> -
> -         for (unsigned s = 0; s < vector_count; s++) {
> -            unsigned i;
> -            for (i = 0; i < vector_size; i++) {
> -               stage_prog_data->param[uniform_index++] = components++;
> -            }
> +/* This mirrors the breakdown of complex uniforms in link_uniforms.cpp */
> +static void
> +brw_nir_recur_to_glsl_uniform(gl_shader_stage stage, bool is_scalar,
> +                              struct brw_stage_prog_data *stage_prog_data,
> +                              const struct gl_uniform_storage **storage,
> +                              unsigned *uniform_index, const glsl_type *t)
> +{
> +   assert(!t->is_interface() && !t->without_array()->is_interface());
>
> -            if (!is_scalar) {
> -               /* Pad out with zeros if needed (only needed for vec4) */
> -               for (; i < 4; i++) {
> -                  static const gl_constant_value zero = { 0.0 };
> -                  stage_prog_data->param[uniform_index++] = &zero;
> -               }
> -            }
> -         }
> +   if (t->is_record()) {
> +      for (unsigned i = 0; i < t->length; i++) {
> +         brw_nir_recur_to_glsl_uniform(stage, is_scalar, stage_prog_data,
> +                                       storage, uniform_index,
> +                                       t->fields.structure[i].type);
> +      }
> +   } else if (t->without_array()->is_record() ||
> +              (t->is_array() && t->fields.array->is_array())) {
> +      for (unsigned i = 0; i < t->length; i++) {
> +         brw_nir_recur_to_glsl_uniform(stage, is_scalar, stage_prog_data,
> +                                       storage, uniform_index,
> +                                       t->fields.array);
>        }
> +   } else {
> +      brw_nir_setup_glsl_uniform(stage, is_scalar, stage_prog_data,
> +                                 *storage, uniform_index);
> +      ++(*storage);
>     }
>  }
>
> +static void
> +brw_nir_setup_glsl_uniform(gl_shader_stage stage, nir_variable *var,
> +                           struct gl_shader_program *shader_prog,
> +                           struct brw_stage_prog_data *stage_prog_data,
> +                           bool is_scalar)
> +{
> +   const unsigned location = var->data.location;
> +   const struct gl_uniform_storage * storage =

No space between * and variable name.