[Mesa-dev] [PATCH] i965/nir: Use uniform index instead of lookup by name
Samuel Iglesias Gonsálvez
siglesias at igalia.com
Thu Mar 17 11:27:25 UTC 2016
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 10/03/16 19:58, Matt Turner wrote:
> 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.
With this change and assuming no regressions:
Reviewed-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> _______________________________________________ mesa-dev mailing
> list mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBCAAGBQJW6pSdAAoJEH/0ujLxfcNDf6IP/34xkqgjG+PgJorAxScRhO52
SQE++EhNNBbJLex9iRvLSALf/TWXtDZFPEnPI9QNYVF5kwwL7WSHtmZUqyD5W2q7
RIAJn3tEjZwSP74IqKNW0wWTkfLxasMMnjuYf2SSfGpRDQyzeQbkjaGZDYLC8Y/Y
95UVMh1pAsqqvY/vXD9SLaBFjrvz0E1Cc26jBtxqnUuSC+6Rss1VZ++BIdsnTUJD
rhCgAvjcpCaRTmen+q08HnyHx6oOGaNNtMlqJOXqCbZRDrw0PGEetwlVV29jRQ/z
E0a//SddDgZRloC+Q8Q+CexcIbhEJlCcwp0eQMjQD+McNCHKekRmNdCrZ15mOUDV
ysPyY/YPAb56OLD3AKClQzz+xTmaJ2Ckb81oUalHMI/LV65fHaj2lMwTXz0CZgy4
KCk3P9KLL4576FL5NXBFw0F6veEyENRECV/P7Ot81GgBB5Ef4bLqa8sT+F8QmZ6n
EyEP13SQadoS5kG2HiIfWPBRQGBNyC9haFdJ33CKcjDa/n5tVhPELtC4nyZL3/WI
P41J9KDaCDlklyN0XmxbpESvCrTGh6JuzaWXiTDdYp6RJJbPYlXov0TMFfi3bwYF
8uWEyCAkbqOUP388nbX1yQELWGi2segdYnznHbikLm8ogvszOYsdRx8Ij7upCLkd
wqb1zQycuZHrEHNmgwoB
=AlIf
-----END PGP SIGNATURE-----
More information about the mesa-dev
mailing list