[Mesa-dev] [PATCH v2] glsl: move uniform calculation to link_uniforms

Lofstedt, Marta marta.lofstedt at intel.com
Tue Jan 19 03:05:59 PST 2016


This seem a bit suboptimal, since the same space is potentially searched multiple times. However, I believe that a better solution would be to use some other data structure which would probably require quite a big effort, so for now, this is:

Reviewed-by: Marta Lofstedt <marta.lofstedt at intel.com>


> -----Original Message-----
> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
> Behalf Of Tapani Pälli
> Sent: Tuesday, January 19, 2016 11:17 AM
> To: mesa-dev at lists.freedesktop.org
> Subject: [Mesa-dev] [PATCH v2] glsl: move uniform calculation to
> link_uniforms
> 
> Patch moves uniform calculation to happen during link_uniforms, this is
> possible with help of UniformRemapTable that has all the reserved locations.
> 
> Location assignment for implicit locations is changed so that we utilize also
> the 'holes' that explicit uniform location assignment might have left in
> UniformRemapTable, this makes it possible to fit more uniforms as
> previously we were lazy here and wasting space.
> 
> Fixes following CTS tests:
>    ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max
>    ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max-
> array
> 
> v2: code cleanups (Matt), increment NumUniformRemapTable correctly
>     (Timothy), fix find_empty_block to work like intended (sigh) and
>     add some more comments.
> 
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> ---
>  src/glsl/link_uniforms.cpp | 87
> ++++++++++++++++++++++++++++++++++++++++------
>  src/glsl/linker.cpp        | 19 ++++------
>  src/glsl/linker.h          |  3 +-
>  3 files changed, 85 insertions(+), 24 deletions(-)
> 
> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp index
> 33b2d4c..76ee70d 100644
> --- a/src/glsl/link_uniforms.cpp
> +++ b/src/glsl/link_uniforms.cpp
> @@ -1057,9 +1057,40 @@ assign_hidden_uniform_slot_id(const char
> *name, unsigned hidden_id,
>     uniform_size->map->put(hidden_uniform_start + hidden_id, name);  }
> 
> +/**
> + * Search UniformRemapTable for empty block big enough to hold given
> uniform.
> + * TODO Optimize this algorithm later if it turns out to be a major
> bottleneck.
> + */
> +static int
> +find_empty_block(struct gl_shader_program *prog,
> +                 struct gl_uniform_storage *uniform) {
> +   const unsigned entries = MAX2(1, uniform->array_elements);
> +   for (unsigned i = 0, j; i < prog->NumUniformRemapTable; i++) {
> +      /* We found empty space in UniformRemapTable. */
> +      if (prog->UniformRemapTable[i] == NULL) {
> +         for (j = i; j < entries && j < prog->NumUniformRemapTable; j++) {
> +            if (prog->UniformRemapTable[j] != NULL) {
> +               /* Entries do not fit in this space, continue searching
> +                * after this location.
> +                */
> +               i = j + 1;
> +               break;
> +            }
> +         }
> +         /* Entries fit, we can return this location. */
> +         if (i != j + 1) {
> +            return i;
> +         }
> +      }
> +   }
> +   return -1;
> +}
> +
>  void
>  link_assign_uniform_locations(struct gl_shader_program *prog,
> -                              unsigned int boolean_true)
> +                              unsigned int boolean_true,
> +                              unsigned int max_locations)
>  {
>     ralloc_free(prog->UniformStorage);
>     prog->UniformStorage = NULL;
> @@ -1150,6 +1181,20 @@ link_assign_uniform_locations(struct
> gl_shader_program *prog,
> 
>     parcel_out_uniform_storage parcel(prog->UniformHash, uniforms, data);
> 
> +   unsigned total_entries = 0;
> +
> +   /* Calculate amount of 'holes' left after explicit locations were
> +    * reserved from UniformRemapTable.
> +    */
> +   unsigned empty_locs = 0;
> +   for (unsigned i = 0; i < prog->NumUniformRemapTable; i++)
> +      if (prog->UniformRemapTable[i] == NULL)
> +         empty_locs++;
> +
> +   /* Add all the reserved explicit locations - empty locations in remap table.
> */
> +   if (prog->NumUniformRemapTable)
> +      total_entries = (prog->NumUniformRemapTable - 1) - empty_locs;
> +
>     for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
>        if (prog->_LinkedShaders[i] == NULL)
>  	 continue;
> @@ -1213,21 +1258,43 @@ link_assign_uniform_locations(struct
> gl_shader_program *prog,
>        /* how many new entries for this uniform? */
>        const unsigned entries = MAX2(1, uniforms[i].array_elements);
> 
> -      /* resize remap table to fit new entries */
> -      prog->UniformRemapTable =
> -         reralloc(prog,
> -                  prog->UniformRemapTable,
> -                  gl_uniform_storage *,
> -                  prog->NumUniformRemapTable + entries);
> +      /* Find UniformRemapTable for empty blocks where we can fit this
> uniform. */
> +      int chosen_location = -1;
> +
> +      if (empty_locs)
> +         chosen_location = find_empty_block(prog, &uniforms[i]);
> +
> +      if (chosen_location != -1) {
> +         empty_locs -= entries;
> +      } else {
> +         chosen_location = prog->NumUniformRemapTable;
> +
> +         /* Add new entries to the total amount of entries. */
> +         total_entries += entries;
> +
> +         /* resize remap table to fit new entries */
> +         prog->UniformRemapTable =
> +            reralloc(prog,
> +                     prog->UniformRemapTable,
> +                     gl_uniform_storage *,
> +                     prog->NumUniformRemapTable + entries);
> +         prog->NumUniformRemapTable += entries;
> +      }
> 
>        /* set pointers for this uniform */
>        for (unsigned j = 0; j < entries; j++)
> -         prog->UniformRemapTable[prog->NumUniformRemapTable+j] =
> &uniforms[i];
> +         prog->UniformRemapTable[chosen_location + j] = &uniforms[i];
> 
>        /* set the base location in remap table for the uniform */
> -      uniforms[i].remap_location = prog->NumUniformRemapTable;
> +      uniforms[i].remap_location = chosen_location;
> +   }
> 
> -      prog->NumUniformRemapTable += entries;
> +    /* Verify that total amount of entries for explicit and implicit locations
> +     * is less than MAX_UNIFORM_LOCATIONS.
> +     */
> +   if (total_entries >= max_locations) {
> +      linker_error(prog, "count of uniform locations >=
> MAX_UNIFORM_LOCATIONS"
> +                   "(%u >= %u)", total_entries, max_locations);
>     }
> 
>     /* Reserve all the explicit locations of the active subroutine uniforms. */
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index 6657777..5be8d9f
> 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -3146,7 +3146,6 @@ check_explicit_uniform_locations(struct gl_context
> *ctx,
>        return;
>     }
> 
> -   unsigned entries_total = 0;
>     for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
>        struct gl_shader *sh = prog->_LinkedShaders[i];
> 
> @@ -3158,8 +3157,6 @@ check_explicit_uniform_locations(struct gl_context
> *ctx,
>           if (!var || var->data.mode != ir_var_uniform)
>              continue;
> 
> -         entries_total += var->type->uniform_locations();
> -
>           if (var->data.explicit_location) {
>              bool ret;
>              if (var->type->without_array()->is_subroutine())
> @@ -3173,15 +3170,6 @@ check_explicit_uniform_locations(struct
> gl_context *ctx,
>           }
>        }
>     }
> -
> -   /* Verify that total amount of entries for explicit and implicit locations
> -    * is less than MAX_UNIFORM_LOCATIONS.
> -    */
> -   if (entries_total >= ctx->Const.MaxUserAssignableUniformLocations) {
> -      linker_error(prog, "count of uniform locations >=
> MAX_UNIFORM_LOCATIONS"
> -                   "(%u >= %u)", entries_total,
> -                   ctx->Const.MaxUserAssignableUniformLocations);
> -   }
>     delete uniform_map;
>  }
> 
> @@ -4556,7 +4544,12 @@ link_shaders(struct gl_context *ctx, struct
> gl_shader_program *prog)
>        goto done;
> 
>     update_array_sizes(prog);
> -   link_assign_uniform_locations(prog, ctx->Const.UniformBooleanTrue);
> +   link_assign_uniform_locations(prog, ctx->Const.UniformBooleanTrue,
> +
> + ctx->Const.MaxUserAssignableUniformLocations);
> +
> +   if (!prog->LinkStatus)
> +      goto done;
> +
>     link_assign_atomic_counter_resources(ctx, prog);
>     store_fragdepth_layout(prog);
> 
> diff --git a/src/glsl/linker.h b/src/glsl/linker.h index c80be1c..76f95c0 100644
> --- a/src/glsl/linker.h
> +++ b/src/glsl/linker.h
> @@ -35,7 +35,8 @@ link_invalidate_variable_locations(exec_list *ir);
> 
>  extern void
>  link_assign_uniform_locations(struct gl_shader_program *prog,
> -                              unsigned int boolean_true);
> +                              unsigned int boolean_true,
> +                              unsigned int max_locations);
> 
>  extern void
>  link_set_uniform_initializers(struct gl_shader_program *prog,
> --
> 2.5.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list