On Mon, Feb 15, 2016 at 4:38 PM, Matt Turner <[email protected]> wrote: > On Mon, Feb 15, 2016 at 12:50 PM, Ilia Mirkin <[email protected]> wrote: >> In a few places your indentation is off -- please look at the 'git >> diff' output, it should be pretty obvious. You used 2 spaces instead >> of 3 (in just a handful of places). > > If you use vim, you can put something like this in your ~/.vimrc: > > autocmd BufNewFile,BufRead /home/mattst88/projects/mesa/* set > expandtab tabstop=8 softtabstop=3 shiftwidth=3 > autocmd BufNewFile,BufRead > /home/mattst88/projects/mesa/src/glsl/glcpp/* set noexpandtab > tabstop=8 softtabstop=8 shiftwidth=8 > autocmd BufNewFile,BufRead > /home/mattst88/projects/mesa/src/glsl/glsl_parser.yy set noexpandtab > tabstop=8 shiftwidth=8 > autocmd BufNewFile,BufRead /home/mattst88/projects/piglit/* set > noexpandtab tabstop=8 softtabstop=8 shiftwidth=8 > autocmd BufNewFile,BufRead Makefile* set noexpandtab tabstop=8 > softtabstop=8 shiftwidth=8 > autocmd BufNewFile,BufRead *.mk set noexpandtab tabstop=8 > softtabstop=8 shiftwidth=8 > > (it'll probably get line wrapped by gmail)
or just use https://github.com/chadversary/vim-mesa that also has the right cinoptions. > >> On Fri, Feb 12, 2016 at 7:38 AM, Plamena Manolova >> <[email protected]> wrote: >>> This patch moves the calculation of current uniforms to >>> link_uniforms, which makes use of UniformRemapTable which >>> stores all the reserved uniform locations. >>> >>> Location assignment for implicit uniforms now tries to use >>> any gaps left in the table after the location assignment >>> for explicit uniforms. This gives us more space to store more >>> uniforms. >>> >>> Patch is based on earlier patch with following changes/additions: >>> >>> 1: Move the counting of explicit locations to >>> check_explicit_uniform_locations and then pass >>> the number to link_assign_uniform_locations. >>> 2: Count the number of empty slots in UniformRemapTable >>> and store them in a list_head. >>> 3: Try to find an empty slot for implicit locations from >>> the list, if that fails resize UniformRemapTable. >>> >>> 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 >>> >>> Signed-off-by: Tapani Pälli <[email protected]> >>> Signed-off-by: Plamena Manolova <[email protected]> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93696 >>> --- >>> src/compiler/glsl/link_uniforms.cpp | 85 >>> ++++++++++++++++++++++++++++++++----- >>> src/compiler/glsl/linker.cpp | 73 ++++++++++++++++++++----------- >>> src/compiler/glsl/linker.h | 17 +++++++- >>> src/mesa/main/mtypes.h | 8 ++++ >>> 4 files changed, 148 insertions(+), 35 deletions(-) >>> >>> diff --git a/src/compiler/glsl/link_uniforms.cpp >>> b/src/compiler/glsl/link_uniforms.cpp >>> index 7072c16..aa07de3 100644 >>> --- a/src/compiler/glsl/link_uniforms.cpp >>> +++ b/src/compiler/glsl/link_uniforms.cpp >>> @@ -1038,9 +1038,43 @@ assign_hidden_uniform_slot_id(const char *name, >>> unsigned hidden_id, >>> uniform_size->map->put(hidden_uniform_start + hidden_id, name); >>> } >>> >>> +/** >>> + * Search through the list of empty blocks to find one that fits the >>> current >>> + * uniform. >>> + */ >>> +static int >>> +find_empty_block(struct gl_shader_program *prog, >>> + struct gl_uniform_storage *uniform) >>> +{ >>> + const unsigned entries = MAX2(1, uniform->array_elements); >>> + >>> + foreach_list_typed(struct empty_uniform_block, block, link, >>> + &prog->EmptyUniformLocations) { >>> + /* Found a block with enough slots to fit the uniform */ >>> + if (block->slots == entries) { >>> + unsigned start = block->start; >>> + exec_node_remove(&block->link); >>> + ralloc_free(block); >>> + >>> + return start; >>> + /* Found a block with more slots than needed. It can still be used. */ >>> + } else if (block->slots > entries) { >>> + unsigned start = block->start; >>> + block->start += entries; >>> + block->slots -= entries; >>> + >>> + return start; >>> + } >>> + } >>> + >>> + return -1; >>> +} >>> + >>> void >>> link_assign_uniform_locations(struct gl_shader_program *prog, >>> - unsigned int boolean_true) >>> + unsigned int boolean_true, >>> + unsigned int num_explicit_uniform_locs, >>> + unsigned int max_uniform_locs) >>> { >>> ralloc_free(prog->UniformStorage); >>> prog->UniformStorage = NULL; >>> @@ -1131,6 +1165,9 @@ link_assign_uniform_locations(struct >>> gl_shader_program *prog, >>> >>> parcel_out_uniform_storage parcel(prog, prog->UniformHash, uniforms, >>> data); >>> >>> + unsigned total_entries = num_explicit_uniform_locs; >>> + unsigned empty_locs = prog->NumUniformRemapTable - >>> num_explicit_uniform_locs; >>> + >>> for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) { >>> if (prog->_LinkedShaders[i] == NULL) >>> continue; >>> @@ -1194,21 +1231,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_uniform_locs) { >>> + linker_error(prog, "count of uniform locations >= >>> MAX_UNIFORM_LOCATIONS" >>> + "(%u >= %u)", total_entries, max_uniform_locs); >>> } >>> >>> /* Reserve all the explicit locations of the active subroutine >>> uniforms. */ >>> @@ -1284,5 +1343,11 @@ link_assign_uniform_locations(struct >>> gl_shader_program *prog, >>> >>> link_set_uniform_initializers(prog, boolean_true); >>> >>> + foreach_list_typed(struct empty_uniform_block, block, link, >>> + &prog->EmptyUniformLocations) { >>> + ralloc_free(block); >>> + } >>> + exec_list_make_empty(&prog->EmptyUniformLocations); >>> + >>> return; >>> } >>> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp >>> index bad1c17..ce4f07c 100644 >>> --- a/src/compiler/glsl/linker.cpp >>> +++ b/src/compiler/glsl/linker.cpp >>> @@ -3008,12 +3008,13 @@ check_image_resources(struct gl_context *ctx, >>> struct gl_shader_program *prog) >>> * for a variable, checks for overlaps between other uniforms using >>> explicit >>> * locations. >>> */ >>> -static bool >>> +static int >>> reserve_explicit_locations(struct gl_shader_program *prog, >>> string_to_uint_map *map, ir_variable *var) >>> { >>> unsigned slots = var->type->uniform_locations(); >>> unsigned max_loc = var->data.location + slots - 1; >>> + unsigned return_value = slots; >>> >>> /* Resize remap table if locations do not fit in the current one. */ >>> if (max_loc + 1 > prog->NumUniformRemapTable) { >>> @@ -3024,7 +3025,7 @@ reserve_explicit_locations(struct gl_shader_program >>> *prog, >>> >>> if (!prog->UniformRemapTable) { >>> linker_error(prog, "Out of memory during linking.\n"); >>> - return false; >>> + return -1; >>> } >>> >>> /* Initialize allocated space. */ >>> @@ -3042,8 +3043,10 @@ reserve_explicit_locations(struct gl_shader_program >>> *prog, >>> >>> /* Possibly same uniform from a different stage, this is ok. */ >>> unsigned hash_loc; >>> - if (map->get(hash_loc, var->name) && hash_loc == loc - i) >>> - continue; >>> + if (map->get(hash_loc, var->name) && hash_loc == loc - i) { >>> + return_value = 0; >>> + continue; >>> + } >>> >>> /* ARB_explicit_uniform_location specification states: >>> * >>> @@ -3055,7 +3058,7 @@ reserve_explicit_locations(struct gl_shader_program >>> *prog, >>> "location qualifier for uniform %s overlaps " >>> "previously used location\n", >>> var->name); >>> - return false; >>> + return -1; >>> } >>> >>> /* Initialize location as inactive before optimization >>> @@ -3067,7 +3070,7 @@ reserve_explicit_locations(struct gl_shader_program >>> *prog, >>> /* Note, base location used for arrays. */ >>> map->put(var->data.location, var->name); >>> >>> - return true; >>> + return return_value; >>> } >>> >>> static bool >>> @@ -3128,12 +3131,12 @@ reserve_subroutine_explicit_locations(struct >>> gl_shader_program *prog, >>> * any optimizations happen to handle also inactive uniforms and >>> * inactive array elements that may get trimmed away. >>> */ >>> -static void >>> +static int >>> check_explicit_uniform_locations(struct gl_context *ctx, >>> struct gl_shader_program *prog) >>> { >>> if (!ctx->Extensions.ARB_explicit_uniform_location) >>> - return; >>> + return -1; >>> >>> /* This map is used to detect if overlapping explicit locations >>> * occur with the same uniform (from different stage) or a different >>> one. >>> @@ -3142,7 +3145,7 @@ check_explicit_uniform_locations(struct gl_context >>> *ctx, >>> >>> if (!uniform_map) { >>> linker_error(prog, "Out of memory during linking.\n"); >>> - return; >>> + return -1; >>> } >>> >>> unsigned entries_total = 0; >>> @@ -3157,31 +3160,50 @@ 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; >>> + bool ret = false; >>> if (var->type->without_array()->is_subroutine()) >>> ret = reserve_subroutine_explicit_locations(prog, sh, var); >>> - else >>> - ret = reserve_explicit_locations(prog, uniform_map, var); >>> + else { >>> + int slots = reserve_explicit_locations(prog, uniform_map, >>> + var); >>> + if (slots != -1) { >>> + ret = true; >>> + entries_total += slots; >>> + } >>> + } >>> if (!ret) { >>> delete uniform_map; >>> - return; >>> + return -1; >>> } >>> } >>> } >>> } >>> >>> - /* 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); >>> + if (entries_total > 0) >>> + entries_total -= 1; >> >> This seems weird... why are you doing that? >> >>> + >>> + exec_list_make_empty(&prog->EmptyUniformLocations); >>> + struct empty_uniform_block *current_block = NULL; >>> + >>> + for (unsigned i = 0; i < prog->NumUniformRemapTable; i++) { >>> + /* We found empty space in UniformRemapTable. */ >>> + if (prog->UniformRemapTable[i] == NULL) { >>> + /* We've found the beginning of a new continous block of empty >>> slots */ >>> + if (!current_block || current_block->start + current_block->slots >>> != i) { >>> + current_block = rzalloc(NULL, struct empty_uniform_block); >> >> is using ralloc with a NULL context a good idea? I don't know too much >> about it, but I thought the whole point of it was to have non-null >> contexts... should you be using an existing ralloc context, or just >> use calloc? Not sure what the policy is... > > Right -- if it's not using a context, there's not much point. > > Either using the temporary mem_ctx, or just using malloc/free is the > right thing to do. I wouldn't feel bad about using malloc/free if you > know exactly where the object's lifetime ends. > _______________________________________________ > mesa-dev mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
