[Mesa-dev] [PATCH v2] glsl: move uniform calculation to link_uniforms
Tapani Pälli
tapani.palli at intel.com
Tue Jan 19 03:35:37 PST 2016
On 01/19/2016 01:14 PM, Ilia Mirkin wrote:
> The data structure is a (memory) heap... there appears to be one in
> mesa/main/mm.h. There's also one in nouveau_heap.h which is quite
> simple and totally unreliant on nouveau, just happens to be there. How
> hard would it be to integrate something like that?
>
> The trouble with adding slow things is that you forget about them, and
> they're not _that_ slow, but this stuff adds up.
The solution I had in mind is to build a list of empty slots when
allocating remaptable or while finding slots (keep pushing unused empty
slots to list) ... but if possible I would prefer optimization later.
First of all, this is quite exotic path to hit with a real program (last
words ... yes yes). Secondly, and more importantly, we can apply for
certification sooner, there are very few failures left.
> -ilia
>
> On Tue, Jan 19, 2016 at 6:05 AM, Lofstedt, Marta
> <marta.lofstedt at intel.com> wrote:
>> 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
>> _______________________________________________
>> 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