On Mon, Feb 15, 2016 at 12:50 PM, Ilia Mirkin <imir...@alum.mit.edu> 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) > On Fri, Feb 12, 2016 at 7:38 AM, Plamena Manolova > <plamena.manol...@intel.com> 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 <tapani.pa...@intel.com> >> Signed-off-by: Plamena Manolova <plamena.manol...@intel.com> >> 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 mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
